followup: separate proxy connection setup logic to a low-level connect_proxy() method

This commit separates the logic that sets up the connection to the proxy server
to a separate connect_proxy() method. This method is provided to users as a
low-level API they can use similarly to the connect() method.

The connect_proxy() will handle the connection establishment to the proxy server
and performs the CONNECT request to setup a TCP tunnel to a https protected host.
Similar to the connect() method, it is then up to the user to take care of the
details that are relevant when using a proxy (i.e use absolute uris for http
requests and perform a TLS handshake for https connections).

There's also a new test case that verifies the CONNECT request is used properly
to establish a tunnel to the remote server when TLS is used. Due to the limitations
of the test framework, this case only considers the format of the outgoing CONNECT
request and how the code handles errors sent by the proxy. Testing a full TLS tunnel
is unfortunately not possible with the tools the test framework provides as it would
require a real reverse proxy or a method of forwarding the TCP connection after the
CONNECT request is received to a real web server that can talk TLS.

Author: sjakthol

README.md

@@ -22,6 +22,7 @@ Production ready.
 
 * [new](#new)
 * [connect](#connect)
+* [connect_proxy](#connect_proxy)
 * [set_timeout](#set_timeout)
 * [set_timeouts](#set_timeouts)
 * [ssl_handshake](#ssl_handshake)
@@ -158,6 +159,24 @@ An optional Lua table can be specified as the last argument to this method to sp
 * `pool`
 : Specifies a custom name for the connection pool being used. If omitted, then the connection pool name will be generated from the string template `<host>:<port>` or `<unix-socket-path>`.
 
+## connect_proxy
+
+`syntax: ok, err = httpc:connect_proxy(proxy_uri, scheme, host, port)`
+
+Attempts to connect to the web server through the given proxy server. The method accepts the following arguments:
+
+* `proxy_uri` - Full URI of the proxy server to use (e.g. `http://proxy.example.com:3128/`). Note: Only `http` protocol is supported.
+* `scheme` - The protocol to use between the proxy server and the remote host (`http` or `https`). If `https` is specified as the scheme, `connect_proxy()` makes a `CONNECT` request to establish a TCP tunnel to the remote host through the proxy server.
+* `host` - The hostname of the remote host to connect to.
+* `port` - The port of the remote host to connect to.
+
+If an error occurs during the connection attempt, this method returns `nil` with a string describing the error. If the connection was successfully established, the method returns `1`.
+
+There's a few key points to keep in mind when using this api:
+
+* If the scheme is `https`, you need to perform the TLS handshake with the remote server manually using the `ssl_handshake()` method before sending any requests through the proxy tunnel.
+* If the scheme is `http`, you need to ensure that the requests you send through the connections conforms to [RFC 7230](https://tools.ietf.org/html/rfc7230) and especially [Section 5.3.2.](https://tools.ietf.org/html/rfc7230#section-5.3.2) which states that the request target must be in absolute form. In practice, this means that when you use `send_request()`, the `path` must be an absolute URI to the resource (e.g. `http://example.com/index.html` instead of just `/index.html`).
+
 ## set_timeout
 
 `syntax: httpc:set_timeout(time)`
@@ -244,7 +263,7 @@ When the request is successful, `res` will contain the following fields:
 * `status` The status code.
 * `reason` The status reason phrase.
 * `headers` A table of headers. Multiple headers with the same field name will be presented as a table of values.
-* `has_body` A boolean flag indicating if there is a body to be read. 
+* `has_body` A boolean flag indicating if there is a body to be read.
 * `body_reader` An iterator function for reading the body in a streaming fashion.
 * `read_body` A method to read the entire body into a string.
 * `read_trailers` A method to merge any trailers underneath the headers, after reading the body.
@@ -420,7 +439,7 @@ local res, err = httpc:request{
 }
 ```
 
-If `sock` is specified, 
+If `sock` is specified,
 
 # Author
 

lib/resty/http.lua

@@ -787,7 +787,6 @@ function _M.request_pipeline(self, requests)
     return responses
 end
 
-
 function _M.request_uri(self, uri, params)
     params = tbl_copy(params or {})  -- Take by value
 
@@ -801,60 +800,50 @@ function _M.request_uri(self, uri, params)
     if not params.query then params.query = query end
 
     -- See if we should use a proxy to make this request
-    local proxy_host, proxy_port
     local proxy_uri = self:get_proxy_uri(scheme, host)
-    if proxy_uri then
-        local parsed_proxy_uri, err = self:parse_uri(proxy_uri, false)
-        if not parsed_proxy_uri then
-            return nil, err
-        end
 
-        proxy_host, proxy_port = parsed_proxy_uri[2], parsed_proxy_uri[3]
+    -- Make the connection either through the proxy or directly
+    -- to the remote host
+    local c, err
+
+    if proxy_uri then
+        c, err = self:connect_proxy(proxy_uri, scheme, host, port)
+    else
+        c, err = self:connect(host, port)
     end
 
-    local c, err = self:connect(proxy_host or host, proxy_port or port)
     if not c then
         return nil, err
     end
 
-    if proxy_uri and scheme == "https" then
-        -- Make a CONNECT request to create a tunnel to the destination through
-        -- the proxy
-        local destination = host .. ":" .. port
-        local res, err = self:request({
-            method = "CONNECT",
-            path = destination,
-            headers = {
-                ["Host"] = destination
-            }
-        })
-
-        if not res then
-            return nil, err
-        end
-
-        if res.status < 200 or res.status > 299 then
-            return nil, "failed to establish a tunnel through a proxy: " .. res.status
+    if proxy_uri then
+        if scheme == "http" then
+            -- When a proxy is used, the target URI must be in absolute-form
+            -- (RFC 7230, Section 5.3.2.). That is, it must be an absolute URI
+            -- to the remote resource with the scheme, host and an optional port
+            -- in place.
+            --
+            -- Since _format_request() constructs the request line by concatenating
+            -- params.path and params.query together, we need to modify the path
+            -- to also include the scheme, host and port so that the final form
+            -- in conformant to RFC 7230.
+            if port == 80 then
+                params.path = scheme .. "://" .. host .. path
+            else
+                params.path = scheme .. "://" .. host .. ":" .. port .. path
+            end
         end
 
-        -- don't keep this connection alive as the next request could target
-        -- any host and re-using the tunnel for that is not possible
-        self.keepalive = false
-    end
-
-    if proxy_uri and scheme == "http" then
-        -- http proxies expect to see the full URI in the request line
-        if port == 80 then
-            params.path = scheme .. "://" .. host .. path
-        else
-            params.path = scheme .. "://" .. host .. ":" .. port .. path
+        if scheme == "https" then
+            -- don't keep this connection alive as the next request could target
+            -- any host and re-using the proxy tunnel for that is not possible
+            self.keepalive = false
         end
-    end
 
-    if proxy_uri then
-        -- self:connect() set the host and port to point to the proxy server. As
+        -- self:connect_uri() set the host and port to point to the proxy server. As
         -- the connection to the proxy has been established, set the host and port
-        -- to point to the actual remote endpoint at the other end of the tunnel
+        -- to point to the actual remote endpoint at the other end of the tunnel to
+        -- ensure the correct Host header added to the requests.
         self.host = host
         self.port = port
     end
@@ -1016,4 +1005,51 @@ function _M.get_proxy_uri(self, scheme, host)
 end
 
 
+function _M.connect_proxy(self, proxy_uri, scheme, host, port)
+    -- Parse the provided proxy URI
+    local parsed_proxy_uri, err = self:parse_uri(proxy_uri, false)
+    if not parsed_proxy_uri then
+        return nil, err
+    end
+
+    -- Check that the scheme is http (https is not supported for
+    -- connections between the client and the proxy)
+    local proxy_scheme = parsed_proxy_uri[1]
+    if proxy_scheme ~= "http" then
+        return nil, "protocol " .. proxy_scheme .. " not supported for proxy connections"
+    end
+
+    -- Make the connection to the given proxy
+    local proxy_host, proxy_port = parsed_proxy_uri[2], parsed_proxy_uri[3]
+    local c, err = self:connect(proxy_host, proxy_port)
+    if not c then
+        return nil, err
+    end
+
+    if scheme == "https" then
+        -- Make a CONNECT request to create a tunnel to the destination through
+        -- the proxy. The request-target and the Host header must be in the
+        -- authority-form of RFC 7230 Section 5.3.3. See also RFC 7231 Section
+        -- 4.3.6 for more details about the CONNECT request
+        local destination = host .. ":" .. port
+        local res, err = self:request({
+            method = "CONNECT",
+            path = destination,
+            headers = {
+                ["Host"] = destination
+            }
+        })
+
+        if not res then
+            return nil, err
+        end
+
+        if res.status < 200 or res.status > 299 then
+            return nil, "failed to establish a tunnel through a proxy: " .. res.status
+        end
+    end
+
+    return c, nil
+end
+
 return _M

t/16-http-proxy.t

@@ -245,3 +245,55 @@ GET /lua
 --- no_error_log
 [error]
 [warn]
+
+=== TEST 6: request_uri makes a proper CONNECT request when proxying https resources
+--- http_config eval: $::HttpConfig
+--- config
+    location /lua {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+            httpc:set_proxy_options({
+                http_proxy = "http://127.0.0.1:12345",
+                https_proxy = "http://127.0.0.1:12345"
+            })
+
+            -- Slight Hack: temporarily change the module global user agent to make it
+            -- predictable for this test case
+            local ua = http._USER_AGENT
+            http._USER_AGENT = "test_ua"
+            local res, err = httpc:request_uri("https://127.0.0.1/target?a=1&b=2")
+            http._USER_AGENT = ua
+
+            if not err then
+                -- The proxy request should fail as the TCP server listening returns
+                -- 403 response. We cannot really test the success case here as that
+                -- would require an actual reverse proxy to be implemented through
+                -- the limited functionality we have available in the raw TCP sockets
+                ngx.log(ngx.ERR, "unexpected success")
+                return
+            end
+
+            ngx.status = 403
+            ngx.say(err)
+        }
+    }
+--- tcp_listen: 12345
+--- tcp_query eval_stdout
+# Note: The incoming request contains CRLF line endings and print needs to
+# be used here to get the same line breaks to the expected request
+print "CONNECT 127.0.0.1:443 HTTP/1.1\r\nUser-Agent: test_ua\r\nHost: 127.0.0.1:443\r\n\r\n"
+
+# The reply cannot be successful or otherwise the client would start
+# to do a TLS handshake with the proxied host and that we cannot
+# do with these sockets
+--- tcp_reply
+HTTP/1.1 403 Forbidden
+Connection: close
+
+--- request
+GET /lua
+--- error_code: 403
+--- no_error_log
+[error]
+[warn]