diff --git a/http/h1_stream.lua b/http/h1_stream.lua index b2469a14..fc5731dc 100644 --- a/http/h1_stream.lua +++ b/http/h1_stream.lua @@ -5,6 +5,7 @@ local ce = require "cqueues.errno" local new_fifo = require "fifo" local lpeg = require "lpeg" local http_patts = require "lpeg_patterns.http" +local uri_patts = require "lpeg_patterns.uri" local new_headers = require "http.headers".new local reason_phrases = require "http.h1_reason_phrases" local stream_common = require "http.stream_common" @@ -22,6 +23,8 @@ local Connection = lpeg.Ct(http_patts.Connection) * EOF local Content_Encoding = lpeg.Ct(http_patts.Content_Encoding) * EOF local Transfer_Encoding = lpeg.Ct(http_patts.Transfer_Encoding) * EOF local TE = lpeg.Ct(http_patts.TE) * EOF +local absolute_form = uri_patts.absolute_uri * EOF +local authority_form = uri_patts.authority * EOF local function has(list, val) if list then @@ -74,6 +77,7 @@ local function new_stream(connection) req_method = nil; -- string peer_version = nil; -- 1.0 or 1.1 + use_absolute_target = nil; -- tristate boolean has_main_headers = false; headers_in_progress = nil; headers_fifo = new_fifo(); @@ -249,6 +253,41 @@ function stream_methods:step(timeout) return true end +-- should return scheme, authority, path +local function parse_target(path) + if path:sub(1, 1) == "/" or path == "*" then + -- 'origin-form' or 'asterisk-form' + -- early exit for common case + return nil, nil, path + end + + local absolute_uri = absolute_form:match(path) + if absolute_uri then + -- don't want normalised form of authority or path + local authority + if absolute_uri.host then + authority, path = path:match("://([^/]*)(.*)") + if path == "" then + path = nil + end + else + -- authority is nil + -- path should be nil if there are no characters. + path = path:match(":(.+)") + end + return absolute_uri.scheme, authority, path + end + + if authority_form:match(path) then + -- don't want normalised form of authority + -- `path` *is* the authority + return nil, path, nil + end + + -- other... + return nil, nil, path +end + -- read_headers may be called more than once for a stream -- e.g. for 100 Continue -- this function *should never throw* under normal operation @@ -281,12 +320,43 @@ function stream_methods:read_headers(timeout) self.peer_version = httpversion headers = new_headers() headers:append(":method", method) - if method == "CONNECT" then - headers:append(":authority", target) - else - headers:append(":path", target) + local scheme, authority, path = parse_target(target) + if authority then + -- RFC 7230 Section 5.4 + -- When a proxy receives a request with an absolute-form of + -- request-target, the proxy MUST ignore the received Host header field + -- (if any) and instead replace it with the host information of the + -- request-target. + headers:append(":authority", authority) end - headers:append(":scheme", self:checktls() and "https" or "http") + -- RFC 7230 Section 5.5 + -- If the request-target is in absolute-form, the effective request URI + -- is the same as the request-target. Otherwise, the effective request + -- URI is constructed as follows: + if not scheme then + -- If the server's configuration (or outbound gateway) provides a + -- fixed URI scheme, that scheme is used for the effective request + -- URI. Otherwise, if the request is received over a TLS-secured TCP + -- connection, the effective request URI's scheme is "https"; if not, + -- the scheme is "http". + if self:checktls() then + scheme = "https" + else + scheme = "http" + end + end + if path then + headers:append(":path", path) + elseif method == "OPTIONS" then + -- RFC 7230 Section 5.3.4 + -- If a proxy receives an OPTIONS request with an absolute-form of + -- request-target in which the URI has an empty path and no query + -- component, then the last proxy on the request chain MUST send a + -- request-target of "*" when it forwards the request to the indicated + -- origin server. + headers:append(":path", "*") + end + headers:append(":scheme", scheme) self:set_state("open") else -- client -- Make sure we're at front of connection pipeline @@ -342,9 +412,17 @@ function stream_methods:read_headers(timeout) end k = k:lower() -- normalise to lower case if k == "host" and not is_trailers then - k = ":authority" + -- RFC 7230 Section 5.4 + -- When a proxy receives a request with an absolute-form of + -- request-target, the proxy MUST ignore the received Host header field + -- (if any) and instead replace it with the host information of the + -- request-target. + if not headers:has(":authority") then + headers:append(":authority", v) + end + else + headers:append(k, v) end - headers:append(k, v) end do @@ -571,8 +649,32 @@ function stream_methods:write_headers(headers, end_stream, timeout) assert(not headers:has(":path"), "CONNECT requests should not have a path") else -- RFC 7230 Section 5.4: A client MUST send a Host header field in all HTTP/1.1 request messages. - assert(self.connection.version < 1.1 or headers:has(":authority"), "missing authority") + local has_authority = headers:has(":authority") + assert(has_authority or self.connection.version < 1.1, "missing authority") target = assert(headers:get(":path"), "missing path") + + local use_absolute_target = self.use_absolute_target + if use_absolute_target then + assert(has_authority, "request-target absolute-form requires an authority") + elseif use_absolute_target == nil then + use_absolute_target = has_authority + end + if use_absolute_target then + -- RFC 7230 Section 5.3.2 + -- When making a request to a proxy, other than a CONNECT or server-wide + -- OPTIONS request (as detailed below), a client MUST send the target + -- URI in absolute-form as the request-target. + -- ... + -- To allow for transition to the absolute-form for all requests in some + -- future version of HTTP, a server MUST accept the absolute-form in + -- requests, even though HTTP/1.1 clients will only send them in + -- requests to proxies. + if target == "*" then + target = headers:get(":scheme") .. "://" .. headers:get(":authority") + else + target = headers:get(":scheme") .. "://" .. headers:get(":authority") .. target + end + end end if self.connection.req_locked then -- Wait until previous request has been fully written @@ -742,13 +844,9 @@ function stream_methods:write_headers(headers, end_stream, timeout) return nil, err, errno end elseif name == ":authority" then - -- for CONNECT requests, :authority is the path - if self.req_method ~= "CONNECT" then - -- otherwise it's the Host header - local ok, err, errno = self.connection:write_header("host", value, 0) - if not ok then - return nil, err, errno - end + local ok, err, errno = self.connection:write_header("host", value, 0) + if not ok then + return nil, err, errno end end end diff --git a/http/request.lua b/http/request.lua index f9b65d67..25428542 100644 --- a/http/request.lua +++ b/http/request.lua @@ -358,6 +358,7 @@ function request_methods:go(timeout) local port = self.port local tls = self.tls local version = self.version + local use_absolute_target -- RFC 6797 Section 8.3 if not tls and self.hsts and self.hsts:check(host) then @@ -456,17 +457,15 @@ function request_methods:go(timeout) if request_headers:get(":method") == "CONNECT" then error("cannot use HTTP Proxy with CONNECT method") end - -- TODO: Check if :path already has authority? - local old_url = self:to_uri(false) host = assert(proxy.host, "proxy is missing host") port = proxy.port or http_util.scheme_to_port[proxy.scheme] - -- proxy requests get a uri that includes host as their path - if not cloned_headers then - request_headers = request_headers:clone() - cloned_headers = true -- luacheck: ignore 311 - end - request_headers:upsert(":path", old_url) + tls = (proxy.scheme == "https") + use_absolute_target = true if proxy.userinfo then + if not cloned_headers then + request_headers = request_headers:clone() + cloned_headers = true -- luacheck: ignore 311 + end request_headers:upsert("proxy-authorization", "basic " .. basexx.to_base64(proxy.userinfo), true) end end @@ -520,6 +519,10 @@ function request_methods:go(timeout) end end + if use_absolute_target and connection.version < 2 then + stream.use_absolute_target = use_absolute_target + end + local body = self.body do -- Write outgoing headers local ok, err, errno = stream:write_headers(request_headers, body == nil, deadline and deadline-monotime()) diff --git a/spec/h1_stream_spec.lua b/spec/h1_stream_spec.lua index f9cfea94..5e5de780 100644 --- a/spec/h1_stream_spec.lua +++ b/spec/h1_stream_spec.lua @@ -25,6 +25,33 @@ describe("http1 stream", function() assert.same("/", h:get(":path")) assert.same("bar", h:get("foo")) end) + it("CONNECT requests should have an host header on the wire", function() + local server, client = new_pair(1.1) + local cq = cqueues.new() + cq:wrap(function() + local stream = client:new_stream() + local req_headers = new_headers() + req_headers:append(":method", "CONNECT") + req_headers:append(":scheme", "http") + req_headers:append(":authority", "myauthority:8888") + assert(stream:write_headers(req_headers, true)) + stream:shutdown() + end) + cq:wrap(function() + local method, path, httpversion = assert(server:read_request_line()) + assert.same("CONNECT", method) + assert.same("myauthority:8888", path) + assert.same(1.1, httpversion) + local k, v = assert(server:read_header()) + assert.same("host", k) + assert.same("myauthority:8888", v) + server:shutdown() + end) + assert_loop(cq, TEST_TIMEOUT) + assert.truthy(cq:empty()) + server:close() + client:close() + end) it("Writing to a shutdown connection returns EPIPE", function() local server, client = new_pair(1.1) local stream = client:new_stream() diff --git a/spec/request_spec.lua b/spec/request_spec.lua index e0b1e057..20e98b7a 100644 --- a/spec/request_spec.lua +++ b/spec/request_spec.lua @@ -611,8 +611,9 @@ describe("http.request module", function() local h = assert(stream:get_headers()) local _, host, port = stream:localname() local authority = http_util.to_authority(host, port, "http") + assert.same("http", h:get(":scheme")) assert.same(authority, h:get ":authority") - assert.same("http://" .. authority .. "/", h:get(":path")) + assert.same("/", h:get(":path")) local resp_headers = new_headers() resp_headers:append(":status", "200") assert(stream:write_headers(resp_headers, false)) @@ -629,13 +630,38 @@ describe("http.request module", function() stream:shutdown() end) end) - it("works with a proxy server with a path component", function() + it("works with a tls proxy server", function() test(function(stream) local h = assert(stream:get_headers()) + assert.truthy(stream:checktls()) -- came in via TLS local _, host, port = stream:localname() local authority = http_util.to_authority(host, port, "http") + assert.same("http", h:get(":scheme")) assert.same(authority, h:get ":authority") - assert.same("http://" .. authority .. "/", h:get(":path")) + assert.same("/", h:get(":path")) + local resp_headers = new_headers() + resp_headers:append(":status", "200") + assert(stream:write_headers(resp_headers, false)) + assert(stream:write_chunk("hello world", true)) + end, function(req) + req.proxy = { + scheme = "https"; + host = req.host; + port = req.port; + } + local headers, stream = assert(req:go()) + assert.same("200", headers:get(":status")) + assert.same("hello world", assert(stream:get_body_as_string())) + stream:shutdown() + end) + end) + it("works with a proxy server with a path component", function() + test(function(stream) + local h = assert(stream:get_headers()) + local _, host, port = stream:localname() + assert.same("http", h:get(":scheme")) + assert.same(http_util.to_authority(host, port, "http"), h:get ":authority") + assert.same("/", h:get(":path")) local resp_headers = new_headers() resp_headers:append(":status", "200") assert(stream:write_headers(resp_headers, false)) @@ -658,7 +684,9 @@ describe("http.request module", function() local h = assert(stream:get_headers()) assert.same("OPTIONS", h:get ":method") local _, host, port = stream:localname() - assert.same("http://" .. http_util.to_authority(host, port, "http"), h:get(":path")) + assert.same("http", h:get(":scheme")) + assert.same(http_util.to_authority(host, port, "http"), h:get(":authority")) + assert.same("*", h:get(":path")) stream:shutdown() end, function(req) req.headers:upsert(":method", "OPTIONS") @@ -693,9 +721,9 @@ describe("http.request module", function() test(function(stream) local h = assert(stream:get_headers()) local _, host, port = stream:localname() - local authority = http_util.to_authority(host, port, "http") - assert.same(authority, h:get ":authority") - assert.same("http://" .. authority .. "/foo", h:get(":path")) + assert.same("http", h:get(":scheme")) + assert.same(http_util.to_authority(host, port, "http"), h:get ":authority") + assert.same("/foo", h:get(":path")) stream:shutdown() end, function(req) req.proxy = {