Skip to content

Commit

Permalink
http2: ignore invalid headers explicitly
Browse files Browse the repository at this point in the history
Required in order for `parallel/test-http2-response-splitting` to pass
after upgrading `nghttp2`.
  • Loading branch information
addaleax committed Aug 20, 2017
1 parent 7c836e0 commit 09531c0
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/node_http2_core-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ inline int Nghttp2Session::OnFrameNotSent(nghttp2_session *session,
return 0;
}

inline int Nghttp2Session::OnInvalidHeader(nghttp2_session* session,
const nghttp2_frame* frame,
nghttp2_rcbuf* name,
nghttp2_rcbuf* value,
uint8_t flags,
void* user_data) {
// Ignore invalid header fields by default.
return 0;
}

// Called when nghttp2 closes a stream, either in response to an RST_STREAM
// frame or the stream closing naturally on it's own
inline int Nghttp2Session::OnStreamClose(nghttp2_session *session,
Expand Down Expand Up @@ -910,6 +920,8 @@ Nghttp2Session::Callbacks::Callbacks(bool kHasGetPaddingCallback) {
callbacks, OnDataChunkReceived);
nghttp2_session_callbacks_set_on_frame_not_send_callback(
callbacks, OnFrameNotSent);
nghttp2_session_callbacks_set_on_invalid_header_callback2(
callbacks, OnInvalidHeader);

#ifdef NODE_DEBUG_HTTP2
nghttp2_session_callbacks_set_error_callback(
Expand Down
6 changes: 6 additions & 0 deletions src/node_http2_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,12 @@ class Nghttp2Session {
int32_t id,
uint32_t code,
void* user_data);
static inline int OnInvalidHeader(nghttp2_session* session,
const nghttp2_frame* frame,
nghttp2_rcbuf* name,
nghttp2_rcbuf* value,
uint8_t flags,
void* user_data);
static inline int OnDataChunkReceived(nghttp2_session* session,
uint8_t flags,
int32_t id,
Expand Down

3 comments on commit 09531c0

@jasnell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the change in behavior we should decide if we should also change and treat invalid headers the way nghttp2 does but default

@addaleax
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. In that case I’d be glad if you could take a look at how to change the test best? :)

@jasnell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

Please sign in to comment.