Skip http method matching in http_parser with UHV

bazel/external/http_parser/http_parser.c

@@ -161,6 +161,31 @@ do { \
  } \
 } while (0)
 
+/* Macro to check if the method parsing has reached its end.
+* - In strict mode (`HTTP_PARSER_STRICT`), it checks if the character at
+* the current `index` in the `matcher` string is the null terminator ('\0'),
+* indicating the end of the method string.
+* - In non-strict mode, it always returns `true`, allowing custom methods to bypass
+* strict validation and when UHV is enabled, HTTP_PARSER_STRICT is disabled.
+*/
+#define IS_METHOD_END(matcher, index) \
+ (HTTP_PARSER_STRICT ? (matcher[index] == '\0') : true)
+
+/* Macro to handle invalid HTTP methods in the parser.
+* - In strict mode (`HTTP_PARSER_STRICT`), it sets the error to `HPE_INVALID_METHOD`
+* and jumps to the error handling section.
+* - In non-strict mode, it assigns `HTTP_GET` as the default method (value 1).
+* This bypasses method validation for custom or unknown HTTP methods.
+*/
+#define HANDLE_INVALID_METHOD(parser) \
+do { \
+ if (HTTP_PARSER_STRICT) { \
+ SET_ERRNO(HPE_INVALID_METHOD); \
+ goto error; \
+ } else { \
+ (parser)->method = 1; \
+ } \
+} while (0)
 
 #define PROXY_CONNECTION "proxy-connection"
 #define CONNECTION "connection"
@@ -960,8 +985,8 @@ size_t http_parser_execute (http_parser *parser,
  case 'T': parser->method = HTTP_TRACE; break;
  case 'U': parser->method = HTTP_UNLOCK; /* or UNSUBSCRIBE, UNBIND, UNLINK */ break;
  default:
- SET_ERRNO(HPE_INVALID_METHOD);
- goto error;
+ HANDLE_INVALID_METHOD(parser);
+ break;
  }
  UPDATE_STATE(s_req_method);
 
@@ -972,15 +997,19 @@ size_t http_parser_execute (http_parser *parser,
 
  case s_req_method:
  {
- const char *matcher;
  if (UNLIKELY(ch == '\0')) {
  SET_ERRNO(HPE_INVALID_METHOD);
  goto error;
  }
 
+ const char *matcher;
  matcher = method_strings[parser->method];
- if (ch == ' ' && matcher[parser->index] == '\0') {
+ if (ch == ' ' && IS_METHOD_END(matcher, parser->index)) {
  UPDATE_STATE(s_req_spaces_before_url);
+ if (matcher[parser->index] != '\0') {
+ /*Default to GET if none of the existing HTTP methods match*/
+ parser->method = 1;
+ }
  } else if (ch == matcher[parser->index]) {
  ; /* nada */
  } else if ((ch >= 'A' && ch <= 'Z') || ch == '-') {
@@ -1011,8 +1040,8 @@ size_t http_parser_execute (http_parser *parser,
  XX(UNLOCK, 3, 'I', UNLINK)
 #undef XX
  default:
- SET_ERRNO(HPE_INVALID_METHOD);
- goto error;
+ HANDLE_INVALID_METHOD(parser);
+ break;
  }
  } else {
  SET_ERRNO(HPE_INVALID_METHOD);

test/common/http/http1/codec_impl_test.cc

@@ -1266,6 +1266,12 @@ TEST_P(Http1ServerConnectionImplTest, BadRequestNoStream) {
 }
 
 TEST_P(Http1ServerConnectionImplTest, RejectCustomMethod) {
+#ifdef ENVOY_ENABLE_UHV
+ // When UHV flag is enabled, strict method matching are ignored
+ // and all the custom methods are allowed by default.
+ return;
+#endif
+
  initialize();
 
  MockRequestDecoder decoder;
@@ -1295,9 +1301,13 @@ TEST_P(Http1ServerConnectionImplTest, RejectInvalidCharacterInMethod) {
 }
 
 TEST_P(Http1ServerConnectionImplTest, AllowCustomMethod) {
+
+#ifndef ENVOY_ENABLE_UHV
+ // If UHV is enabled HttpParser allows custom methods
  if (parser_impl_ == Http1ParserImpl::HttpParser) {
  return;
  }
+#endif
 
  codec_settings_.allow_custom_methods_ = true;
  initialize();
@@ -1309,13 +1319,15 @@ TEST_P(Http1ServerConnectionImplTest, AllowCustomMethod) {
  auto status = codec_->dispatch(buffer);
  ASSERT_TRUE(status.ok());
 
- TestRequestHeaderMapImpl expected_headers{
- {":authority", "example.com"}, {":path", "/"}, {":method", "BAD"}, {"foo", "bar"}};
- EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), true));
+ if (parser_impl_ == Http1ParserImpl::BalsaParser) {
+ TestRequestHeaderMapImpl expected_headers{
+ {":authority", "example.com"}, {":path", "/"}, {":method", "BAD"}, {"foo", "bar"}};
+ EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), true));
 
- Buffer::OwnedImpl headers("host: example.com\r\nfoo: bar\r\n\r\n");
- status = codec_->dispatch(headers);
- EXPECT_TRUE(status.ok());
+ Buffer::OwnedImpl headers("host: example.com\r\nfoo: bar\r\n\r\n");
+ status = codec_->dispatch(headers);
+ EXPECT_TRUE(status.ok());
+ }
 }
 
 TEST_P(Http1ServerConnectionImplTest, BadRequestStartedStream) {
@@ -4197,10 +4209,8 @@ TEST_P(Http1ServerConnectionImplTest, ValidMethodFirstCharacter) {
 // Receiving a first byte that cannot start a valid method name is an error.
 TEST_P(Http1ServerConnectionImplTest, InvalidMethodFirstCharacter) {
 #ifdef ENVOY_ENABLE_UHV
- if (parser_impl_ == Http1ParserImpl::BalsaParser) {
- // BalsaParser allows custom methods if UHV is enabled.
- return;
- }
+ // When UHV flag is enabled both BalsaParser and HttpParser allow custom methods
+ return;
 #endif
 
  initialize();

test/extensions/filters/listener/http_inspector/http_inspector_test.cc

@@ -397,13 +397,23 @@ TEST_F(HttpInspectorTest, ExtraSpaceInRequestLine) {
 }
 
 TEST_F(HttpInspectorTest, InvalidHttpMethod) {
+#ifdef ENVOY_ENABLE_UHV
+ // enabling UHV skips method name checks
+ return;
+#endif
+
  const absl::string_view header = "BAD /anything HTTP/1.1";
  testHttpInspectNotFound(header);
 }
 
 TEST_F(HttpInspectorTest, InvalidHttpRequestLine) {
  const absl::string_view header = "BAD /anything HTTP/1.1\r\n";
+#ifdef ENVOY_ENABLE_UHV
+ // UHV allows custom methods
+ testHttpInspectFound(header, Http::Utility::AlpnNames::get().Http11);
+#else
  testHttpInspectNotFound(header);
+#endif
 }
 
 TEST_F(HttpInspectorTest, OldHttpProtocol) {
@@ -484,6 +494,10 @@ TEST_F(HttpInspectorTest, MultipleReadsHttp2) {
 }
 
 TEST_F(HttpInspectorTest, MultipleReadsHttp2BadPreface) {
+#ifdef ENVOY_ENABLE_UHV
+ return;
+#endif
+
  const std::string header = "505249202a20485454502f322e300d0a0d0c";
  testHttpInspectMultipleReadsNotFound(header, true);
 }
@@ -494,6 +508,10 @@ TEST_F(HttpInspectorTest, MultipleReadsHttp1) {
 }
 
 TEST_F(HttpInspectorTest, MultipleReadsHttp1IncompleteBadHeader) {
+#ifdef ENVOY_ENABLE_UHV
+ return;
+#endif
+
  const absl::string_view data = "X";
  testHttpInspectMultipleReadsNotFound(data);
 }