Skip to content

Commit

Permalink
Merge branch 'http-request-line-parsing'
Browse files Browse the repository at this point in the history 
* http-request-line-parsing:
  Fix http https_basic/https_filter_basic under valgrind (increase timeout)
  http: cover various non RFC3986 conformant URIs
  http: allow non RFC3986 conformant during parsing request-line (http server)
  http: do not try to parse request-line if we do not have enough bytes
  http: allow trailing spaces (and only them) in request-line (like nginx)
  http: cleanup of the request-line parsing
  • Loading branch information
@azat
azat committed Oct 22, 2018
2 parents 2f43d1d + 5f1b4df commit 0ec12bc
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 43 deletions.
50 changes: 29 additions & 21 deletions http.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1686,8 +1686,9 @@ evhttp_parse_response_line(struct evhttp_request *req, char *line)
/* Parse the first line of a HTTP request */

static int
evhttp_parse_request_line(struct evhttp_request *req, char *line)
evhttp_parse_request_line(struct evhttp_request *req, char *line, size_t len)
{
char *eos = line + len;
char *method;
char *uri;
char *version;
Expand All @@ -1696,16 +1697,24 @@ evhttp_parse_request_line(struct evhttp_request *req, char *line)
size_t method_len;
enum evhttp_cmd_type type;

while (eos > line && *(eos-1) == ' ') {
*(eos-1) = '\0';
--eos;
--len;
}
if (len < strlen("GET / HTTP/1.0"))
return -1;

/* Parse the request line */
method = strsep(&line, " ");
if (line == NULL)
return (-1);
uri = strsep(&line, " ");
if (line == NULL)
return (-1);
version = strsep(&line, " ");
if (line != NULL)
return (-1);
if (!line)
return -1;
uri = line;
version = strrchr(uri, ' ');
if (!version || uri == version)
return -1;
*version = '\0';
version++;

method_len = (uri - method) - 1;
type = EVHTTP_REQ_UNKNOWN_;
Expand Down Expand Up @@ -1825,11 +1834,11 @@ evhttp_parse_request_line(struct evhttp_request *req, char *line)
req->type = type;

if (evhttp_parse_http_version(version, req) < 0)
return (-1);
return -1;

if ((req->uri = mm_strdup(uri)) == NULL) {
event_debug(("%s: mm_strdup", __func__));
return (-1);
return -1;
}

if (type == EVHTTP_REQ_CONNECT) {
Expand All @@ -1854,7 +1863,7 @@ evhttp_parse_request_line(struct evhttp_request *req, char *line)
!evhttp_find_vhost(req->evcon->http_server, NULL, hostname))
req->flags |= EVHTTP_PROXY_REQUEST;

return (0);
return 0;
}

const char *
Expand Down Expand Up @@ -1989,9 +1998,9 @@ evhttp_parse_firstline_(struct evhttp_request *req, struct evbuffer *buffer)
char *line;
enum message_read_status status = ALL_DATA_READ;

size_t line_length;
size_t len;
/* XXX try */
line = evbuffer_readln(buffer, &line_length, EVBUFFER_EOL_CRLF);
line = evbuffer_readln(buffer, &len, EVBUFFER_EOL_CRLF);
if (line == NULL) {
if (req->evcon != NULL &&
evbuffer_get_length(buffer) > req->evcon->max_headers_size)
Expand All @@ -2000,17 +2009,16 @@ evhttp_parse_firstline_(struct evhttp_request *req, struct evbuffer *buffer)
return (MORE_DATA_EXPECTED);
}

if (req->evcon != NULL &&
line_length > req->evcon->max_headers_size) {
if (req->evcon != NULL && len > req->evcon->max_headers_size) {
mm_free(line);
return (DATA_TOO_LONG);
}

req->headers_size = line_length;
req->headers_size = len;

switch (req->kind) {
case EVHTTP_REQUEST:
if (evhttp_parse_request_line(req, line) == -1)
if (evhttp_parse_request_line(req, line, len) == -1)
status = DATA_CORRUPTED;
break;
case EVHTTP_RESPONSE:
Expand Down Expand Up @@ -2063,12 +2071,12 @@ evhttp_parse_headers_(struct evhttp_request *req, struct evbuffer* buffer)
enum message_read_status status = MORE_DATA_EXPECTED;

struct evkeyvalq* headers = req->input_headers;
size_t line_length;
while ((line = evbuffer_readln(buffer, &line_length, EVBUFFER_EOL_CRLF))
size_t len;
while ((line = evbuffer_readln(buffer, &len, EVBUFFER_EOL_CRLF))
!= NULL) {
char *skey, *svalue;

req->headers_size += line_length;
req->headers_size += len;

if (req->evcon != NULL &&
req->headers_size > req->evcon->max_headers_size) {
Expand Down
53 changes: 31 additions & 22 deletions test/regress_http.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ http_setup_gencb(ev_uint16_t *pport, struct event_base *base, int mask,

/* Register a callback for certain types of requests */
evhttp_set_cb(myhttp, "/test", http_basic_cb, myhttp);
evhttp_set_cb(myhttp, "/test nonconformant", http_basic_cb, myhttp);
evhttp_set_cb(myhttp, "/timeout", http_timeout_cb, myhttp);
evhttp_set_cb(myhttp, "/large", http_large_cb, base);
evhttp_set_cb(myhttp, "/chunked", http_chunked_cb, base);
Expand Down Expand Up @@ -493,7 +494,7 @@ create_bev(struct event_base *base, int fd, int ssl_mask)
}

static void
http_basic_test_impl(void *arg, int ssl)
http_basic_test_impl(void *arg, int ssl, const char *request_line)
{
struct basic_test_data *data = arg;
struct timeval tv;
Expand All @@ -503,6 +504,7 @@ http_basic_test_impl(void *arg, int ssl)
ev_uint16_t port = 0, port2 = 0;
int server_flags = ssl ? HTTP_BIND_SSL : 0;
struct evhttp *http = http_setup(&port, data->base, server_flags);
struct evbuffer *out;

exit_base = data->base;
test_ok = 0;
Expand All @@ -519,21 +521,23 @@ http_basic_test_impl(void *arg, int ssl)
bev = create_bev(data->base, fd, ssl);
bufferevent_setcb(bev, http_readcb, http_writecb,
http_errorcb, data->base);
out = bufferevent_get_output(bev);

/* first half of the http request */
http_request =
"GET /test HTTP/1.1\r\n"
"Host: some";
evbuffer_add_printf(out,
"%s\r\n"
"Host: some", request_line);

bufferevent_write(bev, http_request, strlen(http_request));
evutil_timerclear(&tv);
tv.tv_usec = 100000;
if (ssl)
tv.tv_usec *= 10;
event_base_once(data->base,
-1, EV_TIMEOUT, http_complete_write, bev, &tv);

event_base_dispatch(data->base);

tt_assert(test_ok == 3);
tt_int_op(test_ok, ==, 3);

/* connect to the second port */
bufferevent_free(bev);
Expand All @@ -545,18 +549,17 @@ http_basic_test_impl(void *arg, int ssl)
bev = create_bev(data->base, fd, ssl);
bufferevent_setcb(bev, http_readcb, http_writecb,
http_errorcb, data->base);
out = bufferevent_get_output(bev);

http_request =
"GET /test HTTP/1.1\r\n"
evbuffer_add_printf(out,
"%s\r\n"
"Host: somehost\r\n"
"Connection: close\r\n"
"\r\n";

bufferevent_write(bev, http_request, strlen(http_request));
"\r\n", request_line);

event_base_dispatch(data->base);

tt_assert(test_ok == 5);
tt_int_op(test_ok, ==, 5);

/* Connect to the second port again. This time, send an absolute uri. */
bufferevent_free(bev);
Expand All @@ -579,14 +582,17 @@ http_basic_test_impl(void *arg, int ssl)

event_base_dispatch(data->base);

tt_assert(test_ok == 7);
tt_int_op(test_ok, ==, 7);

evhttp_free(http);
end:
if (bev)
bufferevent_free(bev);
}
static void http_basic_test(void *arg) { http_basic_test_impl(arg, 0); }
static void http_basic_test(void *arg)\
{ http_basic_test_impl(arg, 0, "GET /test HTTP/1.1"); }
static void http_basic_trailing_space_test(void *arg)
{ http_basic_test_impl(arg, 0, "GET /test HTTP/1.1 "); }


static void
Expand Down Expand Up @@ -3642,7 +3648,7 @@ http_make_web_server(evutil_socket_t fd, short what, void *arg)
}

static void
http_simple_test_impl(void *arg, int ssl, int dirty)
http_simple_test_impl(void *arg, int ssl, int dirty, const char *uri)
{
struct basic_test_data *data = arg;
struct evhttp_connection *evcon = NULL;
Expand All @@ -3667,9 +3673,8 @@ http_simple_test_impl(void *arg, int ssl, int dirty)
req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY);
tt_assert(req);

if (evhttp_make_request(evcon, req, EVHTTP_REQ_GET, "/test") == -1) {
if (evhttp_make_request(evcon, req, EVHTTP_REQ_GET, uri) == -1)
tt_abort_msg("Couldn't make request");
}

event_base_dispatch(data->base);
tt_int_op(test_ok, ==, 1);
Expand All @@ -3681,7 +3686,9 @@ http_simple_test_impl(void *arg, int ssl, int dirty)
evhttp_free(http);
}
static void http_simple_test(void *arg)
{ http_simple_test_impl(arg, 0, 0); }
{ http_simple_test_impl(arg, 0, 0, "/test"); }
static void http_simple_nonconformant_test(void *arg)
{ http_simple_test_impl(arg, 0, 0, "/test nonconformant"); }

static void
http_connection_retry_test_basic(void *arg, const char *addr, struct evdns_base *dns_base, int ssl)
Expand Down Expand Up @@ -4761,17 +4768,17 @@ http_newreqcb_test(void *arg)

#ifdef EVENT__HAVE_OPENSSL
static void https_basic_test(void *arg)
{ http_basic_test_impl(arg, 1); }
{ http_basic_test_impl(arg, 1, "GET /test HTTP/1.1"); }
static void https_filter_basic_test(void *arg)
{ http_basic_test_impl(arg, 1 | HTTP_SSL_FILTER); }
{ http_basic_test_impl(arg, 1 | HTTP_SSL_FILTER, "GET /test HTTP/1.1"); }
static void https_incomplete_test(void *arg)
{ http_incomplete_test_(arg, 0, 1); }
static void https_incomplete_timeout_test(void *arg)
{ http_incomplete_test_(arg, 1, 1); }
static void https_simple_test(void *arg)
{ http_simple_test_impl(arg, 1, 0); }
{ http_simple_test_impl(arg, 1, 0, "/test"); }
static void https_simple_dirty_test(void *arg)
{ http_simple_test_impl(arg, 1, 1); }
{ http_simple_test_impl(arg, 1, 1, "/test"); }
static void https_connection_retry_conn_address_test(void *arg)
{ http_connection_retry_conn_address_test_impl(arg, 1); }
static void https_connection_retry_test(void *arg)
Expand Down Expand Up @@ -4801,7 +4808,9 @@ struct testcase_t http_testcases[] = {
{ "parse_uri_nc", http_parse_uri_test, 0, &basic_setup, (void*)"nc" },
{ "uriencode", http_uriencode_test, 0, NULL, NULL },
HTTP(basic),
HTTP(basic_trailing_space),
HTTP(simple),
HTTP(simple_nonconformant),

HTTP_N(cancel, cancel, BASIC),
HTTP_N(cancel_by_host, cancel, BY_HOST),
Expand Down

0 comments on commit 0ec12bc

Please sign in to comment.