Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

Implementing extension methods in a way that does not slow down performance... #158

Closed
jasnell wants to merge 2 commits into from
Closed

Implementing extension methods in a way that does not slow down performance... #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e353a1d
Support for unknown extension methods in the http parser.. implemented
jasnell Aug 21, 2013
41d5c46
let's make sure all our method characters are valid in a token.
jasnell Aug 21, 2013
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
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,7 @@ parsertrace_g
*.Makefile
*.so.*
*.a

.cproject

This comment was marked as off-topic.

Sign in to view


.project
65 changes: 40 additions & 25 deletions http_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ do { \
#define CALLBACK_NOTIFY_NOADVANCE(FOR) CALLBACK_NOTIFY_(FOR, p - data)

/* Run data callback FOR with LEN bytes, returning ER if it fails */
#define CALLBACK_DATA_(FOR, LEN, ER) \
#define CALLBACK_DATA_(FOR, LEN, ER, ERR) \
do { \
assert(HTTP_PARSER_ERRNO(parser) == HPE_OK); \
\
if (FOR##_mark) { \
if (settings->on_##FOR) { \
if (0 != settings->on_##FOR(parser, FOR##_mark, (LEN))) { \
SET_ERRNO(HPE_CB_##FOR); \
SET_ERRNO(ERR); \

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

} \
\
/* We either errored above or got paused; get out */ \
Expand All @@ -102,11 +102,11 @@ do { \

/* Run the data callback FOR and consume the current byte */
#define CALLBACK_DATA(FOR) \
CALLBACK_DATA_(FOR, p - FOR##_mark, p - data + 1)
CALLBACK_DATA_(FOR, p - FOR##_mark, p - data + 1, HPE_CB_##FOR)

/* Run the data callback FOR and don't consume the current byte */
#define CALLBACK_DATA_NOADVANCE(FOR) \
CALLBACK_DATA_(FOR, p - FOR##_mark, p - data)
CALLBACK_DATA_(FOR, p - FOR##_mark, p - data, HPE_CB_##FOR)

/* Set the mark FOR; non-destructive if mark is already set */
#define MARK(FOR) \
Expand Down Expand Up @@ -579,6 +579,7 @@ size_t http_parser_execute (http_parser *parser,
const char *p = data;
const char *header_field_mark = 0;
const char *header_value_mark = 0;
const char *unknown_method_mark = 0;
const char *url_mark = 0;
const char *body_mark = 0;

Expand Down Expand Up @@ -613,6 +614,8 @@ size_t http_parser_execute (http_parser *parser,
header_field_mark = data;
if (parser->state == s_header_value)
header_value_mark = data;
if (parser->state == s_start_req)
unknown_method_mark = data;

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

switch (parser->state) {
case s_req_path:
case s_req_schema:
Expand Down Expand Up @@ -876,7 +879,7 @@ size_t http_parser_execute (http_parser *parser,
parser->flags = 0;
parser->content_length = ULLONG_MAX;

if (!IS_ALPHA(ch)) {
if (!TOKEN(ch)) {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
}
Expand All @@ -900,8 +903,7 @@ size_t http_parser_execute (http_parser *parser,
case 'T': parser->method = HTTP_TRACE; break;
case 'U': parser->method = HTTP_UNLOCK; /* or UNSUBSCRIBE */ break;
default:
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}
parser->state = s_req_method;

Expand All @@ -919,8 +921,16 @@ size_t http_parser_execute (http_parser *parser,
}

matcher = method_strings[parser->method];
if (ch == ' ' && matcher[parser->index] == '\0') {

if (!TOKEN(ch) && ch != ' ') {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
}

if (ch == ' ' && (parser->method == HTTP_unknown || matcher[parser->index] == '\0')) {

This comment was marked as off-topic.

Sign in to view

parser->state = s_req_spaces_before_url;
} else if (parser->method == HTTP_unknown) {
; /* nada */
} else if (ch == matcher[parser->index]) {
; /* nada */
} else if (parser->method == HTTP_CONNECT) {
Expand All @@ -929,8 +939,7 @@ size_t http_parser_execute (http_parser *parser,
} else if (parser->index == 2 && ch == 'P') {
parser->method = HTTP_COPY;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;

This comment was marked as off-topic.

Sign in to view

}
} else if (parser->method == HTTP_MKCOL) {
if (parser->index == 1 && ch == 'O') {
Expand All @@ -942,15 +951,13 @@ size_t http_parser_execute (http_parser *parser,
} else if (parser->index == 2 && ch == 'A') {
parser->method = HTTP_MKACTIVITY;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;

This comment was marked as off-topic.

Sign in to view

}
} else if (parser->method == HTTP_SUBSCRIBE) {
if (parser->index == 1 && ch == 'E') {
parser->method = HTTP_SEARCH;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}
} else if (parser->index == 1 && parser->method == HTTP_POST) {
if (ch == 'R') {
Expand All @@ -960,33 +967,28 @@ size_t http_parser_execute (http_parser *parser,
} else if (ch == 'A') {
parser->method = HTTP_PATCH;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}
} else if (parser->index == 2) {
if (parser->method == HTTP_PUT) {
if (ch == 'R') {
parser->method = HTTP_PURGE;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}
} else if (parser->method == HTTP_UNLOCK) {
if (ch == 'S') {
parser->method = HTTP_UNSUBSCRIBE;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}
} else if (parser->index == 4 && parser->method == HTTP_PROPFIND && ch == 'P') {
parser->method = HTTP_PROPPATCH;
} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
parser->method = HTTP_unknown;
}

++parser->index;
Expand All @@ -995,6 +997,19 @@ size_t http_parser_execute (http_parser *parser,

case s_req_spaces_before_url:
{
// Check if the method is unknown, if it is, send it to the

This comment was marked as off-topic.

Sign in to view

// unknown method callback handler (if set). If that callback
// returns != 0, specify an HPE_INVALID_METHOD error. Otherwise,
// just accept it and keep going.
if (parser->method == HTTP_unknown) {
if (settings->on_unknown_method) {
CALLBACK_DATA_(unknown_method, p - unknown_method_mark - 1, p - data, HPE_INVALID_METHOD);

This comment was marked as off-topic.

Sign in to view

} else {
SET_ERRNO(HPE_INVALID_METHOD);
goto error;
}
}

if (ch == ' ') break;

MARK(url);
Expand Down Expand Up @@ -1687,7 +1702,7 @@ size_t http_parser_execute (http_parser *parser,
* complete-on-length. It's not clear that this distinction is
* important for applications, but let's keep it for now.
*/
CALLBACK_DATA_(body, p - body_mark + 1, p - data);
CALLBACK_DATA_(body, p - body_mark + 1, p - data, HPE_CB_body);
goto reexecute_byte;
}

Expand Down
3 changes: 3 additions & 0 deletions http_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ typedef int (*http_cb) (http_parser*);
/* RFC-5789 */ \
XX(24, PATCH, PATCH) \
XX(25, PURGE, PURGE) \
XX(99, unknown, unknown) \

enum http_method
{
Expand Down Expand Up @@ -150,6 +151,7 @@ enum flags
XX(CB_headers_complete, "the on_headers_complete callback failed") \
XX(CB_body, "the on_body callback failed") \
XX(CB_message_complete, "the on_message_complete callback failed") \
XX(CB_unknown_method, "the on_unknown_method callback failed") \
\
/* Parsing-related errors */ \
XX(INVALID_EOF_STATE, "stream ended at an unexpected time") \
Expand Down Expand Up @@ -230,6 +232,7 @@ struct http_parser_settings {
http_cb on_headers_complete;
http_data_cb on_body;
http_cb on_message_complete;
http_data_cb on_unknown_method;
};


Expand Down
46 changes: 41 additions & 5 deletions test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct message {
enum http_parser_type type;
enum http_method method;
int status_code;
char method_string[MAX_ELEMENT_SIZE];
char request_path[MAX_ELEMENT_SIZE];
char request_url[MAX_ELEMENT_SIZE];
char fragment[MAX_ELEMENT_SIZE];
Expand Down Expand Up @@ -1498,6 +1499,20 @@ status_complete_cb (http_parser *p) {
return 0;
}

int
unknown_method_cb (http_parser *p, const char *buf, size_t len)
{
assert(p == parser);
strlncat(messages[num_messages].method_string,
sizeof(messages[num_messages].method_string),
buf,
len);
// fprintf(stderr, messages[num_messages].method_string
// return 0 if the unknown method is handled by the callback,
// otherwise return -1 to cause an HPE_INVALID_METHOD
return 0;
}

int
header_field_cb (http_parser *p, const char *buf, size_t len)
{
Expand Down Expand Up @@ -1761,6 +1776,7 @@ static http_parser_settings settings_pause =
,.on_body = pause_body_cb
,.on_headers_complete = pause_headers_complete_cb
,.on_message_complete = pause_message_complete_cb
,.on_unknown_method = unknown_method_cb
};

static http_parser_settings settings =
Expand All @@ -1771,6 +1787,7 @@ static http_parser_settings settings =
,.on_body = body_cb
,.on_headers_complete = headers_complete_cb
,.on_message_complete = message_complete_cb
,.on_unknown_method = unknown_method_cb
};

static http_parser_settings settings_count_body =
Expand All @@ -1781,6 +1798,7 @@ static http_parser_settings settings_count_body =
,.on_body = count_body_cb
,.on_headers_complete = headers_complete_cb
,.on_message_complete = message_complete_cb
,.on_unknown_method = unknown_method_cb
};

static http_parser_settings settings_null =
Expand All @@ -1791,6 +1809,7 @@ static http_parser_settings settings_null =
,.on_body = 0
,.on_headers_complete = 0
,.on_message_complete = 0
,.on_unknown_method = 0
};

void
Expand Down Expand Up @@ -3316,7 +3335,9 @@ main (void)
test_simple(buf, HPE_OK);
}

static const char *bad_methods[] = {
static const char *ext_methods[] = {
"LINK",
"UNLINK",
"ASDF",
"C******",
"COLA",
Expand All @@ -3328,12 +3349,27 @@ main (void)
"PUN",
"PX",
"SA",
"hello world",
0 };
for (this_method = bad_methods; *this_method; this_method++) {
"+1",
0
};

for (this_method = ext_methods; *this_method; this_method++) {
char buf[200];
sprintf(buf, "%s / HTTP/1.1\r\n\r\n", *this_method);
test_simple(buf, HPE_INVALID_METHOD);
test_simple(buf, HPE_OK);
}

static const char *bad_methods[] = {
"JO]",
"GE]",
"[GET]",
0
};

for (this_method = bad_methods; *this_method; this_method++) {
char buf[200];
sprintf(buf, "%s / HTTP/1.1\r\n\r\n", *this_method);
test_simple(buf, HPE_INVALID_METHOD);
}

const char *dumbfuck2 =
Expand Down