fix: Do not allow OBS fold in headers by default.

Port nodejs#350 to 2.1.x.
richardlau · Apr 11, 2024 · 6ed8467 · 6ed8467
1 parent ccef2c7
commit 6ed8467
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 14 deletions.
diff --git a/src/llhttp/http.ts b/src/llhttp/http.ts
@@ -598,11 +598,14 @@ export class HTTP {
         'Missing expected LF after header value'));
 
     n('header_value_lws')
-      .peek([ ' ', '\t' ],
-        this.load('header_state', {
-          [HEADER_STATE.TRANSFER_ENCODING_CHUNKED]:
-            this.resetHeaderState(span.headerValue.start(n('header_value_start'))),
-        }, span.headerValue.start(n('header_value_start'))))
+      .peek(
+        [ ' ', '\t' ],
+        this.testFlags(FLAGS.LENIENT, {
+          1: this.load('header_state', {
+            [HEADER_STATE.TRANSFER_ENCODING_CHUNKED]:
+              this.resetHeaderState(span.headerValue.start(n('header_value_start'))),
+          }, span.headerValue.start(n('header_value_start'))),
+        }, p.error(ERROR.INVALID_HEADER_TOKEN, 'Unexpected whitespace after header value')))
       .otherwise(this.setHeaderFlags('header_field_start'));
 
     const checkTrailing = this.testFlags(FLAGS.TRAILING, {

diff --git a/test/fixtures/index.ts b/test/fixtures/index.ts
@@ -9,7 +9,7 @@ import * as path from 'path';
 import * as llhttp from '../../src/llhttp';
 
 export type TestType = 'request' | 'response' | 'request-lenient' |
-  'request-finish' | 'response-finish' | 'none' | 'url';
+  'request-finish' | 'response-lenient-headers' | 'response-finish' | 'none' | 'url';
 
 export { FixtureResult };
 
@@ -58,7 +58,8 @@ export async function build(
   }
 
   const extra = options.extra === undefined ? [] : options.extra.slice();
-  if (ty === 'request' || ty === 'response' || ty === 'request-lenient') {
+  if (ty === 'request' || ty === 'response' || ty === 'request-lenient' ||
+      ty === 'response-lenient-headers') {
     extra.push(
       `-DLLPARSE__TEST_INIT=llhttp__test_init_${ty.replace(/-/g, '_')}`);
   } else if (ty === 'request-finish' || ty === 'response-finish') {

diff --git a/test/md-test.ts b/test/md-test.ts
@@ -82,6 +82,7 @@ const http: IFixtureMap = {
     'request-lenient': buildMode('loose', 'request-lenient'),
     'response': buildMode('loose', 'response'),
     'response-finish': buildMode('loose', 'response-finish'),
+    'response-lenient-headers': buildMode( 'loose', 'response-lenient-headers'),
     'url': buildMode('loose', 'url'),
   },
   strict: {
@@ -91,6 +92,7 @@ const http: IFixtureMap = {
     'request-lenient': buildMode('strict', 'request-lenient'),
     'response': buildMode('strict', 'response'),
     'response-finish': buildMode('strict', 'response-finish'),
+    'response-lenient-headers': buildMode('strict', 'response-lenient-headers'),
     'url': buildMode('strict', 'url'),
   },
 };
@@ -153,6 +155,8 @@ function run(name: string): void {
           types = [ 'response' ];
         } else if (meta.type === 'request-finish') {
           types = [ 'request-finish' ];
+        } else if (meta.type === 'response-lenient-headers') {
+          types = [ 'response-lenient-headers' ];
         } else if (meta.type === 'response-finish') {
           types = [ 'response-finish' ];
         } else {

diff --git a/test/request/connection.md b/test/request/connection.md
@@ -263,7 +263,7 @@ off=75 message complete
 
 ### Multiple tokens with folding
 
-<!-- meta={"type": "request"} -->
+<!-- meta={"type": "request-lenient"} -->
 ```http
 GET /demo HTTP/1.1
 Host: example.com
@@ -326,7 +326,7 @@ off=75 error code=22 reason="Pause on CONNECT/Upgrade"
 
 ### Multiple tokens with folding, LWS, and CRLF
 
-<!-- meta={"type": "request"} -->
+<!-- meta={"type": "request-lenient"} -->
 ```http
 GET /demo HTTP/1.1
 Connection: keep-alive, \r\n upgrade

diff --git a/test/request/invalid.md b/test/request/invalid.md
@@ -222,4 +222,70 @@ off=22 len=9 span[header_value]="localhost"
 off=33 len=5 span[header_field]="Dummy"
 off=40 len=1 span[header_value]="x"
 off=41 error code=10 reason="Invalid header value char"
-```
+```
+
+### Spaces before headers
+
+<!-- meta={ "type": "request" } -->
+
+```http
+POST /hello HTTP/1.1
+Host: localhost
+Foo: bar
+ Content-Length: 38
+GET /bye HTTP/1.1
+Host: localhost
+```
+
+```log
+off=0 message begin
+off=5 len=6 span[url]="/hello"
+off=12 url complete
+off=22 len=4 span[header_field]="Host"
+off=27 header_field complete
+off=28 len=9 span[header_value]="localhost"
+off=39 header_value complete
+off=39 len=3 span[header_field]="Foo"
+off=43 header_field complete
+off=44 len=3 span[header_value]="bar"
+off=49 error code=10 reason="Unexpected whitespace after header value"
+```
+
+### Spaces before headers (lenient)
+
+<!-- meta={ "type": "request-lenient" } -->
+
+```http
+POST /hello HTTP/1.1
+Host: localhost
+Foo: bar
+ Content-Length: 38
+GET /bye HTTP/1.1
+Host: localhost
+```
+
+```log
+off=0 message begin
+off=5 len=6 span[url]="/hello"
+off=12 url complete
+off=22 len=4 span[header_field]="Host"
+off=27 header_field complete
+off=28 len=9 span[header_value]="localhost"
+off=39 header_value complete
+off=39 len=3 span[header_field]="Foo"
+off=43 header_field complete
+off=44 len=3 span[header_value]="bar"
+off=49 len=19 span[header_value]=" Content-Length: 38"
+off=70 header_value complete
+off=72 headers complete method=3 v=1/1 flags=0 content_length=0
+off=72 message complete
+off=72 message begin
+off=76 len=4 span[url]="/bye"
+off=81 url complete
+off=91 len=4 span[header_field]="Host"
+off=96 header_field complete
+off=97 len=9 span[header_value]="localhost"
+off=108 header_value complete
+off=110 headers complete method=1 v=1/1 flags=0 content_length=0
+off=110 message complete
+```
diff --git a/test/request/sample.md b/test/request/sample.md
@@ -369,7 +369,7 @@ off=61 message complete
 
 See nodejs/test/parallel/test-http-headers-obstext.js
 
-<!-- meta={"type": "request"} -->
+<!-- meta={"type": "request-lenient"} -->
 ```http
 GET / HTTP/1.1
 X-SSL-Nonsense:   -----BEGIN CERTIFICATE-----

diff --git a/test/request/transfer-encoding.md b/test/request/transfer-encoding.md
@@ -518,7 +518,7 @@ off=50 error code=12 reason="Invalid character in chunk size"
 
 ## Invalid OBS fold after chunked value
 
-<!-- meta={"type": "request", "mode": "strict"} -->
+<!-- meta={"type": "request-lenient", "mode": "strict"} -->
 ```http
 PUT /url HTTP/1.1
 Transfer-Encoding: chunked

diff --git a/test/response/transfer-encoding.md b/test/response/transfer-encoding.md
@@ -118,7 +118,7 @@ off=78 len=1 span[body]=lf
 
 ## Invalid OBS fold after chunked value
 
-<!-- meta={"type": "response" } -->
+<!-- meta={"type": "response-lenient-headers"} -->
 ```http
 HTTP/1.1 200 OK
 Transfer-Encoding: chunked

diff --git a/tsconfig.json b/tsconfig.json
@@ -7,7 +7,8 @@
     "outDir": "./lib",
     "declaration": true,
     "pretty": true,
-    "sourceMap": true
+    "sourceMap": true,
+    "skipLibCheck": true
   },
   "include": [
     "src/**/*.ts"