Skip to content

http: lenient parsing flag #33

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.

Sign up for GitHub

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

Closed
Parent: Update README.md
wants to merge 1 commit into from
Closed

http: lenient parsing flag #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/llhttp/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export enum FLAGS {
CONTENT_LENGTH = 1 << 5,
SKIPBODY = 1 << 6,
TRAILING = 1 << 7,
LENIENT = 1 << 8,
}

export enum METHODS {
Expand Down
16 changes: 12 additions & 4 deletions src/llhttp/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const NODES: ReadonlyArray<string> = [
'header_value_start',
'header_value',
'header_value_otherwise',
'header_value_lenient',
'header_value_lws',
'header_value_te_chunked',
'header_value_content_length_once',
Expand Down Expand Up @@ -177,7 +178,7 @@ export class HTTP {
p.property('i8', 'http_major');
p.property('i8', 'http_minor');
p.property('i8', 'header_state');
p.property('i8', 'flags');
p.property('i16', 'flags');
p.property('i8', 'upgrade');
p.property('i16', 'status_code');
p.property('i8', 'finish');
Expand Down Expand Up @@ -491,12 +492,19 @@ export class HTTP {
.match(HEADER_CHARS, n('header_value'))
.otherwise(n('header_value_otherwise'));

const checkLenient = this.testFlags(FLAGS.LENIENT, {
1: n('header_value_lenient'),
}, p.error(ERROR.INVALID_HEADER_TOKEN, 'Invalid header value char'));

n('header_value_otherwise')
.peek('\r', span.headerValue.end().skipTo(n('header_value_almost_done')))
.peek('\n', span.headerValue.end(n('header_value_almost_done')))
// TODO(indutny): do we need `lenient` option? (it is always off now)
.otherwise(p.error(ERROR.INVALID_HEADER_TOKEN,
'Invalid header value char'));
.otherwise(checkLenient);

n('header_value_lenient')
.peek('\r', span.headerValue.end().skipTo(n('header_value_almost_done')))
.peek('\n', span.headerValue.end(n('header_value_almost_done')))
.skipTo(n('header_value_lenient'));

n('header_value_almost_done')
.match('\n', n('header_value_lws'))
Expand Down
9 changes: 9 additions & 0 deletions src/native/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ const char* llhttp_method_name(llhttp_method_t method) {
}


void llhttp_set_lenient(llhttp_t* parser, int enabled) {
if (enabled) {
parser->flags |= F_LENIENT;
} else {
parser->flags &= ~F_LENIENT;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter/branchless way of doing the same (not that it really matters but, you know, it looks neat):

parser->flags ^= F_LENIENT & parser->flags;
parser->flags ^= F_LENIENT * !!enabled;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. While we are here, am I right that &= ~F_LENIENT would not work reliably here? (out of curiosity)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does actually but two statements with a common prefix is aesthetically pleasing to me. :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to go with ~F_LENIENT as it is less tricky and explicit.



/* Callbacks */


Expand Down
12 changes: 12 additions & 0 deletions src/native/api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,18 @@ const char* llhttp_errno_name(llhttp_errno_t err);
/* Returns textual name of HTTP method */
const char* llhttp_method_name(llhttp_method_t method);


/* Enables/disables lenient header value parsing (disabled by default).
*
* Lenient parsing disables header value token checks, extending llhttp's
* protocol support to highly non-compliant clients/server. No
* `HPE_INVALID_HEADER_TOKEN` will be raised for incorrect header values when
* lenient parsing is "on".
*
* **(USE AT YOUR OWN RISK)**
*/
void llhttp_set_lenient(llhttp_t* parser, int enabled);

#ifdef __cplusplus
} /* extern "C" */
#endif
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/extra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ void llhttp__test_init_response(llparse_t* s) {
}


void llhttp__test_init_request_lenient(llparse_t* s) {
llhttp__test_init_request(s);
s->flags |= F_LENIENT;
}


void llhttp__test_finish(llparse_t* s) {
llparse__print(NULL, NULL, "finish=%d", s->finish);
}
Expand Down
9 changes: 5 additions & 4 deletions test/fixtures/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import * as path from 'path';

import * as llhttp from '../../src/llhttp';

export type TestType = 'request' | 'response' | 'request-finish' |
'response-finish' | 'none' | 'url';
export type TestType = 'request' | 'response' | 'request-lenient' |
'request-finish' | 'response-finish' | 'none' | 'url';

export { FixtureResult };

Expand Down Expand Up @@ -58,8 +58,9 @@ export function build(llparse: LLParse, node: any, outFile: string,
}

const extra = options.extra === undefined ? [] : options.extra.slice();
if (ty === 'request' || ty === 'response') {
extra.push(`-DLLPARSE__TEST_INIT=llhttp__test_init_${ty}`);
if (ty === 'request' || ty === 'response' || ty === 'request-lenient') {
extra.push(
`-DLLPARSE__TEST_INIT=llhttp__test_init_${ty.replace(/-/g, '_')}`);
} else if (ty === 'request-finish' || ty === 'response-finish') {
if (ty === 'request-finish') {
extra.push('-DLLPARSE__TEST_INIT=llhttp__test_init_request');
Expand Down
5 changes: 5 additions & 0 deletions test/md-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const http: IFixtureMap = {
'none': buildMode('loose', 'none'),
'request': buildMode('loose', 'request'),
'request-finish': buildMode('loose', 'request-finish'),
'request-lenient': buildMode('loose', 'request-lenient'),
'response': buildMode('loose', 'response'),
'response-finish': buildMode('loose', 'response-finish'),
'url': buildMode('loose', 'url'),
Expand All @@ -86,6 +87,7 @@ const http: IFixtureMap = {
'none': buildMode('strict', 'none'),
'request': buildMode('strict', 'request'),
'request-finish': buildMode('strict', 'request-finish'),
'request-lenient': buildMode('strict', 'request-lenient'),
'response': buildMode('strict', 'response'),
'response-finish': buildMode('strict', 'response-finish'),
'url': buildMode('strict', 'url'),
Expand Down Expand Up @@ -143,6 +145,8 @@ function run(name: string): void {
types.push('response');
} else if (meta.type === 'request-only') {
types = [ 'request' ];
} else if (meta.type === 'request-lenient') {
types = [ 'request-lenient' ];
} else if (meta.type === 'response-only') {
types = [ 'response' ];
} else if (meta.type === 'request-finish') {
Expand Down Expand Up @@ -231,6 +235,7 @@ function run(name: string): void {
}

run('request/sample');
run('request/lenient');
run('request/method');
run('request/uri');
run('request/connection');
Expand Down
41 changes: 41 additions & 0 deletions test/request/lenient.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
Lenient header value parsing
============================

Parsing with header value token checks off.

## Header value with lenient

<!-- meta={"type": "request-lenient"} -->
```http
GET /url HTTP/1.1
Header1: \f


```

```log
off=0 message begin
off=4 len=4 span[url]="/url"
off=19 len=7 span[header_field]="Header1"
off=28 len=1 span[header_value]="\f"
off=33 headers complete method=1 v=1/1 flags=100 content_length=0
off=33 message complete
```

## Header value without lenient

<!-- meta={"type": "request"} -->
```http
GET /url HTTP/1.1
Header1: \f



```

```log
off=0 message begin
off=4 len=4 span[url]="/url"
off=19 len=7 span[header_field]="Header1"
off=28 error code=10 reason="Invalid header value char"
```