Skip to content

Commit 4b9b57d

Browse files
mcollinaShogunPanda
mcollina
and
ShogunPanda
authored
Chunked transfer encoding (#161)
* http: stricter Transfer-Encoding parsing * http: stricter header separator parsing * http: renamed test * http: change otherwise rule * http: allow lenient CRLF header handling * http: remove useless rule * http: remove useless comment * http: fixed tests Co-authored-by: Shogun <paolo@cowtech.it>
1 parent 75b4512 commit 4b9b57d

13 files changed

Lines changed: 326 additions & 93 deletions

File tree

‎src/llhttp/constants.ts‎

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,39 +6,40 @@ export type HTTPMode = 'loose' | 'strict';
66

77
export enum ERROR {
88
OK = 0,
9-
INTERNAL,
10-
STRICT,
11-
LF_EXPECTED,
12-
UNEXPECTED_CONTENT_LENGTH,
13-
CLOSED_CONNECTION,
14-
INVALID_METHOD,
15-
INVALID_URL,
16-
INVALID_CONSTANT,
17-
INVALID_VERSION,
18-
INVALID_HEADER_TOKEN,
19-
INVALID_CONTENT_LENGTH,
20-
INVALID_CHUNK_SIZE,
21-
INVALID_STATUS,
22-
INVALID_EOF_STATE,
23-
INVALID_TRANSFER_ENCODING,
24-
25-
CB_MESSAGE_BEGIN,
26-
CB_HEADERS_COMPLETE,
27-
CB_MESSAGE_COMPLETE,
28-
CB_CHUNK_HEADER,
29-
CB_CHUNK_COMPLETE,
30-
31-
PAUSED,
32-
PAUSED_UPGRADE,
33-
PAUSED_H2_UPGRADE,
34-
35-
USER,
9+
INTERNAL = 1,
10+
STRICT = 2,
11+
CR_EXPECTED = 25,
12+
LF_EXPECTED = 3,
13+
UNEXPECTED_CONTENT_LENGTH = 4,
14+
CLOSED_CONNECTION = 5,
15+
INVALID_METHOD = 6,
16+
INVALID_URL = 7,
17+
INVALID_CONSTANT = 8,
18+
INVALID_VERSION = 9,
19+
INVALID_HEADER_TOKEN = 10,
20+
INVALID_CONTENT_LENGTH = 11,
21+
INVALID_CHUNK_SIZE = 12,
22+
INVALID_STATUS = 13,
23+
INVALID_EOF_STATE = 14,
24+
INVALID_TRANSFER_ENCODING = 15,
25+
26+
CB_MESSAGE_BEGIN = 16,
27+
CB_HEADERS_COMPLETE = 17,
28+
CB_MESSAGE_COMPLETE = 18,
29+
CB_CHUNK_HEADER = 19,
30+
CB_CHUNK_COMPLETE = 20,
31+
32+
PAUSED = 21,
33+
PAUSED_UPGRADE = 22,
34+
PAUSED_H2_UPGRADE = 23,
35+
36+
USER = 24,
3637
}
3738

3839
export enum TYPE {
3940
BOTH = 0, // default
40-
REQUEST,
41-
RESPONSE,
41+
REQUEST = 1,
42+
RESPONSE = 2,
4243
}
4344

4445
export enum FLAGS {
@@ -58,6 +59,7 @@ export enum LENIENT_FLAGS {
5859
HEADERS = 1 << 0,
5960
CHUNKED_LENGTH = 1 << 1,
6061
KEEP_ALIVE = 1 << 2,
62+
TRANSFER_ENCODING = 1 << 3,
6163
}
6264

6365
export enum METHODS {
@@ -194,8 +196,8 @@ Object.keys(METHOD_MAP).forEach((key) => {
194196

195197
export enum FINISH {
196198
SAFE = 0,
197-
SAFE_WITH_CB,
198-
UNSAFE,
199+
SAFE_WITH_CB = 1,
200+
UNSAFE = 2,
199201
}
200202

201203
// Internal
@@ -291,15 +293,15 @@ export const MINOR = MAJOR;
291293

292294
export enum HEADER_STATE {
293295
GENERAL = 0,
294-
CONNECTION,
295-
CONTENT_LENGTH,
296-
TRANSFER_ENCODING,
297-
UPGRADE,
298-
299-
CONNECTION_KEEP_ALIVE,
300-
CONNECTION_CLOSE,
301-
CONNECTION_UPGRADE,
302-
TRANSFER_ENCODING_CHUNKED,
296+
CONNECTION = 1,
297+
CONTENT_LENGTH = 2,
298+
TRANSFER_ENCODING = 3,
299+
UPGRADE = 4,
300+
301+
CONNECTION_KEEP_ALIVE = 5,
302+
CONNECTION_CLOSE = 6,
303+
CONNECTION_UPGRADE = 7,
304+
TRANSFER_ENCODING_CHUNKED = 8,
303305
}
304306

305307
export const SPECIAL_HEADERS = {

‎src/llhttp/http.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const NODES: ReadonlyArray<string> = [
6060
'header_value',
6161
'header_value_otherwise',
6262
'header_value_lenient',
63+
'header_value_lenient_failed',
6364
'header_value_lws',
6465
'header_value_te_chunked',
6566
'header_value_te_chunked_last',
@@ -490,11 +491,27 @@ export class HTTP {
490491
FLAGS.CHUNKED,
491492
'header_value_te_chunked');
492493

494+
// Once chunked has been selected, no other encoding is possible in requests
495+
// https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1
496+
const forbidAfterChunkedInRequest = (otherwise: Node) => {
497+
return this.load('type', {
498+
[TYPE.REQUEST]: this.testLenientFlags(LENIENT_FLAGS.TRANSFER_ENCODING, {
499+
0: span.headerValue.end().skipTo(
500+
p.error(ERROR.INVALID_TRANSFER_ENCODING, 'Invalid `Transfer-Encoding` header value'),
501+
),
502+
}).otherwise(otherwise),
503+
}, otherwise);
504+
};
505+
493506
n('header_value_start')
494507
.otherwise(this.load('header_state', {
495508
[HEADER_STATE.UPGRADE]: this.setFlag(FLAGS.UPGRADE, fallback),
496-
[HEADER_STATE.TRANSFER_ENCODING]: this.setFlag(
497-
FLAGS.TRANSFER_ENCODING, toTransferEncoding),
509+
[HEADER_STATE.TRANSFER_ENCODING]: this.testFlags(
510+
FLAGS.CHUNKED,
511+
{
512+
1: forbidAfterChunkedInRequest(this.setFlag(FLAGS.TRANSFER_ENCODING, toTransferEncoding)),
513+
},
514+
this.setFlag(FLAGS.TRANSFER_ENCODING, toTransferEncoding)),
498515
[HEADER_STATE.CONTENT_LENGTH]: n('header_value_content_length_once'),
499516
[HEADER_STATE.CONNECTION]: n('header_value_connection'),
500517
}, 'header_value'));
@@ -516,7 +533,8 @@ export class HTTP {
516533
.peek([ '\r', '\n' ], this.update('header_state',
517534
HEADER_STATE.TRANSFER_ENCODING_CHUNKED,
518535
'header_value_otherwise'))
519-
.otherwise(n('header_value_te_chunked'));
536+
.peek(',', forbidAfterChunkedInRequest(n('header_value_te_chunked')))
537+
.otherwise(n('header_value_te_token'));
520538

521539
n('header_value_te_token')
522540
.match(',', n('header_value_te_token_ows'))
@@ -601,18 +619,23 @@ export class HTTP {
601619

602620
const checkLenient = this.testLenientFlags(LENIENT_FLAGS.HEADERS, {
603621
1: n('header_value_lenient'),
604-
}, p.error(ERROR.INVALID_HEADER_TOKEN, 'Invalid header value char'));
622+
}, n('header_value_lenient_failed'));
605623

606624
n('header_value_otherwise')
607625
.peek('\r', span.headerValue.end().skipTo(n('header_value_almost_done')))
608-
.peek('\n', span.headerValue.end(n('header_value_almost_done')))
609626
.otherwise(checkLenient);
610627

611628
n('header_value_lenient')
612629
.peek('\r', span.headerValue.end().skipTo(n('header_value_almost_done')))
613630
.peek('\n', span.headerValue.end(n('header_value_almost_done')))
614631
.skipTo(n('header_value_lenient'));
615632

633+
n('header_value_lenient_failed')
634+
.peek('\n', span.headerValue.end().skipTo(
635+
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after header value')),
636+
)
637+
.otherwise(p.error(ERROR.INVALID_HEADER_TOKEN, 'Invalid header value char'));
638+
616639
n('header_value_almost_done')
617640
.match('\n', n('header_value_lws'))
618641
.otherwise(p.error(ERROR.LF_EXPECTED,

‎src/native/api.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,14 @@ void llhttp_set_lenient_keep_alive(llhttp_t* parser, int enabled) {
253253
}
254254
}
255255

256+
void llhttp_set_lenient_transfer_encoding(llhttp_t* parser, int enabled) {
257+
if (enabled) {
258+
parser->lenient_flags |= LENIENT_TRANSFER_ENCODING;
259+
} else {
260+
parser->lenient_flags &= ~LENIENT_TRANSFER_ENCODING;
261+
}
262+
}
263+
256264
/* Callbacks */
257265

258266

‎src/native/api.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,19 @@ void llhttp_set_lenient_chunked_length(llhttp_t* parser, int enabled);
246246
*/
247247
void llhttp_set_lenient_keep_alive(llhttp_t* parser, int enabled);
248248

249+
/* Enables/disables lenient handling of `Transfer-Encoding` header.
250+
*
251+
* Normally `llhttp` would error when a `Transfer-Encoding` has `chunked` value
252+
* and another value after it (either in a single header or in multiple
253+
* headers whose value are internally joined using `, `).
254+
* This is mandated by the spec to reliably determine request body size and thus
255+
* avoid request smuggling.
256+
* With this flag the extra value will be parsed normally.
257+
*
258+
* **(USE AT YOUR OWN RISK)**
259+
*/
260+
void llhttp_set_lenient_transfer_encoding(llhttp_t* parser, int enabled);
261+
249262
#ifdef __cplusplus
250263
} /* extern "C" */
251264
#endif

‎src/native/http.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ int llhttp__after_headers_complete(llhttp_t* parser, const char* p,
5252
return 2;
5353
} else if (parser->flags & F_TRANSFER_ENCODING) {
5454
if (parser->type == HTTP_REQUEST &&
55-
(parser->lenient_flags & LENIENT_CHUNKED_LENGTH) == 0) {
55+
(parser->lenient_flags & LENIENT_CHUNKED_LENGTH) == 0 &&
56+
(parser->lenient_flags & LENIENT_TRANSFER_ENCODING) == 0) {
5657
/* RFC 7230 3.3.3 */
5758

5859
/* If a Transfer-Encoding header field

‎test/fixtures/extra.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ void llhttp__test_init_request_lenient_keep_alive(llparse_t* s) {
8282
s->lenient_flags |= LENIENT_KEEP_ALIVE;
8383
}
8484

85+
void llhttp__test_init_request_lenient_transfer_encoding(llparse_t* s) {
86+
llhttp__test_init_request(s);
87+
s->lenient_flags |= LENIENT_TRANSFER_ENCODING;
88+
}
89+
8590

8691
void llhttp__test_init_response_lenient_keep_alive(llparse_t* s) {
8792
llhttp__test_init_response(s);

‎test/fixtures/index.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ import * as path from 'path';
99
import * as llhttp from '../../src/llhttp';
1010

1111
export type TestType = 'request' | 'response' | 'request-lenient-headers' |
12-
'request-lenient-chunked-length' | 'request-lenient-keep-alive' |
13-
'response-lenient-keep-alive' | 'request-finish' | 'response-finish' |
12+
'request-lenient-chunked-length' | 'request-lenient-transfer-encoding' |
13+
'request-lenient-keep-alive' | 'response-lenient-keep-alive' |
14+
'request-finish' | 'response-finish' |
1415
'none' | 'url';
1516

1617
export { FixtureResult };
@@ -63,6 +64,7 @@ export async function build(
6364
if (ty === 'request' || ty === 'response' ||
6465
ty === 'request-lenient-headers' ||
6566
ty === 'request-lenient-chunked-length' ||
67+
ty === 'request-lenient-transfer-encoding' ||
6668
ty === 'request-lenient-keep-alive' ||
6769
ty === 'response-lenient-keep-alive') {
6870
extra.push(

‎test/md-test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ const http: IFixtureMap = {
8484
'request-lenient-headers': buildMode('loose', 'request-lenient-headers'),
8585
'request-lenient-keep-alive': buildMode(
8686
'loose', 'request-lenient-keep-alive'),
87+
'request-lenient-transfer-encoding':
88+
buildMode('loose', 'request-lenient-transfer-encoding'),
8789
'response': buildMode('loose', 'response'),
8890
'response-finish': buildMode('loose', 'response-finish'),
8991
'response-lenient-keep-alive': buildMode(
@@ -99,6 +101,8 @@ const http: IFixtureMap = {
99101
'request-lenient-headers': buildMode('strict', 'request-lenient-headers'),
100102
'request-lenient-keep-alive': buildMode(
101103
'strict', 'request-lenient-keep-alive'),
104+
'request-lenient-transfer-encoding':
105+
buildMode('strict', 'request-lenient-transfer-encoding'),
102106
'response': buildMode('strict', 'response'),
103107
'response-finish': buildMode('strict', 'response-finish'),
104108
'response-lenient-keep-alive': buildMode(
@@ -165,6 +169,8 @@ function run(name: string): void {
165169
types = [ 'request-lenient-chunked-length' ];
166170
} else if (meta.type === 'request-lenient-keep-alive') {
167171
types = [ 'request-lenient-keep-alive' ];
172+
} else if (meta.type === 'request-lenient-transfer-encoding') {
173+
types = [ 'request-lenient-transfer-encoding' ];
168174
} else if (meta.type === 'response-lenient-keep-alive') {
169175
types = [ 'response-lenient-keep-alive' ];
170176
} else if (meta.type === 'response-only') {

‎test/request/invalid.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,33 @@ off=22 len=15 span[header_value]="www.example.com"
200200
off=39 header_value complete
201201
off=52 error code=10 reason="Invalid header token"
202202
```
203+
204+
### Missing CR between headers
205+
206+
<!-- meta={"type": "request", "noScan": true} -->
207+
208+
```http
209+
GET / HTTP/1.1
210+
Host: localhost
211+
Dummy: x\nContent-Length: 23
212+
213+
GET / HTTP/1.1
214+
Dummy: GET /admin HTTP/1.1
215+
Host: localhost
216+
217+
218+
```
219+
220+
```log
221+
off=0 message begin
222+
off=4 len=1 span[url]="/"
223+
off=6 url complete
224+
off=16 len=4 span[header_field]="Host"
225+
off=21 header_field complete
226+
off=22 len=9 span[header_value]="localhost"
227+
off=33 header_value complete
228+
off=33 len=5 span[header_field]="Dummy"
229+
off=39 header_field complete
230+
off=40 len=1 span[header_value]="x"
231+
off=42 error code=25 reason="Missing expected CR after header value"
232+
```

‎test/request/sample.md‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -357,30 +357,7 @@ off=6 url complete
357357
off=15 len=5 span[header_field]="Line1"
358358
off=21 header_field complete
359359
off=24 len=3 span[header_value]="abc"
360-
off=28 len=4 span[header_value]="\tdef"
361-
off=33 len=4 span[header_value]=" ghi"
362-
off=38 len=5 span[header_value]="\t\tjkl"
363-
off=44 len=6 span[header_value]=" mno "
364-
off=51 len=6 span[header_value]="\t \tqrs"
365-
off=58 header_value complete
366-
off=58 len=5 span[header_field]="Line2"
367-
off=64 header_field complete
368-
off=67 len=6 span[header_value]="line2\t"
369-
off=74 header_value complete
370-
off=74 len=5 span[header_field]="Line3"
371-
off=80 header_field complete
372-
off=82 len=5 span[header_value]="line3"
373-
off=88 header_value complete
374-
off=88 len=5 span[header_field]="Line4"
375-
off=94 header_field complete
376-
off=98 len=0 span[header_value]=""
377-
off=98 header_value complete
378-
off=98 len=10 span[header_field]="Connection"
379-
off=109 header_field complete
380-
off=111 len=5 span[header_value]="close"
381-
off=117 header_value complete
382-
off=118 headers complete method=1 v=1/1 flags=2 content_length=0
383-
off=118 message complete
360+
off=28 error code=25 reason="Missing expected CR after header value"
384361
```
385362

386363
## Request starting with CRLF

0 commit comments

Comments
 (0)