bp/1.32: original_ip_detection: revert unintended XFF header appending behavior in CustomHeaderIPDetection

changelogs/current.yaml

@@ -12,6 +12,11 @@ bug_fixes:
   change: |
     Relaxed the restriction on SNI logging to allow the ``_`` character, even if
     ``envoy.reloadable_features.sanitize_sni_in_access_log`` is enabled.
+- area: original_ip_detection custom header extension
+  change: |
+    Reverted :ref:`custom header
+    <envoy_v3_api_msg_extensions.http.original_ip_detection.custom_header.v3.CustomHeaderConfig>` extension to its
+    original behavior by disabling automatic XFF header appending that was inadvertently introduced in PR #31831.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/extensions/http/original_ip_detection/custom_header/custom_header.cc

@@ -26,18 +26,22 @@ CustomHeaderIPDetection::CustomHeaderIPDetection(
 
 Envoy::Http::OriginalIPDetectionResult
 CustomHeaderIPDetection::detect(Envoy::Http::OriginalIPDetectionParams& params) {
+  // NOTE: The ``XFF`` header from this extension is intentionally not appended.
+  // To preserve the behavior prior to #31831, ``skip_xff_append`` is explicitly set to true.
+  constexpr bool skip_xff_append = true;
+
   auto hdr = params.request_headers.get(header_name_);
   if (hdr.empty()) {
-    return {nullptr, false, reject_options_, false};
+    return {nullptr, false, reject_options_, skip_xff_append};
   }
 
   auto header_value = hdr[0]->value().getStringView();
   auto addr = Network::Utility::parseInternetAddressNoThrow(std::string(header_value));
   if (addr) {
-    return {addr, allow_trusted_address_checks_, absl::nullopt, false};
+    return {addr, allow_trusted_address_checks_, absl::nullopt, skip_xff_append};
   }
 
-  return {nullptr, false, reject_options_, false};
+  return {nullptr, false, reject_options_, skip_xff_append};
 }
 
 } // namespace CustomHeader

test/extensions/http/original_ip_detection/custom_header/custom_header_test.cc

@@ -99,6 +99,46 @@ TEST_F(CustomHeaderTest, FallbacksToDefaultResponseCode) {
   EXPECT_EQ(reject_options.body, "");
 }
 
+TEST_F(CustomHeaderTest, SkipXFFAppendBehavior) {
+  // Test all scenarios to ensure XFF header is never appended
+
+  // When header is missing
+  {
+    Envoy::Http::TestRequestHeaderMapImpl headers{{"x-other", "abc"}};
+    Envoy::Http::OriginalIPDetectionParams params = {headers, nullptr};
+    auto result = custom_header_extension_->detect(params);
+
+    EXPECT_TRUE(result.skip_xff_append) << "XFF append should be skipped when header is missing";
+  }
+
+  // When header contains invalid IP
+  {
+    Envoy::Http::TestRequestHeaderMapImpl headers{{"x-real-ip", "not-a-real-ip"}};
+    Envoy::Http::OriginalIPDetectionParams params = {headers, nullptr};
+    auto result = custom_header_extension_->detect(params);
+
+    EXPECT_TRUE(result.skip_xff_append) << "XFF append should be skipped for invalid IP";
+  }
+
+  // When header contains valid IPv4
+  {
+    Envoy::Http::TestRequestHeaderMapImpl headers{{"x-real-ip", "1.2.3.4"}};
+    Envoy::Http::OriginalIPDetectionParams params = {headers, nullptr};
+    auto result = custom_header_extension_->detect(params);
+
+    EXPECT_TRUE(result.skip_xff_append) << "XFF append should be skipped for valid IPv4";
+  }
+
+  // When header contains valid IPv6
+  {
+    Envoy::Http::TestRequestHeaderMapImpl headers{{"x-real-ip", "fc00::1"}};
+    Envoy::Http::OriginalIPDetectionParams params = {headers, nullptr};
+    auto result = custom_header_extension_->detect(params);
+
+    EXPECT_TRUE(result.skip_xff_append) << "XFF append should be skipped for valid IPv6";
+  }
+}
+
 } // namespace CustomHeader
 } // namespace OriginalIPDetection
 } // namespace Http