From 69dafa1ae4c54f40d6959857b08a7f304ac3d41d Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Wed, 13 Sep 2023 13:42:25 +0200 Subject: [PATCH] Fix HTTP 1xx handling (#247) * fix: Correctly handle 1xx cases. * fix: Invoke missing callback. --- src/llhttp/http.ts | 58 +++++++++++++++++++++---------------- src/native/http.c | 7 ++++- test/request/invalid.md | 1 + test/response/connection.md | 55 +++++++++++++++++++++++++++++++++++ test/response/invalid.md | 2 ++ test/response/sample.md | 1 + 6 files changed, 98 insertions(+), 26 deletions(-) diff --git a/src/llhttp/http.ts b/src/llhttp/http.ts index 83c6df80..42073262 100644 --- a/src/llhttp/http.ts +++ b/src/llhttp/http.ts @@ -538,7 +538,7 @@ export class HTTP { 1: this.testFlags(FLAGS.TRAILING, { 1: this.invokePausable('on_chunk_complete', ERROR.CB_CHUNK_COMPLETE, 'message_done'), - }).otherwise(n('headers_done')), + }).otherwise(this.headersCompleted()), }, onInvalidHeaderFieldChar), ) .otherwise(span.headerField.start(n('header_field'))); @@ -833,13 +833,10 @@ export class HTTP { }, span.headerValue.start(n('header_value_start')))) .otherwise(this.setHeaderFlags(onHeaderValueComplete)); - // Set `upgrade` if needed - const beforeHeadersComplete = p.invoke(callback.beforeHeadersComplete); - const checkTrailing = this.testFlags(FLAGS.TRAILING, { 1: this.invokePausable('on_chunk_complete', ERROR.CB_CHUNK_COMPLETE, 'message_done'), - }).otherwise(beforeHeadersComplete); + }).otherwise(this.headersCompleted()); n('headers_almost_done') .match('\n', checkTrailing) @@ -848,26 +845,6 @@ export class HTTP { 1: checkTrailing, }, p.error(ERROR.STRICT, 'Expected LF after headers'))); - /* Here we call the headers_complete callback. This is somewhat - * different than other callbacks because if the user returns 1, we - * will interpret that as saying that this message has no body. This - * is needed for the annoying case of receiving a response to a HEAD - * request. - * - * We'd like to use CALLBACK_NOTIFY_NOADVANCE() here but we cannot, so - * we have to simulate it by handling a change in errno below. - */ - const onHeadersComplete = p.invoke(callback.onHeadersComplete, { - 0: n('headers_done'), - 1: this.setFlag(FLAGS.SKIPBODY, 'headers_done'), - 2: this.update('upgrade', 1, - this.setFlag(FLAGS.SKIPBODY, 'headers_done')), - [ERROR.PAUSED]: this.pause('Paused by on_headers_complete', - 'headers_done'), - }, p.error(ERROR.CB_HEADERS_COMPLETE, 'User callback error')); - - beforeHeadersComplete.otherwise(onHeadersComplete); - const upgradePause = p.pause(ERROR.PAUSED_UPGRADE, 'Pause on CONNECT/Upgrade'); @@ -1106,6 +1083,37 @@ export class HTTP { )); } + private headersCompleted(): Node { + const p = this.llparse; + const callback = this.callback; + const n = (name: string): Match => this.node(name); + + // Set `upgrade` if needed + const beforeHeadersComplete = p.invoke(callback.beforeHeadersComplete); + + /* Here we call the headers_complete callback. This is somewhat + * different than other callbacks because if the user returns 1, we + * will interpret that as saying that this message has no body. This + * is needed for the annoying case of receiving a response to a HEAD + * request. + * + * We'd like to use CALLBACK_NOTIFY_NOADVANCE() here but we cannot, so + * we have to simulate it by handling a change in errno below. + */ + const onHeadersComplete = p.invoke(callback.onHeadersComplete, { + 0: n('headers_done'), + 1: this.setFlag(FLAGS.SKIPBODY, 'headers_done'), + 2: this.update('upgrade', 1, + this.setFlag(FLAGS.SKIPBODY, 'headers_done')), + [ERROR.PAUSED]: this.pause('Paused by on_headers_complete', + 'headers_done'), + }, p.error(ERROR.CB_HEADERS_COMPLETE, 'User callback error')); + + beforeHeadersComplete.otherwise(onHeadersComplete); + + return beforeHeadersComplete; + } + private node(name: string | T): T { if (name instanceof Node) { return name; diff --git a/src/native/http.c b/src/native/http.c index 99eefb47..e44f36e3 100644 --- a/src/native/http.c +++ b/src/native/http.c @@ -43,7 +43,10 @@ int llhttp__after_headers_complete(llhttp_t* parser, const char* p, (parser->upgrade && (parser->method == HTTP_CONNECT || (parser->flags & F_SKIPBODY) || !hasBody)) || /* See RFC 2616 section 4.4 - 1xx e.g. Continue */ - (parser->type == HTTP_RESPONSE && parser->status_code / 100 == 1) + ( + parser->type == HTTP_RESPONSE && + (parser->status_code == 100 || parser->status_code == 101) + ) ) { /* Exit, the rest of the message is in a different protocol. */ return 1; @@ -54,6 +57,8 @@ int llhttp__after_headers_complete(llhttp_t* parser, const char* p, parser->flags & F_SKIPBODY || /* response to a HEAD request */ ( parser->type == HTTP_RESPONSE && ( + parser->status_code == 102 || /* Processing */ + parser->status_code == 103 || /* Early Hints */ parser->status_code == 204 || /* No Content */ parser->status_code == 304 /* Not Modified */ ) diff --git a/test/request/invalid.md b/test/request/invalid.md index bcc41b48..8eadacf8 100644 --- a/test/request/invalid.md +++ b/test/request/invalid.md @@ -553,6 +553,7 @@ off=66 len=3 span[header_field]="Bar" off=70 header_field complete off=71 len=3 span[header_value]="def" off=75 header_value complete +off=76 headers complete method=3 v=1/1 flags=208 content_length=0 off=78 chunk header len=1 off=78 len=1 span[body]="A" off=80 chunk complete diff --git a/test/response/connection.md b/test/response/connection.md index 6294ab9e..21506493 100644 --- a/test/response/connection.md +++ b/test/response/connection.md @@ -540,3 +540,58 @@ off=107 len=5 span[body]="hello" off=114 chunk complete off=117 chunk header len=0 ``` + + +## HTTP 103 first, then 200 + + +```http +HTTP/1.1 103 Early Hints +Link: ; rel=preload; as=style + +HTTP/1.1 200 OK +Date: Wed, 13 Sep 2023 11:09:41 GMT +Connection: keep-alive +Keep-Alive: timeout=5 +Content-Length: 17 + +response content +``` + +```log +off=0 message begin +off=5 len=3 span[version]="1.1" +off=8 version complete +off=13 len=11 span[status]="Early Hints" +off=26 status complete +off=26 len=4 span[header_field]="Link" +off=31 header_field complete +off=32 len=36 span[header_value]="; rel=preload; as=style" +off=70 header_value complete +off=72 headers complete status=103 v=1/1 flags=0 content_length=0 +off=72 message complete +off=72 reset +off=72 message begin +off=77 len=3 span[version]="1.1" +off=80 version complete +off=85 len=2 span[status]="OK" +off=89 status complete +off=89 len=4 span[header_field]="Date" +off=94 header_field complete +off=95 len=29 span[header_value]="Wed, 13 Sep 2023 11:09:41 GMT" +off=126 header_value complete +off=126 len=10 span[header_field]="Connection" +off=137 header_field complete +off=138 len=10 span[header_value]="keep-alive" +off=150 header_value complete +off=150 len=10 span[header_field]="Keep-Alive" +off=161 header_field complete +off=162 len=9 span[header_value]="timeout=5" +off=173 header_value complete +off=173 len=14 span[header_field]="Content-Length" +off=188 header_field complete +off=189 len=2 span[header_value]="17" +off=193 header_value complete +off=195 headers complete status=200 v=1/1 flags=21 content_length=17 +off=195 len=16 span[body]="response content" +``` \ No newline at end of file diff --git a/test/response/invalid.md b/test/response/invalid.md index 5fba2555..034fc4d9 100644 --- a/test/response/invalid.md +++ b/test/response/invalid.md @@ -230,6 +230,7 @@ off=16 len=14 span[header_field]="Content-Length" off=31 header_field complete off=32 len=1 span[header_value]="0" off=34 header_value complete +off=35 headers complete status=200 v=1/1 flags=20 content_length=0 off=35 message complete ``` @@ -277,6 +278,7 @@ off=25 len=3 span[header_field]="Bar" off=29 header_field complete off=30 len=3 span[header_value]="def" off=34 header_value complete +off=35 headers complete status=200 v=1/1 flags=0 content_length=0 off=35 len=4 span[body]="BODY" off=39 len=1 span[body]=lf off=40 len=1 span[body]="\" diff --git a/test/response/sample.md b/test/response/sample.md index 0fd3c1da..be2e82dc 100644 --- a/test/response/sample.md +++ b/test/response/sample.md @@ -315,6 +315,7 @@ off=55 len=10 span[header_field]="Connection" off=66 header_field complete off=67 len=5 span[header_value]="close" off=73 header_value complete +off=74 headers complete status=200 v=1/1 flags=2 content_length=0 off=74 len=51 span[body]="these headers are from http://news.ycombinator.com/" ```