dfp: On proxying, continue the filter if the DNS cache has a host

Signed-off-by: Ali Beyad <abeyad@google.com>
abeyad · Jul 8, 2024 · 2aad26b · 2aad26b
1 parent 8f246ac
commit 2aad26b
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 28 deletions.
diff --git a/mobile/test/swift/integration/proxying/HTTPRequestUsingProxyTest.swift b/mobile/test/swift/integration/proxying/HTTPRequestUsingProxyTest.swift
@@ -12,34 +12,14 @@ final class HTTPRequestUsingProxyTest: XCTestCase {
     register_test_extensions()
   }
 
-  // https://github.com/envoyproxy/envoy/issues/33014
-  func skipped_testHTTPRequestUsingProxy() throws {
-    EnvoyTestServer.startHttpProxyServer()
-    let port = EnvoyTestServer.getEnvoyPort()
-
-    let engineExpectation = self.expectation(description: "Run started engine")
+  private func executeRequest(engine: Engine, scheme: String, authority: String) -> String? {
     let responseHeadersExpectation =
         self.expectation(description: "Successful response headers received")
     let responseTrailersExpectation =
         self.expectation(description: "Successful response trailers received")
 
-    let engine = EngineBuilder()
-      .setLogLevel(.debug)
-      .setLogger { _, msg in
-        print(msg, terminator: "")
-      }
-      .setOnEngineRunning {
-        engineExpectation.fulfill()
-      }
-      .respectSystemProxySettings(true)
-      .build()
-
-    EnvoyTestApi.registerTestProxyResolver("127.0.0.1", port: port, usePacResolver: false)
-
-    XCTAssertEqual(XCTWaiter.wait(for: [engineExpectation], timeout: 5), .completed)
-
-    let requestHeaders = RequestHeadersBuilder(method: .get, scheme: "http",
-                                               authority: "neverssl.com", path: "/")
+    let requestHeaders = RequestHeadersBuilder(method: .get, scheme: scheme,
+                                               authority: authority, path: "/")
       .build()
 
     var responseBuffer = Data()
@@ -61,8 +41,32 @@ final class HTTPRequestUsingProxyTest: XCTestCase {
     let expectations = [responseHeadersExpectation, responseTrailersExpectation]
     XCTAssertEqual(XCTWaiter.wait(for: expectations, timeout: 10), .completed)
 
-    if let responseBody = String(data: responseBuffer, encoding: .utf8) {
-      XCTAssertGreaterThanOrEqual(responseBody.utf8.count, 3900)
+    return String(data: responseBuffer, encoding: .utf8)
+  }
+
+  func testHTTPRequestUsingProxy() throws {
+    EnvoyTestServer.startHttpProxyServer()
+    let port = EnvoyTestServer.getEnvoyPort()
+
+    let engineExpectation = self.expectation(description: "Run started engine")
+
+    let engine = EngineBuilder()
+      .setLogLevel(.debug)
+      .setLogger { _, msg in
+        print(msg, terminator: "")
+      }
+      .setOnEngineRunning {
+        engineExpectation.fulfill()
+      }
+      .respectSystemProxySettings(true)
+      .build()
+
+    EnvoyTestApi.registerTestProxyResolver("127.0.0.1", port: port, usePacResolver: false)
+
+    XCTAssertEqual(XCTWaiter.wait(for: [engineExpectation], timeout: 5), .completed)
+
+    if let respBody = executeRequest(engine: engine, scheme: "http", authority: "neverssl.com") {
+      XCTAssertGreaterThanOrEqual(respBody.utf8.count, 3900)
     }
 
     engine.terminate()
@@ -187,8 +191,39 @@ final class HTTPRequestUsingProxyTest: XCTestCase {
     EnvoyTestServer.shutdownTestServer()
   }
 
-  // https://github.com/envoyproxy/envoy/issues/33014
-  func skipped_testHTTPRequestUsingProxyCancelStream() throws {
+  func testTwoHTTPRequestsUsingProxy() throws {
+    EnvoyTestServer.startHttpProxyServer()
+    let port = EnvoyTestServer.getEnvoyPort()
+
+    let engineExpectation = self.expectation(description: "Run started engine")
+
+    let engine = EngineBuilder()
+      .setLogLevel(.debug)
+      .setLogger { _, msg in
+        print(msg, terminator: "")
+      }
+      .setOnEngineRunning {
+        engineExpectation.fulfill()
+      }
+      .respectSystemProxySettings(true)
+      .build()
+
+    EnvoyTestApi.registerTestProxyResolver("127.0.0.1", port: port, usePacResolver: false)
+
+    XCTAssertEqual(XCTWaiter.wait(for: [engineExpectation], timeout: 5), .completed)
+
+    if let resp1 = executeRequest(engine: engine, scheme: "http", authority: "neverssl.com") {
+      XCTAssertGreaterThanOrEqual(resp1.utf8.count, 3900)
+    }
+    if let resp2 = executeRequest(engine: engine, scheme: "http", authority: "neverssl.com") {
+      XCTAssertGreaterThanOrEqual(resp2.utf8.count, 3900)
+    }
+
+    engine.terminate()
+    EnvoyTestServer.shutdownTestServer()
+  }
+
+  func testHTTPRequestUsingProxyCancelStream() throws {
     EnvoyTestServer.startHttpProxyServer()
     let port = EnvoyTestServer.getEnvoyPort()
 

diff --git a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc
@@ -266,8 +266,9 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea
   }
 
   latchTime(decoder_callbacks_, DNS_START);
+  const bool is_proxying = isProxying();
   auto result = config_->cache().loadDnsCacheEntryWithForceRefresh(
-      headers.Host()->value().getStringView(), default_port, isProxying(), force_cache_refresh,
+      headers.Host()->value().getStringView(), default_port, is_proxying, force_cache_refresh,
       *this);
   cache_load_handle_ = std::move(result.handle_);
   if (cache_load_handle_ == nullptr) {
@@ -281,6 +282,10 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea
 
     auto const& host = result.host_info_;
     latchTime(decoder_callbacks_, DNS_END);
+    if (is_proxying) {
+      ENVOY_BUG(host.has_value(), "Proxying request but not host entry in DNS cache.");
+      return Http::FilterHeadersStatus::Continue;
+    }
     if (!host.has_value() || !host.value()->address()) {
       onDnsResolutionFail((host.has_value() && *host) ? ((*host)->details()) : "no_host");
       return Http::FilterHeadersStatus::StopIteration;