http/h1_stream: parse request-target out into scheme/authority/path

  - absolute-form host overrides host header
  - CONNECT requests should have a host header
  - OPTIONS requests in absolute-form without a path should have path '*'

Author: daurnimator

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
@@ -249,6 +252,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 +319,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
+			-- 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", self:checktls() and "https" or "http")
+			headers:append(":scheme", scheme)
 			self:set_state("open")
 		else -- client
 			-- Make sure we're at front of connection pipeline
@@ -342,9 +411,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
@@ -742,13 +819,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

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()

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))
@@ -633,9 +634,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 .. "/", 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"))
 				local resp_headers = new_headers()
 				resp_headers:append(":status", "200")
 				assert(stream:write_headers(resp_headers, false))
@@ -658,7 +659,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 +696,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 = {