connect: working around balsa bug

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
envoyproxy · May 13, 2024 · 4375345 · 4375345
1 parent 6c6518e
commit 4375345
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/source/extensions/transport_sockets/http_11_proxy/connect.cc b/source/extensions/transport_sockets/http_11_proxy/connect.cc
@@ -23,8 +23,7 @@ bool UpstreamHttp11ConnectSocket::isValidConnectResponse(absl::string_view respo
   bytes_processed = parser.parser().execute(response_payload.data(), response_payload.length());
   headers_complete = parser.headersComplete();
 
-  return parser.parser().getStatus() != Http::Http1::ParserStatus::Error &&
-         parser.headersComplete() && parser.parser().statusCode() == Http::Code::OK;
+  return parser.headersComplete() && parser.status200();
 }
 
 UpstreamHttp11ConnectSocket::UpstreamHttp11ConnectSocket(

diff --git a/source/extensions/transport_sockets/http_11_proxy/connect.h b/source/extensions/transport_sockets/http_11_proxy/connect.h
@@ -81,6 +81,8 @@ class SelfContainedParser : public Http::Http1::ParserCallbacks {
   }
   Http::Http1::CallbackResult onHeadersComplete() override {
     headers_complete_ = true;
+    status_200_ = parser().getStatus() != Http::Http1::ParserStatus::Error &&
+        parser().statusCode() == Http::Code::OK;
     parser_.pause();
     return Http::Http1::CallbackResult::Success;
   }
@@ -91,10 +93,12 @@ class SelfContainedParser : public Http::Http1::ParserCallbacks {
   void onChunkHeader(bool) override {}
 
   bool headersComplete() const { return headers_complete_; }
+  bool status200() const { return status_200_; }
   Http::Http1::BalsaParser& parser() { return parser_; }
 
 private:
   bool headers_complete_ = false;
+  bool status_200_ = false;
   Http::Http1::BalsaParser parser_;
 };
 

diff --git a/test/extensions/transport_sockets/http_11_proxy/connect_integration_test.cc b/test/extensions/transport_sockets/http_11_proxy/connect_integration_test.cc
@@ -120,15 +120,16 @@ name: envoy.clusters.dynamic_forward_proxy
     }
   }
 
-  void stripConnectUpgradeAndRespond() {
+  void stripConnectUpgradeAndRespond(bool include_content_length = false) {
     // Strip the CONNECT upgrade.
     std::string prefix_data;
     const std::string hostname(default_request_headers_.getHostValue());
     ASSERT_TRUE(fake_upstream_connection_->waitForInexactRawData("\r\n\r\n", &prefix_data));
     EXPECT_EQ(absl::StrCat("CONNECT ", hostname, ":443 HTTP/1.1\r\n\r\n"), prefix_data);
 
+    std::string content_length = include_content_length ? "Content-Length: 0\r\n" : "";
     // Ship the CONNECT response.
-    fake_upstream_connection_->writeRawData("HTTP/1.1 200 OK\r\n\r\n");
+    fake_upstream_connection_->writeRawData(absl::StrCat("HTTP/1.1 200 OK\r\n", content_length, "\r\n"));
   }
   bool use_alpn_ = false;
   bool try_http3_ = false;
@@ -450,7 +451,8 @@ TEST_P(Http11ConnectHttpIntegrationTest, DfpNoProxyAddress) {
   EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("dns_resolution_failure"));
 }
 
-TEST_P(Http11ConnectHttpIntegrationTest, DfpWithProxyAddress) {
+// Regression tests https://github.com/envoyproxy/envoy/issues/34096
+TEST_P(Http11ConnectHttpIntegrationTest, DfpWithProxyAddressAndContentLength) {
   addDfpConfig();
 
   initialize();
@@ -470,7 +472,7 @@ TEST_P(Http11ConnectHttpIntegrationTest, DfpWithProxyAddress) {
   // The request should be sent to fake upstream 1, as DNS lookup is bypassed due to the
   // connect-proxy header.
   ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
-  stripConnectUpgradeAndRespond();
+  stripConnectUpgradeAndRespond(true);
 
   ASSERT_TRUE(fake_upstream_connection_->readDisable(false));
   ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));