http: turning on absolute url support by default (envoyproxy#8329)

As discussed on envoyproxy#8078 absolute URLs are part of the HTTP/1 spec and the code has been used for some time now - turning it up by default. Risk Level: Medium (changes to L7 for many users) Testing: updated tests for new defaults Docs Changes: no Release Notes: yes Signed-off-by: Alyssa Wilk <[email protected]>
danzh2010 · Sep 23, 2019 · d205e4d · d205e4d
1 parent 7beb1f8
commit d205e4d
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 10 deletions.
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -35,6 +35,7 @@ Version history
 * http: added the ability to :ref:`merge adjacent slashes<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.merge_slashes>` in the path.
 * http: :ref:`AUTO <envoy_api_enum_value_config.filter.network.http_connection_manager.v2.HttpConnectionManager.CodecType.AUTO>` codec protocol inference now requires the H2 magic bytes to be the first bytes transmitted by a downstream client.
 * http: remove h2c upgrade headers for HTTP/1 as h2c upgrades are currently not supported.
+* http: absolute URL support is now on by default. The prior behavior can be reinstated by setting :ref:`allow_absolute_url <envoy_api_field_core.Http1ProtocolOptions.allow_absolute_url>` to false.
 * listeners: added :ref:`continue_on_listener_filters_timeout <envoy_api_field_Listener.continue_on_listener_filters_timeout>` to configure whether a listener will still create a connection when listener filters time out.
 * listeners: added :ref:`HTTP inspector listener filter <config_listener_filters_http_inspector>`.
 * lua: extended `httpCall()` and `respond()` APIs to accept headers with entry values that can be a string or table of strings.

diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc
@@ -273,7 +273,7 @@ Utility::parseHttp2Settings(const envoy::api::v2::core::Http2ProtocolOptions& co
 Http1Settings
 Utility::parseHttp1Settings(const envoy::api::v2::core::Http1ProtocolOptions& config) {
   Http1Settings ret;
-  ret.allow_absolute_url_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, allow_absolute_url, false);
+  ret.allow_absolute_url_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, allow_absolute_url, true);
   ret.accept_http_10_ = config.accept_http_10();
   ret.default_host_for_http_10_ = config.default_host_for_http_10();
   return ret;

diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc
@@ -33,11 +33,9 @@ std::string normalizeDate(const std::string& s) {
   return std::regex_replace(s, date_regex, "date: Mon, 01 Jan 2017 00:00:00 GMT");
 }
 
-void setAllowAbsoluteUrl(
+void setDisallowAbsoluteUrl(
     envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) {
-  envoy::api::v2::core::Http1ProtocolOptions options;
-  options.mutable_allow_absolute_url()->set_value(true);
-  hcm.mutable_http_protocol_options()->CopyFrom(options);
+  hcm.mutable_http_protocol_options()->mutable_allow_absolute_url()->set_value(false);
 };
 
 void setAllowHttp10WithDefaultHost(
@@ -416,6 +414,7 @@ TEST_P(IntegrationTest, NoHost) {
 }
 
 TEST_P(IntegrationTest, BadPath) {
+  config_helper_.addConfigModifier(&setDisallowAbsoluteUrl);
   initialize();
   std::string response;
   sendRawHttpAndWaitForResponse(lookupPort("http"),
@@ -430,7 +429,6 @@ TEST_P(IntegrationTest, AbsolutePath) {
   auto host = config_helper_.createVirtualHost("www.redirect.com", "/");
   host.set_require_tls(envoy::api::v2::route::VirtualHost::ALL);
   config_helper_.addVirtualHost(host);
-  config_helper_.addConfigModifier(&setAllowAbsoluteUrl);
 
   initialize();
   std::string response;
@@ -446,7 +444,6 @@ TEST_P(IntegrationTest, AbsolutePathWithPort) {
   auto host = config_helper_.createVirtualHost("www.namewithport.com:1234", "/");
   host.set_require_tls(envoy::api::v2::route::VirtualHost::ALL);
   config_helper_.addVirtualHost(host);
-  config_helper_.addConfigModifier(&setAllowAbsoluteUrl);
   initialize();
   std::string response;
   sendRawHttpAndWaitForResponse(
@@ -463,7 +460,6 @@ TEST_P(IntegrationTest, AbsolutePathWithoutPort) {
   auto host = config_helper_.createVirtualHost("www.namewithport.com:1234", "/");
   host.set_require_tls(envoy::api::v2::route::VirtualHost::ALL);
   config_helper_.addVirtualHost(host);
-  config_helper_.addConfigModifier(&setAllowAbsoluteUrl);
   initialize();
   std::string response;
   sendRawHttpAndWaitForResponse(lookupPort("http"),
@@ -483,8 +479,8 @@ TEST_P(IntegrationTest, Connect) {
     cloned_listener->CopyFrom(*old_listener);
     old_listener->set_name("http_forward");
   });
-  // Set the first listener to allow absolute URLs.
-  config_helper_.addConfigModifier(&setAllowAbsoluteUrl);
+  // Set the first listener to disallow absolute URLs.
+  config_helper_.addConfigModifier(&setDisallowAbsoluteUrl);
   initialize();
 
   std::string response1;