Skip to content

Commit

Permalink
api: http_parser_finish
Browse files Browse the repository at this point in the history
  • Loading branch information
indutny committed Mar 21, 2018
1 parent 5a00fdd commit 6fdd3af
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 16 deletions.
3 changes: 3 additions & 0 deletions src/llhttp/c-headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export class CHeaders {
res += this.buildEnum('http_parser_type', 'HTTP',
enumToMap(constants.TYPE));
res += '\n';
res += this.buildEnum('http_finish', 'HTTP_FINISH',
enumToMap(constants.FINISH));
res += '\n';

const methodMap = enumToMap(constants.METHODS);
res += this.buildEnum('http_method', 'HTTP', methodMap);
Expand Down
7 changes: 7 additions & 0 deletions src/llhttp/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export enum ERROR {
INVALID_CONTENT_LENGTH,
INVALID_CHUNK_SIZE,
INVALID_STATUS,
INVALID_EOF_STATE,

CB_MESSAGE_BEGIN,
CB_HEADERS_COMPLETE,
Expand Down Expand Up @@ -100,6 +101,12 @@ Object.keys(METHOD_MAP).forEach((key) => {
}
});

export enum FINISH {
SAFE = 0,
SAFE_WITH_CB,
UNSAFE,
}

// Internal

export type CharList = Array<string | number>;
Expand Down
23 changes: 16 additions & 7 deletions src/llhttp/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import Node = apiNode.Node;

import {
CharList,
CONNECTION_TOKEN_CHARS, ERROR, FLAGS, H_METHOD_MAP, HEADER_CHARS, HEADER_STATE, HEX_MAP,
CONNECTION_TOKEN_CHARS, ERROR, FINISH, FLAGS, H_METHOD_MAP, HEADER_CHARS,
HEADER_STATE, HEX_MAP,
HTTPMode,
MAJOR, METHOD_MAP, METHODS, MINOR, NUM_MAP, SPECIAL_HEADERS, STRICT_TOKEN,
TOKEN, TYPE,
Expand Down Expand Up @@ -167,6 +168,7 @@ export class HTTP {
public build(): IHTTPResult {
const p = this.llparse;

p.property('i64', 'content_length');
p.property('i8', 'type');
p.property('i8', 'method');
p.property('i8', 'http_major');
Expand All @@ -175,7 +177,11 @@ export class HTTP {
p.property('i8', 'flags');
p.property('i8', 'upgrade');
p.property('i16', 'status_code');
p.property('i64', 'content_length');
p.property('i8', 'finish');

// Verify defaults
assert.strictEqual(FINISH.SAFE, 0);
assert.strictEqual(TYPE.BOTH, 0);

// Shared settings (to be used in C wrapper)
p.property('ptr', 'settings');
Expand All @@ -202,8 +208,9 @@ export class HTTP {

n('start')
.match([ '\r', '\n' ], n('start'))
.otherwise(this.invokePausable('on_message_begin',
ERROR.CB_MESSAGE_BEGIN, switchType));
.otherwise(this.update('finish', FINISH.UNSAFE,
this.invokePausable('on_message_begin',
ERROR.CB_MESSAGE_BEGIN, switchType)));

n('start_req_or_res')
.peek('H', n('req_or_res_method'))
Expand All @@ -219,7 +226,7 @@ export class HTTP {

n('start_res')
.match('HTTP/', n('res_http_major'))
.skipTo(n('start_res'));
.otherwise(p.error(ERROR.INVALID_CONSTANT, 'Expected HTTP/'));

n('res_http_major')
.select(MAJOR, this.store('http_major', 'res_http_dot'))
Expand Down Expand Up @@ -271,7 +278,8 @@ export class HTTP {
// Request

n('start_req')
.select(METHOD_MAP, this.store('method', 'req_spaces_before_url'))
.select(METHOD_MAP,
this.store('method', 'req_spaces_before_url'))
.otherwise(p.error(ERROR.INVALID_METHOD, 'Invalid method encountered'));

n('req_spaces_before_url')
Expand Down Expand Up @@ -553,7 +561,8 @@ export class HTTP {
span.body.end(n('message_done')))));

n('body_identity_eof')
.otherwise(span.body.start(n('eof')));
.otherwise(span.body.start(
this.update('finish', FINISH.SAFE_WITH_CB, 'eof')));

// Just read everything until EOF
n('eof')
Expand Down
23 changes: 23 additions & 0 deletions src/native/api.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include <stdlib.h>
#include <string.h>

#include "http_parser.h"
Expand All @@ -18,6 +19,28 @@ void http_parser_settings_init(http_parser_settings_t* settings) {
}


int http_parser_finish(http_parser_t* parser) {
int ret;

switch (parser->finish) {
case HTTP_FINISH_SAFE_WITH_CB:
ret = ((http_parser_settings_t*) parser->settings)
->on_message_complete(parser);
if (ret != 0) {
return ret;
}

/* FALLTHROUGH */
case HTTP_FINISH_SAFE:
return 0;
case HTTP_FINISH_UNSAFE:
return HPE_INVALID_EOF_STATE;
default:
abort();
}
}


/* Callbacks */


Expand Down
7 changes: 4 additions & 3 deletions src/native/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ struct http_parser_settings_s {
http_cb on_chunk_complete;
};

int http_message_needs_eof(const http_parser_t* parser);
int http_should_keep_alive(const http_parser_t* parser);

void http_parser_set_type(http_parser_t* parser, enum http_parser_type type);
void http_parser_set_settings(http_parser_t* parser,
const http_parser_settings_t* settings);
void http_parser_settings_init(http_parser_settings_t* settings);

int http_parser_message_needs_eof(const http_parser_t* parser);
int http_parser_should_keep_alive(const http_parser_t* parser);
int http_parser_finish(http_parser_t* parser);

#ifdef __cplusplus
} /* extern "C" */
#endif
Expand Down
15 changes: 10 additions & 5 deletions src/native/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,17 @@ int http_parser__after_headers_complete(http_parser_t* parser, const char* p,

int http_parser__after_message_complete(http_parser_t* parser, const char* p,
const char* endp) {
int should_keep_alive;

should_keep_alive = http_should_keep_alive(parser);
parser->flags = 0;
return http_should_keep_alive(parser);
parser->finish = HTTP_FINISH_SAFE;

return should_keep_alive;
}


int http_message_needs_eof(const http_parser_t* parser) {
int http_parser_message_needs_eof(const http_parser_t* parser) {
if (parser->type == HTTP_REQUEST) {
return 0;
}
Expand All @@ -86,7 +91,7 @@ int http_message_needs_eof(const http_parser_t* parser) {
if (parser->status_code / 100 == 1 || /* 1xx e.g. Continue */
parser->status_code == 204 || /* No Content */
parser->status_code == 304 || /* Not Modified */
parser->flags & F_SKIPBODY) { /* response to a HEAD request */
(parser->flags & F_SKIPBODY)) { /* response to a HEAD request */

This comment has been minimized.

Copy link
@ronag

ronag Jan 16, 2023

Member

@indutny What is the reason for this? Now it's impossible to have keep-alive HEAD requests. Refs: nodejs/undici#1858

This comment has been minimized.

Copy link
@ShogunPanda

ShogunPanda Jan 17, 2023

Contributor

As the file says, it seems he was trying to strictly implement section 4.4 of RFC 2616.
I guess for your use case we need a new API for setting a flag to force keep-alive (since the same bug would arise even for 204 responses and similar.

This comment has been minimized.

Copy link
@pallas

pallas via email Jan 17, 2023

Contributor
return 0;
}

Expand All @@ -98,7 +103,7 @@ int http_message_needs_eof(const http_parser_t* parser) {
}


int http_should_keep_alive(const http_parser_t* parser) {
int http_parser_should_keep_alive(const http_parser_t* parser) {
if (parser->http_major > 0 && parser->http_minor > 0) {
/* HTTP/1.1 */
if (parser->flags & F_CONNECTION_CLOSE) {
Expand All @@ -111,5 +116,5 @@ int http_should_keep_alive(const http_parser_t* parser) {
}
}

return !http_message_needs_eof(parser);
return !http_parser_message_needs_eof(parser);
}
6 changes: 5 additions & 1 deletion test/fixtures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,9 @@ export function build(llparse: LLParse, node: any, outFile: string,
const dot = new Dot();
fs.writeFileSync(path.join(BUILD_DIR, outFile + '.dot'),
dot.build(node));
return fixtures.build(llparse.build(node), outFile, options);

const artifacts = llparse.build(node, {
debug: process.env.LLPARSE_DEBUG ? 'llparse__debug' : undefined,
});
return fixtures.build(artifacts, outFile, options);
}
20 changes: 20 additions & 0 deletions test/http-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ describe('http_parser/http', () => {
await http.check(req, expected);
});

it('should parse emit proper error on invalid response', async () => {
const req =
'HTTP/1.1 200 OK\r\nContent-Length: 0\r\n' +
'\r\n' +
'HTTPER/1.1 200 OK\r\n\r\n';

const expected = [
'off=0 message begin',
'off=13 len=2 span[status]="OK"',
'off=17 len=14 span[header_field]="Content-Length"',
'off=33 len=1 span[header_value]="0"',
'off=38 headers complete status=200 v=1/1 flags=20 content_length=0',
'off=38 message complete',
'off=38 message begin',
'off=42 error code=8 reason="Expected HTTP/"',
];

await http.check(req, expected);
});

describe('content-length', () => {
it('should parse content-length', async () => {
const req =
Expand Down

0 comments on commit 6fdd3af

Please sign in to comment.