From b4354f147f657a22309ebf871c0433130acf2ecf Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Tue, 25 Mar 2025 14:58:32 -0400 Subject: [PATCH 1/4] HTTP: Use common header validation function for HTTP/2 and HTTP/3 The header validation required by HTTP/2 and HTTP/3 is identical, so use a common function for both. This will make it easier to add additional validation in the future. Move the function to a new file. No functional change intended. Signed-off-by: Demi Marie Obenour --- auto/modules | 2 +- auto/sources | 5 +- src/http/modules/ngx_http_proxy_v2_module.c | 72 +---------------- src/http/ngx_http.h | 2 + src/http/ngx_http_v23_common.c | 86 +++++++++++++++++++++ src/http/v2/ngx_http_v2.c | 57 +------------- src/http/v3/ngx_http_v3_request.c | 55 +------------ 7 files changed, 96 insertions(+), 183 deletions(-) create mode 100644 src/http/ngx_http_v23_common.c diff --git a/auto/modules b/auto/modules index f02691e16..9cfd582d6 100644 --- a/auto/modules +++ b/auto/modules @@ -103,7 +103,7 @@ if [ $HTTP = YES ]; then if [ $HTTP_V2 = YES -o $HTTP_V3 = YES ]; then - HTTP_SRCS="$HTTP_SRCS $HTTP_HUFF_SRCS" + HTTP_SRCS="$HTTP_SRCS $HTTP_V23_COMMON_SRCS" fi diff --git a/auto/sources b/auto/sources index 46408ee53..a5c173fb9 100644 --- a/auto/sources +++ b/auto/sources @@ -257,5 +257,6 @@ NGX_WIN32_RC="src/os/win32/nginx.rc" HTTP_FILE_CACHE_SRCS=src/http/ngx_http_file_cache.c -HTTP_HUFF_SRCS="src/http/ngx_http_huff_decode.c - src/http/ngx_http_huff_encode.c" +HTTP_V23_COMMON_SRCS="src/http/ngx_http_huff_decode.c \ + src/http/ngx_http_huff_encode.c \ + src/http/ngx_http_v23_common.c" diff --git a/src/http/modules/ngx_http_proxy_v2_module.c b/src/http/modules/ngx_http_proxy_v2_module.c index 4c282a864..3bd7cd9a8 100644 --- a/src/http/modules/ngx_http_proxy_v2_module.c +++ b/src/http/modules/ngx_http_proxy_v2_module.c @@ -132,10 +132,6 @@ static ngx_int_t ngx_http_proxy_v2_parse_header(ngx_http_request_t *r, ngx_http_proxy_v2_ctx_t *ctx, ngx_buf_t *b); static ngx_int_t ngx_http_proxy_v2_parse_fragment(ngx_http_request_t *r, ngx_http_proxy_v2_ctx_t *ctx, ngx_buf_t *b); -static ngx_int_t ngx_http_proxy_v2_validate_header_name(ngx_http_request_t *r, - ngx_str_t *s); -static ngx_int_t ngx_http_proxy_v2_validate_header_value(ngx_http_request_t *r, - ngx_str_t *s); static ngx_int_t ngx_http_proxy_v2_parse_rst_stream(ngx_http_request_t *r, ngx_http_proxy_v2_ctx_t *ctx, ngx_buf_t *b); static ngx_int_t ngx_http_proxy_v2_parse_goaway(ngx_http_request_t *r, @@ -3206,29 +3202,7 @@ ngx_http_proxy_v2_parse_fragment(ngx_http_request_t *r, ctx->value = *ngx_http_v2_get_static_value(ctx->index); } - if (!ctx->index) { - if (ngx_http_proxy_v2_validate_header_name(r, &ctx->name) - != NGX_OK) - { - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent invalid header: \"%V: %V\"", - &ctx->name, &ctx->value); - return NGX_ERROR; - } - } - - if (!ctx->index || ctx->literal) { - if (ngx_http_proxy_v2_validate_header_value(r, &ctx->value) - != NGX_OK) - { - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent invalid header: \"%V: %V\"", - &ctx->name, &ctx->value); - return NGX_ERROR; - } - } - - return NGX_OK; + return ngx_http_v23_validate_header(r, &ctx->name, &ctx->value, 0); } ctx->rest -= p - b->pos; @@ -3243,50 +3217,6 @@ ngx_http_proxy_v2_parse_fragment(ngx_http_request_t *r, } -static ngx_int_t -ngx_http_proxy_v2_validate_header_name(ngx_http_request_t *r, ngx_str_t *s) -{ - u_char ch; - ngx_uint_t i; - - for (i = 0; i < s->len; i++) { - ch = s->data[i]; - - if (ch == ':' && i > 0) { - return NGX_ERROR; - } - - if (ch >= 'A' && ch <= 'Z') { - return NGX_ERROR; - } - - if (ch <= 0x20 || ch == 0x7f) { - return NGX_ERROR; - } - } - - return NGX_OK; -} - - -static ngx_int_t -ngx_http_proxy_v2_validate_header_value(ngx_http_request_t *r, ngx_str_t *s) -{ - u_char ch; - ngx_uint_t i; - - for (i = 0; i < s->len; i++) { - ch = s->data[i]; - - if (ch == '\0' || ch == CR || ch == LF) { - return NGX_ERROR; - } - } - - return NGX_OK; -} - - static ngx_int_t ngx_http_proxy_v2_parse_rst_stream(ngx_http_request_t *r, ngx_http_proxy_v2_ctx_t *ctx, ngx_buf_t *b) diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index 4e4511cc5..92a626f22 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -184,6 +184,8 @@ ngx_int_t ngx_http_huff_decode(u_char *state, u_char *src, size_t len, u_char **dst, ngx_uint_t last, ngx_log_t *log); size_t ngx_http_huff_encode(u_char *src, size_t len, u_char *dst, ngx_uint_t lower); +ngx_int_t ngx_http_v23_validate_header(ngx_http_request_t *r, + ngx_str_t *name, ngx_str_t *value, ngx_int_t is_client); #endif diff --git a/src/http/ngx_http_v23_common.c b/src/http/ngx_http_v23_common.c new file mode 100644 index 000000000..fe009919a --- /dev/null +++ b/src/http/ngx_http_v23_common.c @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2026 Demi Marie Obenour + */ + + +#include +#include +#include + + +static inline ngx_int_t +ngx_isspace(u_char ch); + + +ngx_int_t +ngx_http_v23_validate_header(ngx_http_request_t *r, + ngx_str_t *name, ngx_str_t *value, ngx_int_t is_client) +{ + u_char ch; + ngx_uint_t i; + ngx_http_core_srv_conf_t *cscf; + + if (is_client) { + r->invalid_header = 0; + } + + cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); + + if (name->len < 1) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "%s sent empty header name", + is_client ? "client" : "upstream"); + + return NGX_ERROR; + } + + for (i = (name->data[0] == ':'); i != name->len; i++) { + ch = name->data[i]; + + if (is_client + && ((ch >= 'a' && ch <= 'z') + || (ch == '-') + || (ch >= '0' && ch <= '9') + || (ch == '_' && cscf->underscores_in_headers))) + { + continue; + } + + if (ch <= 0x20 || ch == 0x7f || ch == ':' + || (ch >= 'A' && ch <= 'Z')) + { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "%s sent invalid header name: \"%V\"", + is_client ? "client" : "upstream", name); + + return NGX_ERROR; + } + + if (is_client) { + r->invalid_header = 1; + } + } + + for (i = 0; i != value->len; i++) { + ch = value->data[i]; + + if (ch == '\0' || ch == LF || ch == CR) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "%s sent header \"%V\" with " + "invalid value: \"%V\"", + is_client ? "client" : "upstream", + name, value); + + return NGX_ERROR; + } + } + + return NGX_OK; +} + + +static inline ngx_int_t +ngx_isspace(u_char ch) +{ + return ch == ' ' || ch == '\t'; +} diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 49ea25ede..cb2c5047a 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -142,8 +142,6 @@ static ngx_http_v2_out_frame_t *ngx_http_v2_get_frame( static ngx_int_t ngx_http_v2_frame_handler(ngx_http_v2_connection_t *h2c, ngx_http_v2_out_frame_t *frame); -static ngx_int_t ngx_http_v2_validate_header(ngx_http_request_t *r, - ngx_http_v2_header_t *header); static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header); static ngx_int_t ngx_http_v2_parse_path(ngx_http_request_t *r, @@ -1775,7 +1773,8 @@ ngx_http_v2_state_process_header(ngx_http_v2_connection_t *h2c, u_char *pos, fc = r->connection; /* TODO Optimization: validate headers while parsing. */ - if (ngx_http_v2_validate_header(r, header) != NGX_OK) { + if (ngx_http_v23_validate_header(r, &header->name, &header->value, 1) + != NGX_OK) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); goto error; } @@ -3233,58 +3232,6 @@ ngx_http_v2_get_closed_node(ngx_http_v2_connection_t *h2c) } -static ngx_int_t -ngx_http_v2_validate_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) -{ - u_char ch; - ngx_uint_t i; - ngx_http_core_srv_conf_t *cscf; - - r->invalid_header = 0; - - cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); - - for (i = (header->name.data[0] == ':'); i != header->name.len; i++) { - ch = header->name.data[i]; - - if ((ch >= 'a' && ch <= 'z') - || (ch == '-') - || (ch >= '0' && ch <= '9') - || (ch == '_' && cscf->underscores_in_headers)) - { - continue; - } - - if (ch <= 0x20 || ch == 0x7f || ch == ':' - || (ch >= 'A' && ch <= 'Z')) - { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid header name: \"%V\"", - &header->name); - - return NGX_ERROR; - } - - r->invalid_header = 1; - } - - for (i = 0; i != header->value.len; i++) { - ch = header->value.data[i]; - - if (ch == '\0' || ch == LF || ch == CR) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent header \"%V\" with " - "invalid value: \"%V\"", - &header->name, &header->value); - - return NGX_ERROR; - } - } - - return NGX_OK; -} - - static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) { diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index 6865e1466..11159d029 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -17,8 +17,6 @@ static void ngx_http_v3_cleanup_request(void *data); static void ngx_http_v3_process_request(ngx_event_t *rev); static ngx_int_t ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value); -static ngx_int_t ngx_http_v3_validate_header(ngx_http_request_t *r, - ngx_str_t *name, ngx_str_t *value); static ngx_int_t ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value); static ngx_int_t ngx_http_v3_init_pseudo_headers(ngx_http_request_t *r); @@ -632,7 +630,7 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, r->v3_parse->header_limit -= len; - if (ngx_http_v3_validate_header(r, name, value) != NGX_OK) { + if (ngx_http_v23_validate_header(r, name, value, 1) != NGX_OK) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); return NGX_ERROR; } @@ -692,57 +690,6 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, } -static ngx_int_t -ngx_http_v3_validate_header(ngx_http_request_t *r, ngx_str_t *name, - ngx_str_t *value) -{ - u_char ch; - ngx_uint_t i; - ngx_http_core_srv_conf_t *cscf; - - r->invalid_header = 0; - - cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); - - for (i = (name->data[0] == ':'); i != name->len; i++) { - ch = name->data[i]; - - if ((ch >= 'a' && ch <= 'z') - || (ch == '-') - || (ch >= '0' && ch <= '9') - || (ch == '_' && cscf->underscores_in_headers)) - { - continue; - } - - if (ch <= 0x20 || ch == 0x7f || ch == ':' - || (ch >= 'A' && ch <= 'Z')) - { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid header name: \"%V\"", name); - - return NGX_ERROR; - } - - r->invalid_header = 1; - } - - for (i = 0; i != value->len; i++) { - ch = value->data[i]; - - if (ch == '\0' || ch == LF || ch == CR) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent header \"%V\" with " - "invalid value: \"%V\"", name, value); - - return NGX_ERROR; - } - } - - return NGX_OK; -} - - static ngx_int_t ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value) From 77dcd5de2e69a9b7ddd9f6d762c966799c4e3d52 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 7 Apr 2025 17:30:13 -0400 Subject: [PATCH 2/4] HTTP: Strip or reject leading or trailing HWS in field values All versions of HTTP forbid field (header and trailer) values from having leading or trailing horizontal whitespace (HWS, 0x20 and 0x09). In HTTP/1.0 and HTTP/1.1, leading and trailing whitespace must be stripped from the field value before further processing. In HTTP/2 and HTTP/3, leading and trailing whitespace must cause the entire message to be considered malformed. Willy Tarreau (lead developer of HAProxy) has indicated that there are clients that actually do send leading and/or trailing whitespace in HTTP/2 and/or HTTP/3 cookie headers, which is why HAProxy accepts them. Therefore, the default is to always strip leading and trailing whitespace, regardless of the HTTP version. Standards-conforming behavior can be obtained with the new reject_leading_trailing_whitespace directive. If one only wants to effect how client requests are processed, one can use reject_leading_trailing_whitespace_client. If one only wants to effect processing of upstream responses, one can use reject_leading_trailing_whitespace_upstream. Fixes: #187 Fixes: #598 --- src/http/ngx_http_core_module.c | 42 ++++++++++++++++++++++ src/http/ngx_http_core_module.h | 3 ++ src/http/ngx_http_parse.c | 3 ++ src/http/ngx_http_v23_common.c | 62 +++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+) diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index a2ff53f82..419a68f35 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -259,6 +259,29 @@ static ngx_command_t ngx_http_core_commands[] = { offsetof(ngx_http_core_srv_conf_t, ignore_invalid_headers), NULL }, + { ngx_string("reject_leading_trailing_whitespace"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_conf_set_flag_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_core_srv_conf_t, reject_leading_trailing_whitespace), + NULL }, + + { ngx_string("reject_leading_trailing_whitespace_client"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_conf_set_flag_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_core_srv_conf_t, + reject_leading_trailing_whitespace_client), + NULL }, + + { ngx_string("reject_leading_trailing_whitespace_upstream"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_conf_set_flag_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_core_srv_conf_t, + reject_leading_trailing_whitespace_upstream), + NULL }, + { ngx_string("merge_slashes"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, ngx_conf_set_flag_slot, @@ -3514,6 +3537,9 @@ ngx_http_core_create_srv_conf(ngx_conf_t *cf) cscf->ignore_invalid_headers = NGX_CONF_UNSET; cscf->merge_slashes = NGX_CONF_UNSET; cscf->underscores_in_headers = NGX_CONF_UNSET; + cscf->reject_leading_trailing_whitespace = NGX_CONF_UNSET; + cscf->reject_leading_trailing_whitespace_client = NGX_CONF_UNSET; + cscf->reject_leading_trailing_whitespace_upstream = NGX_CONF_UNSET; cscf->file_name = cf->conf_file->file.name.data; cscf->line = cf->conf_file->line; @@ -3560,6 +3586,22 @@ ngx_http_core_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_value(conf->underscores_in_headers, prev->underscores_in_headers, 0); + if (conf->reject_leading_trailing_whitespace_client == NGX_CONF_UNSET) { + conf->reject_leading_trailing_whitespace_client = ( + conf->reject_leading_trailing_whitespace != NGX_CONF_UNSET ? + conf->reject_leading_trailing_whitespace : + (prev->reject_leading_trailing_whitespace_client != NGX_CONF_UNSET ? + prev->reject_leading_trailing_whitespace_client : 0)); + } + + if (conf->reject_leading_trailing_whitespace_upstream == NGX_CONF_UNSET) { + conf->reject_leading_trailing_whitespace_upstream = ( + conf->reject_leading_trailing_whitespace != NGX_CONF_UNSET ? + conf->reject_leading_trailing_whitespace : + (prev->reject_leading_trailing_whitespace_upstream != NGX_CONF_UNSET ? + prev->reject_leading_trailing_whitespace_upstream : 0)); + } + if (conf->server_names.nelts == 0) { /* the array has 4 empty preallocated elements, so push cannot fail */ sn = ngx_array_push(&conf->server_names); diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h index 6062d3a23..b288d0886 100644 --- a/src/http/ngx_http_core_module.h +++ b/src/http/ngx_http_core_module.h @@ -210,6 +210,9 @@ typedef struct { unsigned allow_connect:1; ngx_http_core_loc_conf_t **named_locations; + ngx_flag_t reject_leading_trailing_whitespace; + ngx_flag_t reject_leading_trailing_whitespace_client; + ngx_flag_t reject_leading_trailing_whitespace_upstream; } ngx_http_core_srv_conf_t; diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index 81f689e5b..51f26e8de 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -1013,6 +1013,7 @@ ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b, case sw_space_before_value: switch (ch) { case ' ': + case '\t': break; case CR: r->header_start = p; @@ -1037,6 +1038,7 @@ ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b, case sw_value: switch (ch) { case ' ': + case '\t': r->header_end = p; state = sw_space_after_value; break; @@ -1057,6 +1059,7 @@ ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b, case sw_space_after_value: switch (ch) { case ' ': + case '\t': break; case CR: state = sw_almost_done; diff --git a/src/http/ngx_http_v23_common.c b/src/http/ngx_http_v23_common.c index fe009919a..81d167949 100644 --- a/src/http/ngx_http_v23_common.c +++ b/src/http/ngx_http_v23_common.c @@ -17,6 +17,7 @@ ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value, ngx_int_t is_client) { u_char ch; + ngx_str_t tmp; ngx_uint_t i; ngx_http_core_srv_conf_t *cscf; @@ -61,6 +62,11 @@ ngx_http_v23_validate_header(ngx_http_request_t *r, } } + /* Keep subsequent code from having to special-case empty strings. */ + if (value->len == 0) { + return NGX_OK; + } + for (i = 0; i != value->len; i++) { ch = value->data[i]; @@ -75,6 +81,62 @@ ngx_http_v23_validate_header(ngx_http_request_t *r, } } + if (!ngx_isspace(value->data[0]) + && !ngx_isspace(value->data[value->len - 1])) { + /* Fast path: nothing to strip. */ + return NGX_OK; + } + + if (is_client ? cscf->reject_leading_trailing_whitespace_client + : cscf->reject_leading_trailing_whitespace_upstream) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "%s sent header \"%V\" with " + "leading or trailing space", + is_client ? "client" : "upstream", name); + + return NGX_ERROR; + } + + tmp = *value; + + /* + * Strip trailing whitespace. Do this first so that + * if the string is all whitespace, tmp.data is not a + * past-the-end pointer, which cannot be safely passed + * to memmove(). After the loop, the string is either + * empty or ends with a non-whitespace character. + */ + while (tmp.len && ngx_isspace(tmp.data[tmp.len - 1])) { + tmp.len--; + } + + /* Strip leading whitespace */ + if (tmp.len && ngx_isspace(tmp.data[0])) { + /* + * Last loop guaranteed that 'tmp' does not end with whitespace, and + * this check guarantees it is not empty and starts with whitespace. + * Therefore, 'tmp' must end with a non-whitespace character, and must + * be of length at least 2. This means that it is safe to keep going + * until a non-whitespace character is found. + */ + do { + tmp.len--; + tmp.data++; + } while (ngx_isspace(tmp.data[0])); + + /* Move remaining string to start of buffer. */ + memmove(value->data, tmp.data, tmp.len); + } + + /* + * NUL-pad the data, so that if it was NUL-terminated before, it stil is. + * At least one byte will have been stripped, so value->data + tmp.len + * is not a past-the-end pointer. + */ + memset(value->data + tmp.len, '\0', value->len - tmp.len); + + /* Fix up length and return. */ + value->len = tmp.len; return NGX_OK; } From cf2a889b3920535895df3ed120e62a1a04577876 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 29 Sep 2025 13:47:08 -0400 Subject: [PATCH 3/4] HTTP: Use common validation for :method pseudo-headers The code was duplicated between HTTP/2 and HTTP/3. No functional change intended. --- src/http/ngx_http.h | 5 ++ src/http/ngx_http_parse.c | 98 +++++++++++++++++++++++++++++ src/http/v2/ngx_http_v2.c | 100 +----------------------------- src/http/v3/ngx_http_v3_request.c | 60 +----------------- 4 files changed, 105 insertions(+), 158 deletions(-) diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index 92a626f22..3bdfbe160 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -186,6 +186,11 @@ size_t ngx_http_huff_encode(u_char *src, size_t len, u_char *dst, ngx_uint_t lower); ngx_int_t ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value, ngx_int_t is_client); +/* + * Parse an HTTP/2 and/or HTTP/3 method. + */ +ngx_int_t +ngx_http_v23_parse_method(ngx_http_request_t *r, ngx_str_t *value); #endif diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index 51f26e8de..b4e8616d8 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -1134,6 +1134,104 @@ header_done: } +#if (NGX_HTTP_V2 || NGX_HTTP_V3) + +ngx_int_t +ngx_http_v23_parse_method(ngx_http_request_t *r, ngx_str_t *value) +{ + size_t k, len; + ngx_uint_t n; + const u_char *p, *m; + + /* + * This array takes less than 256 sequential bytes, + * and if typical CPU cache line size is 64 bytes, + * it is prefetched for 4 load operations. + */ + static const struct { + u_char len; + const u_char method[11]; + uint32_t value; + } tests[] = { + { 3, "GET", NGX_HTTP_GET }, + { 4, "POST", NGX_HTTP_POST }, + { 4, "HEAD", NGX_HTTP_HEAD }, + { 7, "OPTIONS", NGX_HTTP_OPTIONS }, + { 8, "PROPFIND", NGX_HTTP_PROPFIND }, + { 3, "PUT", NGX_HTTP_PUT }, + { 5, "MKCOL", NGX_HTTP_MKCOL }, + { 6, "DELETE", NGX_HTTP_DELETE }, + { 4, "COPY", NGX_HTTP_COPY }, + { 4, "MOVE", NGX_HTTP_MOVE }, + { 9, "PROPPATCH", NGX_HTTP_PROPPATCH }, + { 4, "LOCK", NGX_HTTP_LOCK }, + { 6, "UNLOCK", NGX_HTTP_UNLOCK }, + { 5, "PATCH", NGX_HTTP_PATCH }, + { 5, "TRACE", NGX_HTTP_TRACE }, + { 7, "CONNECT", NGX_HTTP_CONNECT } + }, *test; + + if (r->method_name.len) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent duplicate :method header"); + + return NGX_DECLINED; + } + + if (value->len == 0) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent empty :method header"); + + return NGX_DECLINED; + } + + r->method_name.len = value->len; + r->method_name.data = value->data; + + len = r->method_name.len; + n = sizeof(tests) / sizeof(tests[0]); + test = tests; + + do { + if (len == test->len) { + p = r->method_name.data; + m = test->method; + k = len; + + do { + if (*p++ != *m++) { + goto next; + } + } while (--k); + + r->method = test->value; + return NGX_OK; + } + + next: + test++; + + } while (--n); + + p = r->method_name.data; + + do { + if ((*p < 'A' || *p > 'Z') && *p != '_' && *p != '-') { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent invalid method: \"%V\"", + &r->method_name); + + return NGX_DECLINED; + } + + p++; + + } while (--len); + + return NGX_OK; +} + + ngx_int_t ngx_http_parse_uri(ngx_http_request_t *r) { diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index cb2c5047a..813702308 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -146,8 +146,6 @@ static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header); static ngx_int_t ngx_http_v2_parse_path(ngx_http_request_t *r, ngx_str_t *value); -static ngx_int_t ngx_http_v2_parse_method(ngx_http_request_t *r, - ngx_str_t *value); static ngx_int_t ngx_http_v2_parse_scheme(ngx_http_request_t *r, ngx_str_t *value); static ngx_int_t ngx_http_v2_parse_authority(ngx_http_request_t *r, @@ -3252,7 +3250,7 @@ ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) if (ngx_memcmp(header->name.data, "method", sizeof("method") - 1) == 0) { - return ngx_http_v2_parse_method(r, &header->value); + return ngx_http_v23_parse_method(r, &header->value); } if (ngx_memcmp(header->name.data, "scheme", sizeof("scheme") - 1) @@ -3320,102 +3318,6 @@ ngx_http_v2_parse_path(ngx_http_request_t *r, ngx_str_t *value) } -static ngx_int_t -ngx_http_v2_parse_method(ngx_http_request_t *r, ngx_str_t *value) -{ - size_t k, len; - ngx_uint_t n; - const u_char *p, *m; - - /* - * This array takes less than 256 sequential bytes, - * and if typical CPU cache line size is 64 bytes, - * it is prefetched for 4 load operations. - */ - static const struct { - u_char len; - const u_char method[11]; - uint32_t value; - } tests[] = { - { 3, "GET", NGX_HTTP_GET }, - { 4, "POST", NGX_HTTP_POST }, - { 4, "HEAD", NGX_HTTP_HEAD }, - { 7, "OPTIONS", NGX_HTTP_OPTIONS }, - { 8, "PROPFIND", NGX_HTTP_PROPFIND }, - { 3, "PUT", NGX_HTTP_PUT }, - { 5, "MKCOL", NGX_HTTP_MKCOL }, - { 6, "DELETE", NGX_HTTP_DELETE }, - { 4, "COPY", NGX_HTTP_COPY }, - { 4, "MOVE", NGX_HTTP_MOVE }, - { 9, "PROPPATCH", NGX_HTTP_PROPPATCH }, - { 4, "LOCK", NGX_HTTP_LOCK }, - { 6, "UNLOCK", NGX_HTTP_UNLOCK }, - { 5, "PATCH", NGX_HTTP_PATCH }, - { 5, "TRACE", NGX_HTTP_TRACE }, - { 7, "CONNECT", NGX_HTTP_CONNECT } - }, *test; - - if (r->method_name.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :method header"); - - return NGX_DECLINED; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :method header"); - - return NGX_DECLINED; - } - - r->method_name.len = value->len; - r->method_name.data = value->data; - - len = r->method_name.len; - n = sizeof(tests) / sizeof(tests[0]); - test = tests; - - do { - if (len == test->len) { - p = r->method_name.data; - m = test->method; - k = len; - - do { - if (*p++ != *m++) { - goto next; - } - } while (--k); - - r->method = test->value; - return NGX_OK; - } - - next: - test++; - - } while (--n); - - p = r->method_name.data; - - do { - if ((*p < 'A' || *p > 'Z') && *p != '_' && *p != '-') { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid method: \"%V\"", - &r->method_name); - - return NGX_DECLINED; - } - - p++; - - } while (--len); - - return NGX_OK; -} - - static ngx_int_t ngx_http_v2_parse_scheme(ngx_http_request_t *r, ngx_str_t *value) { diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index 11159d029..4b58e36b5 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -29,30 +29,6 @@ static ngx_int_t ngx_http_v3_request_body_filter(ngx_http_request_t *r, ngx_chain_t *in); -static const struct { - ngx_str_t name; - ngx_uint_t method; -} ngx_http_v3_methods[] = { - - { ngx_string("GET"), NGX_HTTP_GET }, - { ngx_string("POST"), NGX_HTTP_POST }, - { ngx_string("HEAD"), NGX_HTTP_HEAD }, - { ngx_string("OPTIONS"), NGX_HTTP_OPTIONS }, - { ngx_string("PROPFIND"), NGX_HTTP_PROPFIND }, - { ngx_string("PUT"), NGX_HTTP_PUT }, - { ngx_string("MKCOL"), NGX_HTTP_MKCOL }, - { ngx_string("DELETE"), NGX_HTTP_DELETE }, - { ngx_string("COPY"), NGX_HTTP_COPY }, - { ngx_string("MOVE"), NGX_HTTP_MOVE }, - { ngx_string("PROPPATCH"), NGX_HTTP_PROPPATCH }, - { ngx_string("LOCK"), NGX_HTTP_LOCK }, - { ngx_string("UNLOCK"), NGX_HTTP_UNLOCK }, - { ngx_string("PATCH"), NGX_HTTP_PATCH }, - { ngx_string("TRACE"), NGX_HTTP_TRACE }, - { ngx_string("CONNECT"), NGX_HTTP_CONNECT } -}; - - void ngx_http_v3_init_stream(ngx_connection_t *c) { @@ -704,44 +680,10 @@ ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, } if (name->len == 7 && ngx_strncmp(name->data, ":method", 7) == 0) { - - if (r->method_name.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate \":method\" header"); + if (ngx_http_v23_parse_method(r, value) != NGX_OK) { goto failed; } - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty \":method\" header"); - goto failed; - } - - r->method_name = *value; - - for (i = 0; i < sizeof(ngx_http_v3_methods) - / sizeof(ngx_http_v3_methods[0]); i++) - { - if (value->len == ngx_http_v3_methods[i].name.len - && ngx_strncmp(value->data, - ngx_http_v3_methods[i].name.data, value->len) - == 0) - { - r->method = ngx_http_v3_methods[i].method; - break; - } - } - - for (i = 0; i < value->len; i++) { - ch = value->data[i]; - - if ((ch < 'A' || ch > 'Z') && ch != '_' && ch != '-') { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid method: \"%V\"", value); - goto failed; - } - } - ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "http3 method \"%V\" %ui", value, r->method); return NGX_OK; From 400d8d1b06f6f2306ee8b0299750fba59a8d647f Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 29 Sep 2025 13:57:11 -0400 Subject: [PATCH 4/4] HTTP: Use common validation for :scheme pseudo-headers The code was duplicated between HTTP/2 and HTTP/3. No functional change intended. --- src/http/ngx_http.h | 6 ++-- src/http/ngx_http_parse.c | 46 +++++++++++++++++++++++++++++ src/http/v2/ngx_http_v2.c | 49 +------------------------------ src/http/v3/ngx_http_v3_request.c | 36 +---------------------- 4 files changed, 52 insertions(+), 85 deletions(-) diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index 3bdfbe160..d971a815a 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -189,8 +189,10 @@ ngx_int_t ngx_http_v23_validate_header(ngx_http_request_t *r, /* * Parse an HTTP/2 and/or HTTP/3 method. */ -ngx_int_t -ngx_http_v23_parse_method(ngx_http_request_t *r, ngx_str_t *value); +ngx_int_t ngx_http_v23_parse_method(ngx_http_request_t *r, + ngx_str_t *value); +ngx_int_t ngx_http_v23_parse_scheme(ngx_http_request_t *r, + ngx_str_t *value); #endif diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index b4e8616d8..830b99381 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -1232,6 +1232,52 @@ ngx_http_v23_parse_method(ngx_http_request_t *r, ngx_str_t *value) } +ngx_int_t +ngx_http_v23_parse_scheme(ngx_http_request_t *r, ngx_str_t *value) +{ + u_char c, ch; + ngx_uint_t i; + + if (r->schema.len) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent duplicate :scheme header"); + + return NGX_DECLINED; + } + + if (value->len == 0) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent empty :scheme header"); + + return NGX_DECLINED; + } + + for (i = 0; i < value->len; i++) { + ch = value->data[i]; + + c = (u_char) (ch | 0x20); + if (c >= 'a' && c <= 'z') { + continue; + } + + if (((ch >= '0' && ch <= '9') || ch == '+' || ch == '-' || ch == '.') + && i > 0) + { + continue; + } + + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent invalid :scheme header: \"%V\"", value); + + return NGX_DECLINED; + } + + r->schema = *value; + + return NGX_OK; +} + + ngx_int_t ngx_http_parse_uri(ngx_http_request_t *r) { diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 813702308..acd486082 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -146,8 +146,6 @@ static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header); static ngx_int_t ngx_http_v2_parse_path(ngx_http_request_t *r, ngx_str_t *value); -static ngx_int_t ngx_http_v2_parse_scheme(ngx_http_request_t *r, - ngx_str_t *value); static ngx_int_t ngx_http_v2_parse_authority(ngx_http_request_t *r, ngx_str_t *value); static ngx_int_t ngx_http_v2_construct_request_line(ngx_http_request_t *r); @@ -3256,7 +3254,7 @@ ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) if (ngx_memcmp(header->name.data, "scheme", sizeof("scheme") - 1) == 0) { - return ngx_http_v2_parse_scheme(r, &header->value); + return ngx_http_v23_parse_scheme(r, &header->value); } break; @@ -3318,51 +3316,6 @@ ngx_http_v2_parse_path(ngx_http_request_t *r, ngx_str_t *value) } -static ngx_int_t -ngx_http_v2_parse_scheme(ngx_http_request_t *r, ngx_str_t *value) -{ - u_char c, ch; - ngx_uint_t i; - - if (r->schema.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :scheme header"); - - return NGX_DECLINED; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :scheme header"); - - return NGX_DECLINED; - } - - for (i = 0; i < value->len; i++) { - ch = value->data[i]; - - c = (u_char) (ch | 0x20); - if (c >= 'a' && c <= 'z') { - continue; - } - - if (((ch >= '0' && ch <= '9') || ch == '+' || ch == '-' || ch == '.') - && i > 0) - { - continue; - } - - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid :scheme header: \"%V\"", value); - - return NGX_DECLINED; - } - - r->schema = *value; - - return NGX_OK; -} - static ngx_int_t ngx_http_v2_parse_authority(ngx_http_request_t *r, ngx_str_t *value) diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index 4b58e36b5..a40fc157b 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -670,9 +670,6 @@ static ngx_int_t ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value) { - u_char ch, c; - ngx_uint_t i; - if (r->request_line.len) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "client sent out of order pseudo-headers"); @@ -720,41 +717,10 @@ ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, if (name->len == 7 && ngx_strncmp(name->data, ":scheme", 7) == 0) { - if (r->schema.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate \":scheme\" header"); + if (ngx_http_v23_parse_scheme(r, value) != NGX_OK) { goto failed; } - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty \":scheme\" header"); - goto failed; - } - - for (i = 0; i < value->len; i++) { - ch = value->data[i]; - - c = (u_char) (ch | 0x20); - if (c >= 'a' && c <= 'z') { - continue; - } - - if (((ch >= '0' && ch <= '9') - || ch == '+' || ch == '-' || ch == '.') - && i > 0) - { - continue; - } - - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid \":scheme\" header: \"%V\"", - value); - goto failed; - } - - r->schema = *value; - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "http3 schema \"%V\"", value); return NGX_OK;