Skip to content

Commit

Permalink
Improve Handling of Some HTTP Edge Cases
Browse files Browse the repository at this point in the history 
- Allow query params that are not key-value pairs, but simply ignore them.
- Ensure a response is always sent back unless there is an I/O error.
- Add some additional logging.
  • Loading branch information
@RaphiaRa
RaphiaRa committed Sep 29, 2024
1 parent 1fdede1 commit 3db5cfc
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
9 changes: 8 additions & 1 deletion src/th_exchange.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ th_exchange_handle_request(th_exchange* handler)
th_response_async_write(response, socket, (th_io_handler*)handler);
}

TH_LOCAL(bool)
th_exchange_is_io_error(th_err err)
{
return err == TH_ERR_EOF || err == TH_EBADF || err == TH_EIO;
}

TH_LOCAL(void)
th_exchange_fn(void* self, size_t len, th_err err)
{
Expand All @@ -104,7 +110,8 @@ th_exchange_fn(void* self, size_t len, th_err err)
switch (handler->state) {
case TH_EXCHANGE_STATE_START: {
if (err != TH_ERR_OK) {
if (TH_ERR_CATEGORY(err) == TH_ERR_CATEGORY_HTTP) {
if (!th_exchange_is_io_error(err)) {
// Unless it's an I/O error, we should send a response
TH_LOG_DEBUG("%p: Rejecting request with error %s", handler, th_strerror(err));
th_exchange_write_error_response((th_exchange*)th_io_composite_ref(&handler->base), err);
} else {
Expand Down
16 changes: 16 additions & 0 deletions src/th_hashmap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
TH_INLINE(void) \
NAME##_init(NAME* map, th_allocator* allocator) TH_MAYBE_UNUSED; \
\
TH_INLINE(void) \
NAME##_reset(NAME* map) TH_MAYBE_UNUSED; \
\
TH_INLINE(th_err) \
NAME##_reserve(NAME* map, size_t capacity) TH_MAYBE_UNUSED; \
\
Expand Down Expand Up @@ -78,6 +81,19 @@
} \
} \
\
TH_INLINE(void) \
NAME##_reset(NAME* map) \
{ \
if (map->entries) { \
th_allocator_free(map->allocator, map->entries); \
} \
map->entries = NULL; \
map->size = 0; \
map->capacity = 0; \
map->begin = 0; \
map->end = 0; \
} \
\
TH_INLINE(th_err) \
NAME##_reserve(NAME* map, size_t capacity) \
{ \
Expand Down
15 changes: 13 additions & 2 deletions src/th_request.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,13 @@ th_request_parse_path(th_request* request, th_string path)
th_string uri_query = th_string_trim(th_string_substr(path, query_start + 1, path.len - query_start - 1));
if ((err = th_request_store_uri_query(request, uri_query)) != TH_ERR_OK)
return err;
return th_request_parse_path_query(request, uri_query);
if (th_request_parse_path_query(request, uri_query) != TH_ERR_OK) {
// If we can't parse the query, that's ok, we just ignore it
// restore the original state and continue
th_cstr_map_reset(&request->query_params);
// TODO, clean up the allocated strings
}
return TH_ERR_OK;
}

TH_LOCAL(th_err)
Expand Down Expand Up @@ -286,6 +292,7 @@ th_request_read_handle_header(th_request_read_handler* handler, size_t len)
int pret = phr_parse_request(th_buf_vec_at(&request->buffer, 0), data_len, &method.ptr, &method.len, &path.ptr, &path.len, &request->minor_version, headers, &num_headers, request->data_len);
request->data_len = data_len;
if (pret == -1) {
TH_LOG_DEBUG("%p: Failed to parse request", request);
th_request_read_handler_complete(handler, 0, TH_ERR_HTTP(TH_CODE_BAD_REQUEST));
return;
} else if (pret == -2) { // we need more data
Expand Down Expand Up @@ -325,18 +332,21 @@ th_request_read_handle_header(th_request_read_handler* handler, size_t len)
// find path query
th_err err = TH_ERR_OK;
if ((err = th_request_parse_path(request, path)) != TH_ERR_OK) {
th_request_read_handler_complete(handler, 0, err);
TH_LOG_DEBUG("%p: Failed to parse uri: %s", request, th_strerror(err));
th_request_read_handler_complete(handler, 0, TH_ERR_HTTP(TH_CODE_BAD_REQUEST));
return;
}

// handle headers
if ((err = th_request_handle_headers(request, headers, num_headers)) != TH_ERR_OK) {
TH_LOG_DEBUG("%p: Failed to handle headers: %s", request, th_strerror(err));
th_request_read_handler_complete(handler, 0, err);
return;
}

// Get is not allowed to have a body
if (request->method == TH_METHOD_GET && request->content_len > 0) {
TH_LOG_DEBUG("%p: Rejecting GET request with body", request);
th_request_read_handler_complete(handler, 0, TH_ERR_HTTP(TH_CODE_BAD_REQUEST));
return;
}
Expand All @@ -359,6 +369,7 @@ th_request_read_handle_header(th_request_read_handler* handler, size_t len)

// check whether the content length is ok
if (request->content_len > TH_CONFIG_MAX_CONTENT_LEN) {
TH_LOG_DEBUG("%p: Rejecting request with too large content length", request);
th_request_read_handler_complete(handler, 0, TH_ERR_HTTP(TH_CODE_PAYLOAD_TOO_LARGE));
return;
}
Expand Down

0 comments on commit 3db5cfc

Please sign in to comment.