From 01d98c11174d52593660de14682254683d6df24e Mon Sep 17 00:00:00 2001 From: Adam Meily Date: Mon, 18 Apr 2022 11:03:48 -0400 Subject: [PATCH 1/5] add permissive parsing for H1 codec baed on uhv_enabled config Signed-off-by: Adam Meily --- bazel/external/http_parser/BUILD | 5 +++- test/common/http/http1/codec_impl_test.cc | 28 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/bazel/external/http_parser/BUILD b/bazel/external/http_parser/BUILD index 5fefacde47dc..bb50c4adeb9d 100644 --- a/bazel/external/http_parser/BUILD +++ b/bazel/external/http_parser/BUILD @@ -12,7 +12,10 @@ cc_library( # This compiler flag is set to an arbtitrarily high number so # as to effectively disables the http_parser header limit, as # we do our own checks in the conn manager and codec. - copts = ["-DHTTP_MAX_HEADER_SIZE=0x2000000"], + copts = ["-DHTTP_MAX_HEADER_SIZE=0x2000000"] + select({ + "@envoy//bazel:uhv_enabled": ["-DHTTP_PARSER_STRICT=0"], + "//conditions:default": [], + }), includes = ["."], visibility = ["//visibility:public"], ) diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 59ced124d59c..e22fbc6fe745 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -3149,6 +3149,34 @@ TEST_F(Http1ServerConnectionImplTest, PipedRequestWithMutipleEvent) { connection_.dispatcher_.clearDeferredDeleteList(); } +TEST_F(Http1ServerConnectionImplTest, Utf8Path) { + initialize(); + + MockRequestDecoder decoder; + Buffer::OwnedImpl buffer("GET /δ¶/δt/pope?q=1#narf HXXP/1.1\r\n\r\n"); +#ifdef ENVOY_ENABLE_UHV + // permissive + EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); + + TestRequestHeaderMapImpl expected_headers{ + {":path", "/δ¶/δt/pope?q=1#narf"}, + {":method", "GET"}, + }; + EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); + + auto status = codec_->dispatch(buffer); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(0U, buffer.length()); +#else + // strict + EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); + + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(isCodecProtocolError(status)); +#endif +} + // Tests that incomplete response headers of 80 kB header value fails. TEST_F(Http1ClientConnectionImplTest, ResponseHeadersWithLargeValueRejected) { initialize(); From 1a2407354615a0b5b027587baedab8bfd0788bce Mon Sep 17 00:00:00 2001 From: Adam Meily Date: Mon, 18 Apr 2022 11:29:35 -0400 Subject: [PATCH 2/5] fix unit tests forwhen UHV is enabled Signed-off-by: Adam Meily --- test/common/http/http1/codec_impl_test.cc | 2 +- test/integration/protocol_integration_test.cc | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index e22fbc6fe745..a267c529a8c0 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -3165,7 +3165,7 @@ TEST_F(Http1ServerConnectionImplTest, Utf8Path) { EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); auto status = codec_->dispatch(buffer); - EXPECT_FALSE(status.ok()); + EXPECT_TRUE(status.ok()); EXPECT_EQ(0U, buffer.length()); #else // strict diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 89bf7268ba51..d3ae35134d1f 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -3468,6 +3468,11 @@ TEST_P(ProtocolIntegrationTest, UpstreamDisconnectBeforeResponseCompleteWireByte TEST_P(DownstreamProtocolIntegrationTest, BadRequest) { config_helper_.disableDelayClose(); // we only care about upstream protocol. +#ifdef ENVOY_ENABLE_UHV + // permissive parsing is enabled + return; +#endif + if (downstreamProtocol() != Http::CodecType::HTTP1) { return; } From 2458f9588bec0182e62ddbf089872387e0bb33e3 Mon Sep 17 00:00:00 2001 From: Adam Meily Date: Mon, 18 Apr 2022 11:47:33 -0400 Subject: [PATCH 3/5] fix unit tests when UHV is enabled Signed-off-by: Adam Meily --- .../filters/listener/http_inspector/http_inspector_test.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/extensions/filters/listener/http_inspector/http_inspector_test.cc b/test/extensions/filters/listener/http_inspector/http_inspector_test.cc index f5e68eae1d9a..8b36d72a1876 100644 --- a/test/extensions/filters/listener/http_inspector/http_inspector_test.cc +++ b/test/extensions/filters/listener/http_inspector/http_inspector_test.cc @@ -543,6 +543,11 @@ TEST_F(HttpInspectorTest, MultipleReadsHttp1IncompleteBadHeader) { } TEST_F(HttpInspectorTest, MultipleReadsHttp1BadProtocol) { +#ifdef ENVOY_ENABLE_UHV + // permissive parsing + return; +#endif + init(); const std::string valid_header = "GET /index HTTP/1.1\r"; // offset: 0 10 From 6bbf73b2be7eb8c86e152ab9815533d9884a16ff Mon Sep 17 00:00:00 2001 From: Adam Meily Date: Mon, 18 Apr 2022 12:45:34 -0400 Subject: [PATCH 4/5] add "UHV" as approved word Signed-off-by: Adam Meily --- tools/spelling/spelling_dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 3fe1262e1458..f2368366d784 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -373,6 +373,7 @@ UA UBSAN UDP UDS +UHV UNC URI URL From 3f319598f301fd586d60e1d4e7ec2fa062699e37 Mon Sep 17 00:00:00 2001 From: Adam Meily Date: Mon, 18 Apr 2022 14:05:55 -0400 Subject: [PATCH 5/5] fix bad merge Signed-off-by: Adam Meily --- .../filters/listener/http_inspector/http_inspector_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/extensions/filters/listener/http_inspector/http_inspector_test.cc b/test/extensions/filters/listener/http_inspector/http_inspector_test.cc index 71dd489fc456..de3dc09ed292 100644 --- a/test/extensions/filters/listener/http_inspector/http_inspector_test.cc +++ b/test/extensions/filters/listener/http_inspector/http_inspector_test.cc @@ -487,7 +487,6 @@ TEST_F(HttpInspectorTest, MultipleReadsHttp1BadProtocol) { return; #endif - init(); const std::string valid_header = "GET /index HTTP/1.1\r"; // offset: 0 10 const std::string truncate_header = valid_header.substr(0, 14).append("\r");