http_proxy: set Host domain correctly

If http_proxy is set the host header is always rewritten to the IP. With
this change the http_proxy will be set to the DNS name to avoid issues
with virtual host in the endpoints.

Fix THREESCALE-4178

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - When PATH routing was enabled the URL was not correctly escaped [THREESCALE-3468](https://issues.jboss.org/browse/THREESCALE-3468) [PR #1150](https://github.com/3scale/APIcast/pull/1150)
+- Add the correct host header when using an http proxy [THREESCALE-4178](https://issues.jboss.org/browse/THREESCALE-4178) [PR #1143](https://github.com/3scale/APIcast/pull/1143)
+
 
 
 ### Fixed

gateway/src/apicast/http_proxy.lua

@@ -140,7 +140,8 @@ function _M.request(upstream, proxy_uri)
 
     if uri.scheme == 'http' then -- rewrite the request to use http_proxy
         local err
-        upstream:use_host_header(uri.host) -- to keep correct Host header in case we need to resolve it to IP
+        local host = upstream:set_host_header()
+        upstream:use_host_header(host)
         upstream.servers, err = resolve_servers(proxy_uri)
         if err then
           ngx.log(ngx.WARN, "HTTP proxy is set, but no servers have been resolved, err: ", err)

gateway/src/apicast/upstream.lua

@@ -138,11 +138,13 @@ end
 
 --- Rewrite request Host header to what is provided in the argument or in the URL.
 function _M:rewrite_request()
-    local uri = self.uri
 
-    if not uri then return nil, 'not initialized' end
+    local _, err = self:set_host_header()
+    if err then
+      return nil, 'not initialized'
+    end
 
-    ngx.req.set_header('Host', self.host or host_header(uri))
+    local uri = self.uri
 
     if uri.path then
         ngx.req.set_uri(prefix_path(uri.path))
@@ -162,6 +164,22 @@ local function exec(self)
     end
 end
 
+function _M:set_host_header()
+    if self.host then
+      ngx.req.set_header('Host', self.host)
+      return self.host, nil
+    end
+
+    -- set Host from uri if Host is not defined
+    local uri = self.uri
+    if not uri then
+      return nil, "Upstream URI not initialized"
+    end
+    local host = host_header(uri)
+    ngx.req.set_header('Host', host)
+    return host, nil
+end
+
 --- Execute the upstream.
 --- @tparam table context any table (policy context, ngx.ctx) to store the upstream for later use by balancer
 function _M:call(context)
@@ -182,7 +200,10 @@ function _M:call(context)
         -- to a proxy
         http_proxy.request(self, proxy_uri)
     else
-        self:rewrite_request()
+        local err = self:rewrite_request()
+        if err then
+          ngx.log(ngx.WARN, "Upstream rewrite request failed:", err)
+        end
     end
 
     if not self.servers then self:resolve() end

spec/upstream_spec.lua

@@ -296,9 +296,58 @@ describe('Upstream', function()
             local host = 'http://another_host'
 
             upstream:use_host_header(host)
-
             upstream:rewrite_request()
             assert.spy(ngx.req.set_header).was_called_with('Host', host)
         end)
     end)
+
+    describe(".set_host_header", function()
+
+        before_each(stub_ngx_request)
+
+        it('self.host is defined', function()
+            local host = "test.com"
+            local upstream = Upstream.new('http://example.com')
+            upstream:use_host_header(host)
+
+            local tmp_host, err = upstream:set_host_header()
+
+            assert.equal(tmp_host, host)
+            assert.falsy(err)
+            assert.spy(ngx.req.set_header).was_called_with('Host', host)
+        end)
+
+        it('self.uri is not defined', function()
+            local host = "test.com"
+            local upstream = Upstream.new('http://example.com')
+            upstream.uri = nil
+
+            local tmp_host, err = upstream:set_host_header()
+
+            assert.falsy(tmp_host, host)
+            assert.equal(err, "Upstream URI not initialized")
+        end)
+
+        it('self.uri is defined in a not default port', function()
+            local expected_host = "example.com:10000"
+            local upstream = Upstream.new('http://example.com:10000')
+
+            local tmp_host, err = upstream:set_host_header()
+
+            assert.equal(tmp_host, expected_host)
+            assert.falsy(err)
+            assert.spy(ngx.req.set_header).was_called_with('Host', expected_host)
+        end)
+
+        it('self.uri is defined in a default port', function()
+            local expected_host = "example.com"
+            local upstream = Upstream.new('http://example.com')
+
+            local tmp_host, err = upstream:set_host_header()
+
+            assert.equal(tmp_host, expected_host)
+            assert.falsy(err)
+            assert.spy(ngx.req.set_header).was_called_with('Host', expected_host)
+        end)
+    end)
 end)

t/apicast-policy-http-proxy.t

@@ -47,7 +47,13 @@ __DATA__
   }
 --- upstream
   location / {
-     echo 'yay, api backend';
+    access_by_lua_block {
+      local host = ngx.req.get_headers()["Host"]
+      local result = string.match(host, "^test:")
+      local assert = require('luassert')
+      assert.equals(result, "test:")
+      ngx.say("yay, api backend")
+    }
   }
 --- request
 GET /?user_key=value
@@ -95,7 +101,13 @@ using proxy: $TEST_NGINX_HTTP_PROXY
   }
 --- upstream
   location / {
-     echo 'yay, api backend';
+    access_by_lua_block {
+      local host = ngx.req.get_headers()["Host"]
+      local result = string.match(host, "^test:")
+      local assert = require('luassert')
+      assert.equals(result, "test:")
+      ngx.say("yay, api backend")
+    }
   }
 --- request
 GET /?user_key=value
@@ -150,10 +162,14 @@ location /test {
     echo_end;
 
     access_by_lua_block {
-       assert = require('luassert')
-       assert.equal('https', ngx.var.scheme)
-       assert.equal('$TEST_NGINX_RANDOM_PORT', ngx.var.server_port)
-       assert.equal('test', ngx.var.ssl_server_name)
+      assert = require('luassert')
+      assert.equal('https', ngx.var.scheme)
+      assert.equal('$TEST_NGINX_RANDOM_PORT', ngx.var.server_port)
+      assert.equal('test', ngx.var.ssl_server_name)
+
+      local host = ngx.req.get_headers()["Host"]
+      local result = string.match(host, "^test:")
+      assert.equals(result, "test:")
     }
 }
 --- request

t/http-proxy.t

@@ -159,8 +159,8 @@ proxy request: CONNECT 127.0.0.1:$TEST_NGINX_RANDOM_PORT
   }
 --- request
 GET /?user_key=value
---- response_body
-yay, api backend: test
+--- response_body_like
+yay, api backend: test:.*
 --- error_code: 200
 --- error_log env
 proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/transactions/authrep.xml?service_token=token-value&service_id=42&usage%5Bhits%5D=2&user_key=value HTTP/1.1
@@ -282,8 +282,8 @@ location /apicast {
   proxy_set_header Host localhost;
   proxy_pass http://$server_addr:$apicast_port;
 }
---- response_body
-yay, api backend: test
+--- response_body_like
+yay, api backend: test:.*
 --- error_code: 200
 --- error_log env
 proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/transactions/authrep.xml?service_token=token-value&service_id=42&usage%5Bhits%5D=2&user_key=value HTTP/1.1
@@ -398,11 +398,13 @@ apicast cache write key: 42:value:usage%5Bhits%5D=2, ttl: nil, context: ngx.time
 --- upstream
   location /test {
      echo 'yay, api backend: $http_host, uri: $uri, is_args: $is_args, args: $args';
+     # echo 'yay, api backend: $http_host, uri:';
   }
 --- request
 GET /test?user_key=value
---- response_body
-yay, api backend: test, uri: /test, is_args: ?, args: user_key=value
+
+--- response_body_like eval
+qw/yay, api backend: test:ooo\d+, uri: \/test, is_args: \?, args: user_key=value/
 --- error_code: 200
 --- error_log env
 proxy request: GET http://127.0.0.1:$TEST_NGINX_SERVER_PORT/test?user_key=value HTTP/1.1