Skip to content

Commit 04e0620

Browse files
indutnyrvagg
authored andcommitted
http: fix header limit errors and test for llhttp
Ref: nodejs-private/node-private#143 PR-URL: nodejs-private/node-private#149 Reviewed-By: Rod Vagg <rod@vagg.org>
1 parent a2b8aba commit 04e0620

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

src/node_http_parser.cc

+21-11
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
#include "v8.h"
3131

3232
#include <stdlib.h> // free()
33-
#include <string.h> // strdup()
33+
#include <string.h> // strdup(), strchr()
3434

3535
#include "http_parser_adaptor.h"
3636

@@ -367,7 +367,7 @@ class Parser : public AsyncWrap, public StreamListener {
367367
if (r.IsEmpty()) {
368368
got_exception_ = true;
369369
#ifdef NODE_EXPERIMENTAL_HTTP
370-
llhttp_set_error_reason(&parser_, "JS Exception");
370+
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
371371
#endif /* NODE_EXPERIMENTAL_HTTP */
372372
return HPE_USER;
373373
}
@@ -395,7 +395,7 @@ class Parser : public AsyncWrap, public StreamListener {
395395

396396
if (r.IsEmpty()) {
397397
got_exception_ = true;
398-
return HPE_USER;
398+
return -1;
399399
}
400400

401401
return 0;
@@ -712,13 +712,23 @@ class Parser : public AsyncWrap, public StreamListener {
712712
env()->bytes_parsed_string(),
713713
nread_obj).FromJust();
714714
#ifdef NODE_EXPERIMENTAL_HTTP
715-
obj->Set(env()->context(),
716-
env()->code_string(),
717-
OneByteString(env()->isolate(),
718-
llhttp_errno_name(err))).FromJust();
719-
obj->Set(env()->context(),
720-
env()->reason_string(),
721-
OneByteString(env()->isolate(), parser_.reason)).FromJust();
715+
const char* errno_reason = llhttp_get_error_reason(&parser_);
716+
717+
Local<String> code;
718+
Local<String> reason;
719+
if (err == HPE_USER) {
720+
const char* colon = strchr(errno_reason, ':');
721+
CHECK_NE(colon, nullptr);
722+
code = OneByteString(env()->isolate(), errno_reason,
723+
colon - errno_reason);
724+
reason = OneByteString(env()->isolate(), colon + 1);
725+
} else {
726+
code = OneByteString(env()->isolate(), llhttp_errno_name(err));
727+
reason = OneByteString(env()->isolate(), errno_reason);
728+
}
729+
730+
obj->Set(env()->context(), env()->code_string(), code).FromJust();
731+
obj->Set(env()->context(), env()->reason_string(), reason).FromJust();
722732
#else /* !NODE_EXPERIMENTAL_HTTP */
723733
obj->Set(env()->context(),
724734
env()->code_string(),
@@ -790,7 +800,7 @@ class Parser : public AsyncWrap, public StreamListener {
790800
#ifdef NODE_EXPERIMENTAL_HTTP
791801
header_nread_ += len;
792802
if (header_nread_ >= kMaxHeaderSize) {
793-
llhttp_set_error_reason(&parser_, "Headers overflow");
803+
llhttp_set_error_reason(&parser_, "HPE_HEADER_OVERFLOW:Header overflow");
794804
return HPE_USER;
795805
}
796806
#endif /* NODE_EXPERIMENTAL_HTTP */

test/sequential/test-http-max-http-headers.js

+9
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ function finished(client, callback) {
2525
}
2626

2727
function fillHeaders(headers, currentSize, valid = false) {
28+
// llhttp counts actual header name/value sizes, excluding the whitespace and
29+
// stripped chars.
30+
if (process.versions.hasOwnProperty('llhttp')) {
31+
// OK, Content-Length, 0, X-CRASH, aaa...
32+
headers += 'a'.repeat(MAX - currentSize);
33+
} else {
34+
headers += 'a'.repeat(MAX - headers.length - 3);
35+
}
36+
2837
// Generate valid headers
2938
if (valid) {
3039
// TODO(mcollina): understand why -9 is needed instead of -1

0 commit comments

Comments
 (0)