Merge pull request #978 from 3scale/https-proxy-with-post-req

[THREESCALE-1781] Fix bug with proxied POST requests to HTTPS API backend via forward proxy
3scale · Jan 16, 2019 · 75c2c02 · 75c2c02
2 parents c214825 + 29a26a1
commit 75c2c02
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fix 3scale Batcher policy failing to cache and report requests containing app ID only [PR #956](https://github.com/3scale/apicast/pull/956), [THREESCALE-1515](https://issues.jboss.org/browse/THREESCALE-1515)
 - Auths against the 3scale backend are now retried when using the 3scale batching policy [PR #961](https://github.com/3scale/apicast/pull/961)
+- Fix timeouts when proxying POST requests to an HTTPS upstream using `HTTPS_PROXY` [PR #978](https://github.com/3scale/apicast/pull/978), [THREESCALE-1781](https://issues.jboss.org/browse/THREESCALE-1781)
 
 ### Added
 

diff --git a/gateway/src/apicast/http_proxy.lua b/gateway/src/apicast/http_proxy.lua
@@ -1,6 +1,5 @@
 local format = string.format
 
-local http = require 'resty.resolver.http'
 local resty_url = require "resty.url"
 local resty_resolver = require 'resty.resolver'
 local round_robin = require 'resty.balancer.round_robin'
@@ -85,12 +84,19 @@ local function current_path(uri)
 end
 
 local function forward_https_request(proxy_uri, uri)
+    -- This is needed to call ngx.req.get_body_data() below.
+    ngx.req.read_body()
+
     local request = {
         uri = uri,
         method = ngx.req.get_method(),
         headers = ngx.req.get_headers(0, true),
         path = current_path(uri),
-        body = http:get_client_body_reader(),
+
+        -- We cannot use resty.http's .get_client_body_reader().
+        -- In POST requests with HTTPS, the result of that call is nil, and it
+        -- results in a time-out.
+        body = ngx.req.get_body_data(),
     }
 
     local httpc, err = http_proxy.new(request)

diff --git a/t/http-proxy.t b/t/http-proxy.t
@@ -470,9 +470,52 @@ proxy request: CONNECT 127.0.0.1:$TEST_NGINX_RANDOM_PORT HTTP/1.1
 [error]
 --- user_files fixture=tls.pl eval
 
+=== TEST 10: Upstream API with HTTPS POST request, HTTPS_PROXY and HTTPS api_backend
+--- env eval
+("https_proxy" => $ENV{TEST_NGINX_HTTPS_PROXY})
+--- configuration random_port env
+{
+  "services": [
+    {
+      "backend_version":  1,
+      "proxy": {
+        "api_backend": "https://test:$TEST_NGINX_RANDOM_PORT",
+        "proxy_rules": [
+          { "pattern": "/test", "http_method": "POST", "metric_system_name": "hits", "delta": 2 }
+        ]
+      }
+    }
+  ]
+}
+--- backend
+  location /transactions/authrep.xml {
+    content_by_lua_block {
+      ngx.exit(ngx.OK)
+    }
+  }
+--- upstream env
+listen $TEST_NGINX_RANDOM_PORT ssl;
 
+ssl_certificate $TEST_NGINX_SERVER_ROOT/html/server.crt;
+ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/server.key;
+
+location /test {
+    echo_read_request_body;
+    echo $request_body;
+}
+--- request
+POST https://localhost/test?user_key=test3
+{ "some_param": "some_value" }
+--- response_body
+{ "some_param": "some_value" }
+--- error_code: 200
+--- error_log env
+proxy request: CONNECT 127.0.0.1:$TEST_NGINX_RANDOM_PORT HTTP/1.1
+--- no_error_log
+[error]
+--- user_files fixture=tls.pl eval
 
-=== TEST 10: Upstream Policy connection uses proxy
+=== TEST 11: Upstream Policy connection uses proxy
 --- env eval
 ("http_proxy" => $ENV{TEST_NGINX_HTTP_PROXY})
 --- configuration
@@ -517,7 +560,7 @@ proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/test?user_key=test3
 
 
 
-=== TEST 11: Upstream Policy connection uses proxy for https
+=== TEST 12: Upstream Policy connection uses proxy for https
 --- env eval
 ("https_proxy" => $ENV{TEST_NGINX_HTTPS_PROXY})
 --- configuration random_port env
@@ -574,8 +617,7 @@ proxy request: CONNECT 127.0.0.1:$TEST_NGINX_RANDOM_PORT HTTP/1.1
 --- user_files fixture=tls.pl eval
 
 
-
-=== TEST 12: Upstream Policy connection uses proxy
+=== TEST 13: Upstream Policy connection uses proxy
 --- env eval
 ("http_proxy" => $ENV{TEST_NGINX_HTTP_PROXY})
 --- configuration
@@ -624,8 +666,7 @@ proxy request: POST http://127.0.0.1:$TEST_NGINX_SERVER_PORT/test?user_key=test3
 [error]
 
 
-
-=== TEST 13: Upstream Policy connection uses proxy for https and forwards request body
+=== TEST 14: Upstream Policy connection uses proxy for https and forwards request body
 --- env eval
 ("https_proxy" => $ENV{TEST_NGINX_HTTPS_PROXY})
 --- configuration random_port env
@@ -687,8 +728,7 @@ proxy request: CONNECT 127.0.0.1:$TEST_NGINX_RANDOM_PORT HTTP/1.1
 --- user_files fixture=tls.pl eval
 
 
-
-=== TEST 14: upstream API connection uses proxy and correctly routes to a path.
+=== TEST 15: upstream API connection uses proxy and correctly routes to a path.
 --- env eval
 ("http_proxy" => $ENV{TEST_NGINX_HTTP_PROXY})
 --- configuration
@@ -726,8 +766,7 @@ proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/foo/test?user_key=va
 [error]
 
 
-
-=== TEST 15: Upstream Policy connection uses proxy and correctly routes to a path.
+=== TEST 16: Upstream Policy connection uses proxy and correctly routes to a path.
 --- env eval
 ("http_proxy" => $ENV{TEST_NGINX_HTTP_PROXY})
 --- configuration
@@ -761,7 +800,63 @@ proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/foo/test?user_key=va
 --- no_error_log
 [error]
 
-=== TEST 16: The path is set correctly when there are no args and proxied to an http upstream
+=== TEST 17: Upstream policy with HTTPS POST request, HTTPS_PROXY and HTTPS backend
+--- env eval
+("https_proxy" => $ENV{TEST_NGINX_HTTPS_PROXY})
+--- configuration random_port env
+{
+  "services": [
+    {
+      "backend_version":  1,
+      "proxy": {
+        "api_backend": "https://test:$TEST_NGINX_RANDOM_PORT",
+        "proxy_rules": [
+          { "pattern": "/test", "http_method": "POST", "metric_system_name": "hits", "delta": 2 }
+        ],
+        "policy_chain": [
+          { "name": "apicast.policy.upstream",
+            "configuration":
+              {
+                "rules": [ { "regex": "/test", "url": "https://test:$TEST_NGINX_RANDOM_PORT" } ]
+              }
+          },
+          {
+            "name": "apicast.policy.apicast"
+          }
+        ]
+      }
+    }
+  ]
+}
+--- backend
+  location /transactions/authrep.xml {
+    content_by_lua_block {
+      ngx.exit(ngx.OK)
+    }
+  }
+--- upstream env
+listen $TEST_NGINX_RANDOM_PORT ssl;
+
+ssl_certificate $TEST_NGINX_SERVER_ROOT/html/server.crt;
+ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/server.key;
+
+location /test {
+    echo_read_request_body;
+    echo $request_body;
+}
+--- request
+POST https://localhost/test?user_key=test3
+{ "some_param": "some_value" }
+--- response_body
+{ "some_param": "some_value" }
+--- error_code: 200
+--- error_log env
+proxy request: CONNECT 127.0.0.1:$TEST_NGINX_RANDOM_PORT HTTP/1.1
+--- no_error_log
+[error]
+--- user_files fixture=tls.pl eval
+
+=== TEST 18: The path is set correctly when there are no args and proxied to an http upstream
 --- env eval
 ("http_proxy" => $ENV{TEST_NGINX_HTTP_PROXY})
 --- configuration
@@ -799,7 +894,7 @@ proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/test HTTP/1.1
 --- no_error_log
 [error]
 
-=== TEST 17: The path is set correctly when there are no args and proxied to an https upstream
+=== TEST 19: The path is set correctly when there are no args and proxied to an https upstream
 Regression test: the string 'nil' was appended to the path
 --- env eval
 ("https_proxy" => $ENV{TEST_NGINX_HTTPS_PROXY})