Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: add specific error code for custom frames #37936

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/errors.md
Expand Up @@ -1364,6 +1364,15 @@ When setting the priority for an HTTP/2 stream, the stream may be marked as
a dependency for a parent stream. This error code is used when an attempt is
made to mark a stream and dependent of itself.

<a id="ERR_HTTP2_TOO_MANY_INVALID_FRAMES"></a>
### `ERR_HTTP2_TOO_MANY_INVALID_FRAMES`
<!--
added: REPLACEME
-->

The limit of acceptable invalid HTTP/2 protocol frames sent by the peer,
as specified through the `maxSessionInvalidFrames` option, has been exceeded.

<a id="ERR_HTTP2_TRAILERS_ALREADY_SENT"></a>
### `ERR_HTTP2_TRAILERS_ALREADY_SENT`

Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Expand Up @@ -992,6 +992,7 @@ E('ERR_HTTP2_STREAM_CANCEL', function(error) {
E('ERR_HTTP2_STREAM_ERROR', 'Stream closed with error code %s', Error);
E('ERR_HTTP2_STREAM_SELF_DEPENDENCY',
'A stream cannot depend on itself', Error);
E('ERR_HTTP2_TOO_MANY_INVALID_FRAMES', 'Too many invalid HTTP/2 frames', Error);
E('ERR_HTTP2_TRAILERS_ALREADY_SENT',
'Trailing headers have already been sent', Error);
E('ERR_HTTP2_TRAILERS_NOT_READY',
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/http2/core.js
Expand Up @@ -778,9 +778,9 @@ const setAndValidatePriorityOptions = hideStackFrames((options) => {

// When an error occurs internally at the binding level, immediately
// destroy the session.
function onSessionInternalError(code) {
function onSessionInternalError(integerCode, customErrorCode) {
if (this[kOwner] !== undefined)
this[kOwner].destroy(new NghttpError(code));
this[kOwner].destroy(new NghttpError(integerCode, customErrorCode));
}

function settingsCallback(cb, ack, duration) {
Expand Down
13 changes: 8 additions & 5 deletions lib/internal/http2/util.js
Expand Up @@ -29,6 +29,7 @@ const {
ERR_INVALID_HTTP_TOKEN
},
addCodeToName,
getMessage,
hideStackFrames
} = require('internal/errors');

Expand Down Expand Up @@ -543,11 +544,13 @@ function mapToHeaders(map,
}

class NghttpError extends Error {
constructor(ret) {
super(binding.nghttp2ErrorString(ret));
this.code = 'ERR_HTTP2_ERROR';
this.errno = ret;
addCodeToName(this, super.name, 'ERR_HTTP2_ERROR');
constructor(integerCode, customErrorCode) {
super(customErrorCode ?
getMessage(customErrorCode, [], null) :
binding.nghttp2ErrorString(integerCode));
this.code = customErrorCode || 'ERR_HTTP2_ERROR';
this.errno = integerCode;
addCodeToName(this, super.name, this.code);
}

toString() {
Expand Down
58 changes: 38 additions & 20 deletions src/node_http2.cc
Expand Up @@ -32,6 +32,7 @@ using v8::Integer;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::NewStringType;
using v8::Number;
using v8::Object;
using v8::ObjectTemplate;
Expand Down Expand Up @@ -782,7 +783,7 @@ ssize_t Http2Session::OnMaxFrameSizePadding(size_t frameLen,
// various callback functions. Each of these will typically result in a call
// out to JavaScript so this particular function is rather hot and can be
// quite expensive. This is a potential performance optimization target later.
ssize_t Http2Session::ConsumeHTTP2Data() {
void Http2Session::ConsumeHTTP2Data() {
CHECK_NOT_NULL(stream_buf_.base);
CHECK_LE(stream_buf_offset_, stream_buf_.len);
size_t read_len = stream_buf_.len - stream_buf_offset_;
Expand All @@ -792,12 +793,14 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
read_len,
nghttp2_session_want_read(session_.get()));
set_receive_paused(false);
custom_recv_error_code_ = nullptr;
ssize_t ret =
nghttp2_session_mem_recv(session_.get(),
reinterpret_cast<uint8_t*>(stream_buf_.base) +
stream_buf_offset_,
read_len);
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
CHECK_IMPLIES(custom_recv_error_code_ != nullptr, ret < 0);

if (is_receive_paused()) {
CHECK(is_reading_stopped());
Expand All @@ -809,7 +812,7 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
// Even if all bytes were received, a paused stream may delay the
// nghttp2_on_frame_recv_callback which may have an END_STREAM flag.
stream_buf_offset_ += ret;
return ret;
goto done;
}

// We are done processing the current input chunk.
Expand All @@ -819,14 +822,34 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
stream_buf_allocation_.clear();
stream_buf_ = uv_buf_init(nullptr, 0);

if (ret < 0)
return ret;

// Send any data that was queued up while processing the received data.
if (!is_destroyed()) {
if (ret >= 0 && !is_destroyed()) {
SendPendingData();
}
return ret;

done:
if (UNLIKELY(ret < 0)) {
Isolate* isolate = env()->isolate();
Debug(this,
"fatal error receiving data: %d (%s)",
ret,
custom_recv_error_code_ != nullptr ?
custom_recv_error_code_ : "(no custom error code)");
Local<Value> args[] = {
Integer::New(isolate, static_cast<int32_t>(ret)),
Null(isolate)
};
if (custom_recv_error_code_ != nullptr) {
args[1] = String::NewFromUtf8(
isolate,
custom_recv_error_code_,
NewStringType::kInternalized).ToLocalChecked();
}
MakeCallback(
env()->http2session_on_error_function(),
arraysize(args),
args);
}
}


Expand Down Expand Up @@ -950,14 +973,17 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
int lib_error_code,
void* user_data) {
Http2Session* session = static_cast<Http2Session*>(user_data);
const uint32_t max_invalid_frames = session->js_fields_->max_invalid_frames;

Debug(session,
"invalid frame received (%u/%u), code: %d",
session->invalid_frame_count_,
session->js_fields_->max_invalid_frames,
max_invalid_frames,
lib_error_code);
if (session->invalid_frame_count_++ > session->js_fields_->max_invalid_frames)
if (session->invalid_frame_count_++ > max_invalid_frames) {
session->custom_recv_error_code_ = "ERR_HTTP2_TOO_MANY_INVALID_FRAMES";
return 1;
}

// If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error
if (nghttp2_is_fatal(lib_error_code) ||
Expand Down Expand Up @@ -1336,6 +1362,7 @@ int Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
stream->EmitRead(UV_EOF);
} else if (frame->hd.length == 0) {
if (invalid_frame_count_++ > js_fields_->max_invalid_frames) {
custom_recv_error_code_ = "ERR_HTTP2_TOO_MANY_INVALID_FRAMES";
Debug(this, "rejecting empty-frame-without-END_STREAM flood\n");
// Consider a flood of 0-length frames without END_STREAM an error.
return 1;
Expand Down Expand Up @@ -1520,7 +1547,7 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
ConsumeHTTP2Data();
}

if (!is_write_scheduled()) {
if (!is_write_scheduled() && !is_destroyed()) {
// Schedule a new write if nghttp2 wants to send data.
MaybeScheduleWrite();
}
Expand Down Expand Up @@ -1848,21 +1875,12 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
// offset of a DATA frame's data into the socket read buffer.
stream_buf_ = uv_buf_init(buf.data(), static_cast<unsigned int>(nread));

Isolate* isolate = env()->isolate();

// Store this so we can create an ArrayBuffer for read data from it.
// DATA frames will be emitted as slices of that ArrayBuffer to avoid having
// to copy memory.
stream_buf_allocation_ = std::move(buf);

ssize_t ret = ConsumeHTTP2Data();

if (UNLIKELY(ret < 0)) {
Debug(this, "fatal error receiving data: %d", ret);
Local<Value> arg = Integer::New(isolate, static_cast<int32_t>(ret));
MakeCallback(env()->http2session_on_error_function(), 1, &arg);
return;
}
ConsumeHTTP2Data();

MaybeStopReading();
}
Expand Down
8 changes: 6 additions & 2 deletions src/node_http2.h
Expand Up @@ -662,8 +662,9 @@ class Http2Session : public AsyncWrap,
// Indicates whether there currently exist outgoing buffers for this stream.
bool HasWritesOnSocketForStream(Http2Stream* stream);

// Write data from stream_buf_ to the session
ssize_t ConsumeHTTP2Data();
// Write data from stream_buf_ to the session.
// This will call the error callback if an error occurs.
void ConsumeHTTP2Data();

void MemoryInfo(MemoryTracker* tracker) const override;
SET_MEMORY_INFO_NAME(Http2Session)
Expand Down Expand Up @@ -898,6 +899,9 @@ class Http2Session : public AsyncWrap,
v8::Global<v8::ArrayBuffer> stream_buf_ab_;
AllocatedBuffer stream_buf_allocation_;
size_t stream_buf_offset_ = 0;
// Custom error code for errors that originated inside one of the callbacks
// called by nghttp2_session_mem_recv.
const char* custom_recv_error_code_ = nullptr;

size_t max_outstanding_pings_ = kDefaultMaxPings;
std::queue<BaseObjectPtr<Http2Ping>> outstanding_pings_;
Expand Down
8 changes: 6 additions & 2 deletions test/parallel/test-http2-empty-frame-without-eof.js
Expand Up @@ -26,8 +26,12 @@ async function main() {
stream.on('error', common.mustNotCall());
client.on('error', common.mustNotCall());
} else {
stream.on('error', common.mustCall());
client.on('error', common.mustCall());
const expected = {
code: 'ERR_HTTP2_TOO_MANY_INVALID_FRAMES',
message: 'Too many invalid HTTP/2 frames'
};
stream.on('error', common.expectsError(expected));
client.on('error', common.expectsError(expected));
}
stream.resume();
await once(stream, 'end');
Expand Down