diff --git a/doc/Makefile.am b/doc/Makefile.am index a1a177e8..61e182cd 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -30,6 +30,7 @@ APIDOCS= \ nghttp2_check_authority.rst \ nghttp2_check_header_name.rst \ nghttp2_check_header_value.rst \ + nghttp2_check_header_value_rfc9113.rst \ nghttp2_check_method.rst \ nghttp2_check_path.rst \ nghttp2_hd_deflate_bound.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 2e87c481..bb8aa1d3 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -5046,9 +5046,23 @@ NGHTTP2_EXTERN int nghttp2_check_header_name(const uint8_t *name, size_t len); * Returns nonzero if HTTP header field value |value| of length |len| * is valid according to * http://tools.ietf.org/html/rfc7230#section-3.2 + * + * This function is considered obsolete, and application should + * consider to use `nghttp2_check_header_value_rfc9113()` instead. */ NGHTTP2_EXTERN int nghttp2_check_header_value(const uint8_t *value, size_t len); +/** + * @function + * + * Returns nonzero if HTTP header field value |value| of length |len| + * is valid according to + * http://tools.ietf.org/html/rfc7230#section-3.2, plus + * https://datatracker.ietf.org/doc/html/rfc9113#section-8.2.1 + */ +NGHTTP2_EXTERN int nghttp2_check_header_value_rfc9113(const uint8_t *value, + size_t len); + /** * @function * diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c index 588e269c..93dd4754 100644 --- a/lib/nghttp2_helper.c +++ b/lib/nghttp2_helper.c @@ -507,6 +507,19 @@ int nghttp2_check_header_value(const uint8_t *value, size_t len) { return 1; } +int nghttp2_check_header_value_rfc9113(const uint8_t *value, size_t len) { + if (len == 0) { + return 1; + } + + if (*value == ' ' || *value == '\t' || *(value + len - 1) == ' ' || + *(value + len - 1) == '\t') { + return 0; + } + + return nghttp2_check_header_value(value, len); +} + /* Generated by genmethodchartbl.py */ static char VALID_METHOD_CHARS[] = { 0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */, diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index e41cbf68..0ce3631f 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -392,14 +392,14 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, if (session->server || frame->hd.type == NGHTTP2_PUSH_PROMISE) { rv = nghttp2_check_authority(nv->value->base, nv->value->len); } else { - rv = nghttp2_check_header_value(nv->value->base, nv->value->len); + rv = nghttp2_check_header_value_rfc9113(nv->value->base, nv->value->len); } break; case NGHTTP2_TOKEN__SCHEME: rv = check_scheme(nv->value->base, nv->value->len); break; default: - rv = nghttp2_check_header_value(nv->value->base, nv->value->len); + rv = nghttp2_check_header_value_rfc9113(nv->value->base, nv->value->len); } if (rv == 0) { diff --git a/src/http2.cc b/src/http2.cc index 64d700f0..080bb731 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -693,17 +693,6 @@ StringRef rewrite_location_uri(BlockAllocator &balloc, const StringRef &uri, return StringRef{iov.base, p}; } -int check_nv(const uint8_t *name, size_t namelen, const uint8_t *value, - size_t valuelen) { - if (!nghttp2_check_header_name(name, namelen)) { - return 0; - } - if (!nghttp2_check_header_value(value, valuelen)) { - return 0; - } - return 1; -} - int parse_http_status_code(const StringRef &src) { if (src.size() != 3) { return -1; diff --git a/src/http2.h b/src/http2.h index f5c07136..fc88e4dd 100644 --- a/src/http2.h +++ b/src/http2.h @@ -289,12 +289,6 @@ StringRef rewrite_location_uri(BlockAllocator &balloc, const StringRef &uri, const StringRef &request_authority, const StringRef &upstream_scheme); -// Checks the header name/value pair using nghttp2_check_header_name() -// and nghttp2_check_header_value(). If both function returns nonzero, -// this function returns nonzero. -int check_nv(const uint8_t *name, size_t namelen, const uint8_t *value, - size_t valuelen); - // Returns parsed HTTP status code. Returns -1 on failure. int parse_http_status_code(const StringRef &src); diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 8af7f52e..25bf4e48 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -379,7 +379,7 @@ HeaderRefs::value_type parse_header(BlockAllocator &balloc, make_string_ref(balloc, StringRef{value, std::end(optarg)})); if (!nghttp2_check_header_name(nv.name.byte(), nv.name.size()) || - !nghttp2_check_header_value(nv.value.byte(), nv.value.size())) { + !nghttp2_check_header_value_rfc9113(nv.value.byte(), nv.value.size())) { return {}; } diff --git a/tests/main.c b/tests/main.c index ed26a80a..bde299a5 100644 --- a/tests/main.c +++ b/tests/main.c @@ -431,6 +431,8 @@ int main(void) { test_nghttp2_check_header_name) || !CU_add_test(pSuite, "check_header_value", test_nghttp2_check_header_value) || + !CU_add_test(pSuite, "check_header_value_rfc9113", + test_nghttp2_check_header_value_rfc9113) || !CU_add_test(pSuite, "bufs_add", test_nghttp2_bufs_add) || !CU_add_test(pSuite, "bufs_add_stack_buffer_overflow_bug", test_nghttp2_bufs_add_stack_buffer_overflow_bug) || diff --git a/tests/nghttp2_helper_test.c b/tests/nghttp2_helper_test.c index b29e67b9..ca91bb6a 100644 --- a/tests/nghttp2_helper_test.c +++ b/tests/nghttp2_helper_test.c @@ -166,4 +166,28 @@ void test_nghttp2_check_header_value(void) { CU_ASSERT(check_header_value(goodval)); CU_ASSERT(!check_header_value(badval1)); CU_ASSERT(!check_header_value(badval2)); + CU_ASSERT(check_header_value("")); + CU_ASSERT(check_header_value(" ")); + CU_ASSERT(check_header_value("\t")); +} + +#define check_header_value_rfc9113(S) \ + nghttp2_check_header_value_rfc9113((const uint8_t *)S, sizeof(S) - 1) + +void test_nghttp2_check_header_value_rfc9113(void) { + uint8_t goodval[] = {'a', 'b', 0x80u, 'c', 0xffu, 'd'}; + uint8_t badval1[] = {'a', 0x1fu, 'b'}; + uint8_t badval2[] = {'a', 0x7fu, 'b'}; + + CU_ASSERT(check_header_value_rfc9113("!|}~")); + CU_ASSERT(!check_header_value_rfc9113(" !|}~")); + CU_ASSERT(!check_header_value_rfc9113("!|}~ ")); + CU_ASSERT(!check_header_value_rfc9113("\t!|}~")); + CU_ASSERT(!check_header_value_rfc9113("!|}~\t")); + CU_ASSERT(check_header_value_rfc9113(goodval)); + CU_ASSERT(!check_header_value_rfc9113(badval1)); + CU_ASSERT(!check_header_value_rfc9113(badval2)); + CU_ASSERT(check_header_value_rfc9113("")); + CU_ASSERT(!check_header_value_rfc9113(" ")); + CU_ASSERT(!check_header_value_rfc9113("\t")); } diff --git a/tests/nghttp2_helper_test.h b/tests/nghttp2_helper_test.h index cca8122a..8790dcf6 100644 --- a/tests/nghttp2_helper_test.h +++ b/tests/nghttp2_helper_test.h @@ -32,5 +32,6 @@ void test_nghttp2_adjust_local_window_size(void); void test_nghttp2_check_header_name(void); void test_nghttp2_check_header_value(void); +void test_nghttp2_check_header_value_rfc9113(void); #endif /* NGHTTP2_HELPER_TEST_H */