diff --git a/src/node_http2.cc b/src/node_http2.cc index cc8afec207ffba..c8aa9e0729d4f7 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -180,6 +180,18 @@ inline void Http2Settings::RefreshDefaults(Environment* env) { (1 << IDX_SETTINGS_MAX_FRAME_SIZE); } +Http2Priority::Http2Priority(Environment* env, + Local parent, + Local weight, + Local exclusive) { + Local context = env->context(); + int32_t parent_ = parent->Int32Value(context).ToChecked(); + int32_t weight_ = weight->Int32Value(context).ToChecked(); + bool exclusive_ = exclusive->BooleanValue(context).ToChecked(); + DEBUG_HTTP2("Http2Priority: parent: %d, weight: %d, exclusive: %d\n", + parent_, weight_, exclusive_); + nghttp2_priority_spec_init(&spec, parent_, weight_, exclusive_ ? 1 : 0); +} Http2Session::Http2Session(Environment* env, Local wrap, @@ -258,12 +270,8 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen, buffer[PADDING_BUF_RETURN_VALUE] = frameLen; MakeCallback(env()->ongetpadding_string(), 0, nullptr); uint32_t retval = buffer[PADDING_BUF_RETURN_VALUE]; - retval = retval <= maxPayloadLen ? retval : maxPayloadLen; - retval = retval >= frameLen ? retval : frameLen; -#if defined(DEBUG) && DEBUG - CHECK_GE(retval, frameLen); - CHECK_LE(retval, maxPayloadLen); -#endif + retval = std::min(retval, static_cast(maxPayloadLen)); + retval = std::max(retval, static_cast(frameLen)); return retval; } @@ -445,30 +453,18 @@ void Http2Session::SubmitPriority(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); Local context = env->context(); - nghttp2_priority_spec spec; int32_t id = args[0]->Int32Value(context).ToChecked(); - int32_t parent = args[1]->Int32Value(context).ToChecked(); - int32_t weight = args[2]->Int32Value(context).ToChecked(); - bool exclusive = args[3]->BooleanValue(context).ToChecked(); + Http2Priority priority(env, args[1], args[2], args[3]); bool silent = args[4]->BooleanValue(context).ToChecked(); - DEBUG_HTTP2("Http2Session: submitting priority for stream %d: " - "parent: %d, weight: %d, exclusive: %d, silent: %d\n", - id, parent, weight, exclusive, silent); - -#if defined(DEBUG) && DEBUG - CHECK_GT(id, 0); - CHECK_GE(parent, 0); - CHECK_GE(weight, 0); -#endif + DEBUG_HTTP2("Http2Session: submitting priority for stream %d", id); Nghttp2Stream* stream; if (!(stream = session->FindStream(id))) { // invalid stream return args.GetReturnValue().Set(NGHTTP2_ERR_INVALID_STREAM_ID); } - nghttp2_priority_spec_init(&spec, parent, weight, exclusive ? 1 : 0); - args.GetReturnValue().Set(stream->SubmitPriority(&spec, silent)); + args.GetReturnValue().Set(stream->SubmitPriority(*priority, silent)); } void Http2Session::SubmitSettings(const FunctionCallbackInfo& args) { @@ -524,20 +520,14 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo& args) { Local headers = args[0].As(); int options = args[1]->IntegerValue(context).ToChecked(); - int32_t parent = args[2]->Int32Value(context).ToChecked(); - int32_t weight = args[3]->Int32Value(context).ToChecked(); - bool exclusive = args[4]->BooleanValue(context).ToChecked(); - - DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, " - "parent: %d, weight: %d, exclusive: %d\n", headers->Length(), - options, parent, weight, exclusive); + Http2Priority priority(env, args[2], args[3], args[4]); - nghttp2_priority_spec prispec; - nghttp2_priority_spec_init(&prispec, parent, weight, exclusive ? 1 : 0); + DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d\n", + headers->Length(), options); Headers list(isolate, context, headers); - int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec, + int32_t ret = session->Nghttp2Session::SubmitRequest(*priority, *list, list.length(), nullptr, options); DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret); diff --git a/src/node_http2.h b/src/node_http2.h index 84cffc613eff64..770bf28075e5db 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -369,6 +369,20 @@ class Http2Settings { MaybeStackBuffer entries_; }; +class Http2Priority { + public: + Http2Priority(Environment* env, + Local parent, + Local weight, + Local exclusive); + + nghttp2_priority_spec* operator*() { + return &spec; + } + private: + nghttp2_priority_spec spec; +}; + class Http2Session : public AsyncWrap, public StreamBase, public Nghttp2Session { diff --git a/src/node_http2_core-inl.h b/src/node_http2_core-inl.h index fa365fee059b4d..5fc0b629ccb205 100644 --- a/src/node_http2_core-inl.h +++ b/src/node_http2_core-inl.h @@ -28,6 +28,13 @@ inline int Nghttp2Session::OnNghttpError(nghttp2_session* session, } #endif +inline int32_t GetFrameID(const nghttp2_frame* frame) { + // If this is a push promise, we want to grab the id of the promised stream + return (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? + frame->push_promise.promised_stream_id : + frame->hd.stream_id; +} + // nghttp2 calls this at the beginning a new HEADERS or PUSH_PROMISE frame. // We use it to ensure that an Nghttp2Stream instance is allocated to store // the state. @@ -35,11 +42,7 @@ inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session, const nghttp2_frame* frame, void* user_data) { Nghttp2Session* handle = static_cast(user_data); - // If this is a push promise frame, we want to grab the handle of - // the promised stream. - int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? - frame->push_promise.promised_stream_id : - frame->hd.stream_id; + int32_t id = GetFrameID(frame); DEBUG_HTTP2("Nghttp2Session %s: beginning headers for stream %d\n", handle->TypeName(), id); @@ -62,11 +65,7 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session, uint8_t flags, void* user_data) { Nghttp2Session* handle = static_cast(user_data); - // If this is a push promise frame, we want to grab the handle of - // the promised stream. - int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? - frame->push_promise.promised_stream_id : - frame->hd.stream_id; + int32_t id = GetFrameID(frame); Nghttp2Stream* stream = handle->FindStream(id); // The header name and value are stored in a reference counted buffer // provided to us by nghttp2. We need to increment the reference counter @@ -418,7 +417,7 @@ inline void Nghttp2Stream::FlushDataChunks() { // see if the END_STREAM flag is set, and will flush the queued data chunks // to JS if the stream is flowing inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) { - int32_t id = frame->hd.stream_id; + int32_t id = GetFrameID(frame); DEBUG_HTTP2("Nghttp2Session %s: handling data frame for stream %d\n", TypeName(), id); Nghttp2Stream* stream = this->FindStream(id); @@ -436,8 +435,7 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) { // The headers are collected as the frame is being processed and sent out // to the JS side only when the frame is fully processed. inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) { - int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ? - frame->push_promise.promised_stream_id : frame->hd.stream_id; + int32_t id = GetFrameID(frame); DEBUG_HTTP2("Nghttp2Session %s: handling headers frame for stream %d\n", TypeName(), id); Nghttp2Stream* stream = FindStream(id); @@ -454,7 +452,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) { // Notifies the JS layer that a PRIORITY frame has been received inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) { nghttp2_priority priority_frame = frame->priority; - int32_t id = frame->hd.stream_id; + int32_t id = GetFrameID(frame); DEBUG_HTTP2("Nghttp2Session %s: handling priority frame for stream %d\n", TypeName(), id); @@ -548,39 +546,22 @@ inline int Nghttp2Session::Init(const nghttp2_session_type type, session_type_ = type; DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName()); destroying_ = false; - int ret = 0; nghttp2_session_callbacks* callbacks = callback_struct_saved[HasGetPaddingCallback() ? 1 : 0].callbacks; - nghttp2_option* opts; - if (options != nullptr) { - opts = options; - } else { - nghttp2_option_new(&opts); - } + CHECK_NE(options, nullptr); - switch (type) { - case NGHTTP2_SESSION_SERVER: - ret = nghttp2_session_server_new3(&session_, - callbacks, - this, - opts, - mem); - break; - case NGHTTP2_SESSION_CLIENT: - ret = nghttp2_session_client_new3(&session_, - callbacks, - this, - opts, - mem); - break; - } - if (opts != options) { - nghttp2_option_del(opts); - } + typedef int (*init_fn)(nghttp2_session** session, + const nghttp2_session_callbacks* callbacks, + void* user_data, + const nghttp2_option* options, + nghttp2_mem* mem); + init_fn fn = type == NGHTTP2_SESSION_SERVER ? + nghttp2_session_server_new3 : + nghttp2_session_client_new3; - return ret; + return fn(&session_, callbacks, this, options, mem); } inline void Nghttp2Session::MarkDestroying() {