From e55900cf24baacd4afdcf032321fe6f5f84ad06e Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 12:38:07 +1300 Subject: [PATCH 01/10] Reject `transfer-encoding` in HTTP/1.0 messages and non-chunked request bodies. --- lib/protocol/http1/connection.rb | 48 +++++++++---- releases.md | 4 ++ test/protocol/http1/connection.rb | 109 ++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 14 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 0ec263e..74a7f4a 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -451,7 +451,7 @@ def read_request @persistent = persistent?(version, method, headers) end - body = read_request_body(method, headers) + body = read_request_body(method, headers, version) unless body self.receive_end_stream! @@ -507,7 +507,7 @@ def read_response(method) end unless interim_status?(status) - body = read_response_body(method, status, headers) + body = read_response_body(method, status, headers, version) unless body self.receive_end_stream! @@ -928,7 +928,8 @@ def extract_content_length(headers) # @parameter method [String] the HTTP method. # @parameter status [Integer] the HTTP status code. # @parameter headers [Hash] the headers of the response. - def read_response_body(method, status, headers) + # @parameter version [String | Nil] the HTTP version of the response, if known. + def read_response_body(method, status, headers, version = nil) # RFC 7230 3.3.3 # 1. Any response to a HEAD request and any response with a 1xx # (Informational), 204 (No Content), or 304 (Not Modified) status @@ -965,7 +966,7 @@ def read_response_body(method, status, headers) return read_tunnel_body end - return read_body(headers, true) + return read_body(headers, true, version) end # Read the body of the request. @@ -976,7 +977,8 @@ def read_response_body(method, status, headers) # # @parameter method [String] the HTTP method. # @parameter headers [Hash] the headers of the request. - def read_request_body(method, headers) + # @parameter version [String | Nil] the HTTP version of the request, if known. + def read_request_body(method, headers, version = nil) # 2. Any 2xx (Successful) response to a CONNECT request implies that # the connection will become a tunnel immediately after the empty # line that concludes the header fields. A client MUST ignore any @@ -993,7 +995,7 @@ def read_request_body(method, headers) # 6. If this is a request message and none of the above are true, then # the message body length is zero (no message body is present). - return read_body(headers) + return read_body(headers, false, version) end # Read the body of the message. @@ -1002,15 +1004,28 @@ def read_request_body(method, headers) # - Otherwise, if the `content-length` is present, the body is read until the content length is reached. # - Otherwise, if `remainder` is true, the body is read until the connection is closed. # + # Framing errors (e.g. `transfer-encoding` in an HTTP/1.0 message, or a request whose final transfer coding is not `chunked`) are rejected and mark the connection as non-persistent, since the remainder of the stream can no longer be delimited reliably. + # # @parameter headers [Hash] the headers of the message. - # @parameter remainder [Boolean] whether to read the remainder of the body. + # @parameter remainder [Boolean] whether to read the remainder of the body, i.e. the message is a response which may be delimited by closing the connection. + # @parameter version [String | Nil] the HTTP version of the message, if known. # @returns [Object] the body. - def read_body(headers, remainder = false) + # @raises [BadRequest] if the message framing is invalid. + def read_body(headers, remainder = false, version = nil) # 3. If a Transfer-Encoding header field is present and the chunked # transfer coding (Section 4.1) is the final encoding, the message # body length is determined by reading and decoding the chunked # data until the transfer coding indicates the data is complete. if transfer_encoding = headers.delete(TRANSFER_ENCODING) + # A server or client that receives an HTTP/1.0 message containing a + # Transfer-Encoding header field MUST treat the message as if the + # framing is faulty, even if a Content-Length is present, and close + # the connection after processing the message. + if version == HTTP10 + @persistent = false + raise BadRequest, "HTTP/1.0 message contains transfer encoding!" + end + # If a message is received with both a Transfer-Encoding and a # Content-Length header field, the Transfer-Encoding overrides the # Content-Length. Such a message might indicate an attempt to @@ -1019,21 +1034,26 @@ def read_body(headers, remainder = false) # remove the received Content-Length field prior to forwarding such # a message downstream. if headers[CONTENT_LENGTH] + @persistent = false raise BadRequest, "Message contains both transfer encoding and content length!" end if transfer_encoding.last == CHUNKED return read_chunked_body(headers) - else + elsif remainder # If a Transfer-Encoding header field is present in a response and # the chunked transfer coding is not the final encoding, the # message body length is determined by reading the connection until - # it is closed by the server. If a Transfer-Encoding header field - # is present in a request and the chunked transfer coding is not - # the final encoding, the message body length cannot be determined - # reliably; the server MUST respond with the 400 (Bad Request) - # status code and then close the connection. + # it is closed by the server. return read_remainder_body + else + # If a Transfer-Encoding header field is present in a request and + # the chunked transfer coding is not the final encoding, the + # message body length cannot be determined reliably; the server + # MUST respond with the 400 (Bad Request) status code and then + # close the connection. + @persistent = false + raise BadRequest, "Request transfer encoding does not end with chunked: #{transfer_encoding.inspect}" end end diff --git a/releases.md b/releases.md index db77727..f289709 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - Reject `transfer-encoding` in HTTP/1.0 messages and requests whose final transfer coding is not `chunked` with `BadRequest`, and mark the connection as non-persistent, as required by RFC 9112 for reliable message framing. + ## v0.41.0 - Map remote socket failures to `Protocol::HTTP::RemoteError` and prevent connection reuse. diff --git a/test/protocol/http1/connection.rb b/test/protocol/http1/connection.rb index 701ea88..32c7428 100644 --- a/test/protocol/http1/connection.rb +++ b/test/protocol/http1/connection.rb @@ -156,6 +156,75 @@ expect(server).to be(:persistent) end + it "reads request with chunked body and trailer" do + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\nTrailer: etag\r\n\r\nb\r\nHello World\r\n0\r\nETag: abcd\r\n\r\n" + client.stream.close + + authority, method, target, version, headers, body = server.read_request + + expect(body.join).to be == "Hello World" + expect(headers).to be == {"trailer" => ["etag"], "etag" => "abcd"} + + expect(server).to be(:persistent) + end + + it "fails with HTTP/1.0 request with transfer-encoding" do + # RFC 9112 Section 6.1: a recipient MUST treat an HTTP/1.0 message containing Transfer-Encoding as faulty framing. + client.stream.write "GET / HTTP/1.0\r\nHost: localhost\r\nConnection: keep-alive\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /HTTP\/1.0/) + + expect(server).not.to be(:persistent) + end + + it "fails with HTTP/1.0 request with transfer-encoding and content-length" do + client.stream.write "GET / HTTP/1.0\r\nHost: localhost\r\nContent-Length: 11\r\nTransfer-Encoding: chunked\r\n\r\nHello World" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest) + + expect(server).not.to be(:persistent) + end + + it "fails with request whose final transfer-encoding is not chunked" do + # RFC 9112 Section 6.3: the server MUST respond with 400 and close the connection, since the body length cannot be determined reliably. + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: gzip\r\n\r\nHello World" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /chunked/) + + expect(server).not.to be(:persistent) + end + + it "fails with request where chunked is not the final transfer-encoding" do + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked, gzip\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest) + + expect(server).not.to be(:persistent) + end + + it "fails with request containing both transfer-encoding and content-length" do + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 11\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest) + + expect(server).not.to be(:persistent) + end + it "reads request with CONNECT method" do client.stream.write "CONNECT localhost:443 HTTP/1.1\r\nHost: localhost\r\n\r\n" client.stream.close @@ -333,6 +402,46 @@ expect(result).to be == :yielded end + + it "should read chunked response" do + client.open! + + server.stream.write("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n") + server.stream.close + + version, status, reason, headers, body = client.read_response("GET") + + expect(headers).to be == {} + expect(body.join).to be == "Hello World" + expect(client).to be(:persistent) + end + + it "should read response with non-chunked transfer-encoding until closed" do + client.open! + + # RFC 9112 Section 6.3: for a response, the body is delimited by closing the connection. + server.stream.write("HTTP/1.1 200 OK\r\nTransfer-Encoding: gzip\r\n\r\nHello World") + server.stream.close + + version, status, reason, headers, body = client.read_response("GET") + + expect(body).to be_a(Protocol::HTTP1::Body::Remainder) + expect(body.join).to be == "Hello World" + expect(client).not.to be(:persistent) + end + + it "should fail with HTTP/1.0 response with transfer-encoding" do + client.open! + + server.stream.write("HTTP/1.0 200 OK\r\nConnection: keep-alive\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n") + server.stream.close + + expect do + client.read_response("GET") + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /HTTP\/1.0/) + + expect(client).not.to be(:persistent) + end end with "#read_response_body" do From 81bee0f77bba887ab9fa162f2ebb69602145c32e Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:12:52 +1300 Subject: [PATCH 02/10] Require `chunked` to be the sole request transfer coding and reject duplicate `chunked` codings. --- lib/protocol/http1/connection.rb | 51 ++++++++++++++------- releases.md | 1 + test/protocol/http1/connection.rb | 76 +++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 16 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 74a7f4a..18bcf96 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -1004,7 +1004,7 @@ def read_request_body(method, headers, version = nil) # - Otherwise, if the `content-length` is present, the body is read until the content length is reached. # - Otherwise, if `remainder` is true, the body is read until the connection is closed. # - # Framing errors (e.g. `transfer-encoding` in an HTTP/1.0 message, or a request whose final transfer coding is not `chunked`) are rejected and mark the connection as non-persistent, since the remainder of the stream can no longer be delimited reliably. + # Framing errors (e.g. `transfer-encoding` in an HTTP/1.0 message, `chunked` applied more than once, or a request whose transfer coding is anything other than exactly `chunked`) are rejected and mark the connection as non-persistent, since the remainder of the stream can no longer be delimited reliably. # # @parameter headers [Hash] the headers of the message. # @parameter remainder [Boolean] whether to read the remainder of the body, i.e. the message is a response which may be delimited by closing the connection. @@ -1038,22 +1038,41 @@ def read_body(headers, remainder = false, version = nil) raise BadRequest, "Message contains both transfer encoding and content length!" end - if transfer_encoding.last == CHUNKED - return read_chunked_body(headers) - elsif remainder - # If a Transfer-Encoding header field is present in a response and - # the chunked transfer coding is not the final encoding, the - # message body length is determined by reading the connection until - # it is closed by the server. - return read_remainder_body - else - # If a Transfer-Encoding header field is present in a request and - # the chunked transfer coding is not the final encoding, the - # message body length cannot be determined reliably; the server - # MUST respond with the 400 (Bad Request) status code and then - # close the connection. + # A sender MUST NOT apply the chunked transfer coding more than once + # to a message body (RFC 9112 Section 7.1). Duplicate codings would + # require decoding the body twice, which we do not support, so the + # framing cannot be trusted. + if transfer_encoding.count(CHUNKED) > 1 @persistent = false - raise BadRequest, "Request transfer encoding does not end with chunked: #{transfer_encoding.inspect}" + raise BadRequest, "Message contains chunked transfer encoding more than once: #{transfer_encoding.inspect}" + end + + if remainder + if transfer_encoding.last == CHUNKED + return read_chunked_body(headers) + else + # If a Transfer-Encoding header field is present in a response and + # the chunked transfer coding is not the final encoding, the + # message body length is determined by reading the connection until + # it is closed by the server. + return read_remainder_body + end + else + # A request body may only use the chunked transfer coding on its + # own: we do not decode any other transfer coding, and a server + # that receives a transfer coding it does not understand SHOULD + # respond with 501 (Not Implemented) (RFC 9112 Section 6.1). If + # chunked is not the final encoding, the message body length cannot + # be determined reliably; the server MUST respond with the 400 (Bad + # Request) status code and then close the connection. In both cases + # the header is consumed here, so accepting the body would hand the + # application an encoded body with no indication of the coding. + if transfer_encoding == [CHUNKED] + return read_chunked_body(headers) + else + @persistent = false + raise BadRequest, "Request transfer encoding must be chunked: #{transfer_encoding.inspect}" + end end end diff --git a/releases.md b/releases.md index f289709..beedff6 100644 --- a/releases.md +++ b/releases.md @@ -3,6 +3,7 @@ ## Unreleased - Reject `transfer-encoding` in HTTP/1.0 messages and requests whose final transfer coding is not `chunked` with `BadRequest`, and mark the connection as non-persistent, as required by RFC 9112 for reliable message framing. + - Reject requests whose `transfer-encoding` is anything other than exactly `chunked` (e.g. `gzip, chunked`), since no other transfer coding is decoded and the application would otherwise receive a still-encoded body. Reject any message that applies `chunked` more than once, including across multiple `transfer-encoding` headers. ## v0.41.0 diff --git a/test/protocol/http1/connection.rb b/test/protocol/http1/connection.rb index 32c7428..47ad7f8 100644 --- a/test/protocol/http1/connection.rb +++ b/test/protocol/http1/connection.rb @@ -214,6 +214,41 @@ expect(server).not.to be(:persistent) end + it "fails with request where chunked is preceded by another transfer-encoding" do + # RFC 9112 Section 6.1: a server that receives a transfer coding it does not understand SHOULD reject the request, rather than silently handing the application a still-encoded body. + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: gzip, chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /must be chunked/) + + expect(server).not.to be(:persistent) + end + + it "fails with request with duplicate chunked transfer-encoding" do + # RFC 9112 Section 7.1: chunked MUST NOT be applied more than once to a message body. + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked, chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /more than once/) + + expect(server).not.to be(:persistent) + end + + it "fails with request with multiple chunked transfer-encoding headers" do + client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /more than once/) + + expect(server).not.to be(:persistent) + end + it "fails with request containing both transfer-encoding and content-length" do client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 11\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" client.stream.close @@ -430,6 +465,47 @@ expect(client).not.to be(:persistent) end + it "should read response where chunked is preceded by another transfer-encoding" do + client.open! + + # RFC 9112 Section 6.1: chunked is the final encoding, so the body is chunked; the application is responsible for the remaining gzip transfer coding. + server.stream.write("HTTP/1.1 200 OK\r\nTransfer-Encoding: gzip, chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n") + server.stream.close + + version, status, reason, headers, body = client.read_response("GET") + + expect(body).to be_a(Protocol::HTTP1::Body::Chunked) + expect(body.join).to be == "Hello World" + expect(client).to be(:persistent) + end + + it "should fail with response with duplicate chunked transfer-encoding" do + client.open! + + # RFC 9112 Section 7.1: chunked MUST NOT be applied more than once to a message body. + server.stream.write("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked, chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n") + server.stream.close + + expect do + client.read_response("GET") + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /more than once/) + + expect(client).not.to be(:persistent) + end + + it "should fail with response with multiple chunked transfer-encoding headers" do + client.open! + + server.stream.write("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n") + server.stream.close + + expect do + client.read_response("GET") + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /more than once/) + + expect(client).not.to be(:persistent) + end + it "should fail with HTTP/1.0 response with transfer-encoding" do client.open! From c01dc9cf445372a9f830ae106f79741ad7b7f57f Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:15:55 +1300 Subject: [PATCH 03/10] Only accept `HTTP/1.x` versions in request and status lines. --- lib/protocol/http1/connection.rb | 11 ++++++- releases.md | 1 + test/protocol/http1/connection.rb | 50 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 18bcf96..11e4956 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -34,7 +34,10 @@ module HTTP1 # HTTP/1.x request line parser: TOKEN = /[!#$%&'*+\-\.\^_`|~0-9a-zA-Z]+/.freeze VALID_METHOD = /\A#{TOKEN}\z/.freeze - REQUEST_LINE = /\A(#{TOKEN}) ([^\s]+) (HTTP\/\d.\d)\z/.freeze + # The HTTP-version is case-sensitive (RFC 9112 Section 2.3). Only the HTTP/1.x family is accepted; any minor version is treated as HTTP/1.1 (RFC 9110 Section 2.5): + HTTP_VERSION = /HTTP\/1\.\d/.freeze + VALID_HTTP_VERSION = /\A#{HTTP_VERSION}\z/.freeze + REQUEST_LINE = /\A(#{TOKEN}) ([^\s]+) (#{HTTP_VERSION})\z/.freeze # HTTP/1.x header parser: FIELD_NAME = TOKEN @@ -418,6 +421,7 @@ def read_line # Read a request line from the connection. # # @returns [Tuple(String, String, String) | Nil] the method, path, and version of the request, or nil if the connection is closed. + # @raises [InvalidRequest] if the request line is malformed, including when the HTTP version is not `HTTP/1.x`. def read_request_line return unless line = read_line? @@ -470,9 +474,14 @@ def read_request # # @returns [Tuple(String, Integer, String)] the version, status, and reason of the response. # @raises [EOFError] if the connection is closed. + # @raises [BadRequest] if the HTTP version is not `HTTP/1.x`. def read_response_line version, status, reason = read_line.split(/\s+/, 3) + unless version&.match?(VALID_HTTP_VERSION) + raise BadRequest, "Invalid response version: #{version.inspect}" + end + status = Integer(status) return version, status, reason diff --git a/releases.md b/releases.md index beedff6..19efd63 100644 --- a/releases.md +++ b/releases.md @@ -4,6 +4,7 @@ - Reject `transfer-encoding` in HTTP/1.0 messages and requests whose final transfer coding is not `chunked` with `BadRequest`, and mark the connection as non-persistent, as required by RFC 9112 for reliable message framing. - Reject requests whose `transfer-encoding` is anything other than exactly `chunked` (e.g. `gzip, chunked`), since no other transfer coding is decoded and the application would otherwise receive a still-encoded body. Reject any message that applies `chunked` more than once, including across multiple `transfer-encoding` headers. + - Only accept `HTTP/1.x` versions (case-sensitive) in request lines (`InvalidRequest`) and response status lines (`BadRequest`). Previously versions such as `HTTP/2.0`, `HTTP/0.9`, `HTTP/1x1` or `http/1.1` were accepted and treated as HTTP/1.1. ## v0.41.0 diff --git a/test/protocol/http1/connection.rb b/test/protocol/http1/connection.rb index 47ad7f8..41f014c 100644 --- a/test/protocol/http1/connection.rb +++ b/test/protocol/http1/connection.rb @@ -293,6 +293,29 @@ end.to raise_exception(Protocol::HTTP1::InvalidRequest) end + ["HTTP/0.9", "HTTP/2.0", "HTTP/1x1", "http/1.1"].each do |version| + it "fails with unsupported version #{version}", unique: version do + client.stream.write "GET / #{version}\r\nHost: localhost\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::InvalidRequest) + end + end + + it "reads request with unknown HTTP/1.x minor version" do + # RFC 9110 Section 2.5: a recipient treats an unknown minor version as the highest minor version it supports. + client.stream.write "GET / HTTP/1.2\r\nHost: localhost\r\nContent-Length: 0\r\n\r\n" + client.stream.close + + authority, method, target, version, headers, body = server.read_request + + expect(version).to be == "HTTP/1.2" + expect(body).to be_nil + expect(server).to be(:persistent) + end + it "yields to block" do client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 0\r\n\r\n" client.stream.close @@ -438,6 +461,33 @@ expect(result).to be == :yielded end + ["http/1.1", "HTTP/2.0", "HTTP/1x1", "HTTP/0.9"].each do |version| + it "should fail with unsupported version #{version}", unique: version do + client.open! + + # RFC 9112 Section 2.3: the HTTP-version is case-sensitive. + server.stream.write("#{version} 200 OK\r\nContent-Length: 0\r\n\r\n") + server.stream.close + + expect do + client.read_response("GET") + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /version/) + end + end + + it "should read response with unknown HTTP/1.x minor version" do + client.open! + + server.stream.write("HTTP/1.2 200 OK\r\nContent-Length: 0\r\n\r\n") + server.stream.close + + version, status, reason, headers, body = client.read_response("GET") + + expect(version).to be == "HTTP/1.2" + expect(status).to be == 200 + expect(body).to be_nil + end + it "should read chunked response" do client.open! From b7e6bba5c5714797bb00937c0c1879ebe9f4ddae Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:16:39 +1300 Subject: [PATCH 04/10] Ignore the `upgrade` header in HTTP/1.0 requests. --- lib/protocol/http1/connection.rb | 6 ++++-- releases.md | 1 + test/protocol/http1/upgrade.rb | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 11e4956..c9299a3 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -981,7 +981,7 @@ def read_response_body(method, status, headers, version = nil) # Read the body of the request. # # - The `CONNECT` method is used to establish a tunnel, so the body is read until the connection is closed. - # - The `UPGRADE` method is used to upgrade the connection to a different protocol (typically WebSockets), so the body is read until the connection is closed. + # - The `upgrade` header is used to upgrade the connection to a different protocol (typically WebSockets), so the body is read until the connection is closed. It is ignored in HTTP/1.0 requests, as required by RFC 9110 Section 7.8. # - Otherwise, the body is read according to {read_body}. # # @parameter method [String] the HTTP method. @@ -998,7 +998,9 @@ def read_request_body(method, headers, version = nil) end # A successful upgrade response implies that the connection will become a tunnel immediately after the empty line that concludes the header fields. - if headers[UPGRADE] + # + # A server that receives an Upgrade header field in an HTTP/1.0 request MUST ignore that Upgrade field (RFC 9110 Section 7.8), so the body is framed according to the remaining headers instead. + if headers[UPGRADE] and version != HTTP10 return read_upgrade_body end diff --git a/releases.md b/releases.md index 19efd63..125a18d 100644 --- a/releases.md +++ b/releases.md @@ -5,6 +5,7 @@ - Reject `transfer-encoding` in HTTP/1.0 messages and requests whose final transfer coding is not `chunked` with `BadRequest`, and mark the connection as non-persistent, as required by RFC 9112 for reliable message framing. - Reject requests whose `transfer-encoding` is anything other than exactly `chunked` (e.g. `gzip, chunked`), since no other transfer coding is decoded and the application would otherwise receive a still-encoded body. Reject any message that applies `chunked` more than once, including across multiple `transfer-encoding` headers. - Only accept `HTTP/1.x` versions (case-sensitive) in request lines (`InvalidRequest`) and response status lines (`BadRequest`). Previously versions such as `HTTP/2.0`, `HTTP/0.9`, `HTTP/1x1` or `http/1.1` were accepted and treated as HTTP/1.1. + - Ignore the `upgrade` header in HTTP/1.0 requests, as required by RFC 9110 Section 7.8, so the request body is framed by the remaining headers rather than being read until the connection is closed. ## v0.41.0 diff --git a/test/protocol/http1/upgrade.rb b/test/protocol/http1/upgrade.rb index 23a474a..51be788 100644 --- a/test/protocol/http1/upgrade.rb +++ b/test/protocol/http1/upgrade.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Released under the MIT License. -# Copyright, 2019-2024, by Samuel Williams. +# Copyright, 2019-2026, by Samuel Williams. require "protocol/http1/connection" require "connection_context" @@ -11,7 +11,7 @@ with "#upgrade" do let(:protocol) {"binary"} - let(:request_version) {Protocol::HTTP1::Connection::HTTP10} + let(:request_version) {Protocol::HTTP1::Connection::HTTP11} it "should upgrade connection" do client.write_request("testing.com", "GET", "/", request_version, []) @@ -29,5 +29,33 @@ stream = server.hijack! expect(stream.read).to be == "Hello World" end + + with "HTTP/1.0 request" do + let(:request_version) {Protocol::HTTP1::Connection::HTTP10} + + it "should ignore upgrade header" do + # RFC 9110 Section 7.8: a server MUST ignore the Upgrade header field in an HTTP/1.0 request. + client.write_request("testing.com", "GET", "/", request_version, []) + client.write_upgrade_body(protocol) + + authority, method, path, version, headers, body = server.read_request + + expect(version).to be == request_version + expect(headers["upgrade"]).to be == [protocol] + expect(body).to be_nil + expect(server).not.to be(:persistent) + end + + it "should not bypass transfer-encoding framing checks" do + client.stream.write "GET / HTTP/1.0\r\nHost: localhost\r\nConnection: keep-alive\r\nUpgrade: #{protocol}\r\nTransfer-Encoding: chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n" + client.stream.close + + expect do + server.read_request + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /HTTP\/1.0/) + + expect(server).not.to be(:persistent) + end + end end end From 5dba537d8e68cd148c22685d78031254aadcde8d Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:17:21 +1300 Subject: [PATCH 05/10] Document `BadRequest` raised by request and response readers and the HTTP/1.0 `transfer-encoding` trade-off. --- lib/protocol/http1/connection.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index c9299a3..3510dfd 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -441,6 +441,7 @@ def read_request_line # @yields {|host, method, path, version, headers, body| ...} if a block is given. # @returns [Tuple(String, String, String, String, HTTP::Headers, Protocol::HTTP1::Body) | Nil] the host, method, path, version, headers, and body of the request, or `nil` if the connection is closed. # @raises [ProtocolError] if the connection is not in the idle state. + # @raises [BadRequest] if the request line, headers, or body framing are invalid, e.g. an unsupported HTTP version, a malformed header, or a `transfer-encoding` that cannot be used to delimit the body. def read_request open! @@ -502,6 +503,7 @@ def interim_status?(status) # @returns [Tuple(String, Integer, String, HTTP::Headers, Protocol::HTTP1::Body)] the version, status, reason, headers, and body of the response. # @raises [ProtocolError] if the connection is not in the open or half-closed local state. # @raises [EOFError] if the connection is closed. + # @raises [BadRequest] if the status line, headers, or body framing are invalid, e.g. an unsupported HTTP version, a malformed header, or a `transfer-encoding` that cannot be used to delimit the body. def read_response(method) unless @state == :open or @state == :half_closed_local raise ProtocolError, "Cannot read response in state: #{@state}!" @@ -938,6 +940,8 @@ def extract_content_length(headers) # @parameter status [Integer] the HTTP status code. # @parameter headers [Hash] the headers of the response. # @parameter version [String | Nil] the HTTP version of the response, if known. + # @returns [Object | Nil] the body, or `nil` if the response has no body. + # @raises [BadRequest] if the `content-length` is invalid or the message framing is invalid, see {read_body}. def read_response_body(method, status, headers, version = nil) # RFC 7230 3.3.3 # 1. Any response to a HEAD request and any response with a 1xx @@ -987,6 +991,8 @@ def read_response_body(method, status, headers, version = nil) # @parameter method [String] the HTTP method. # @parameter headers [Hash] the headers of the request. # @parameter version [String | Nil] the HTTP version of the request, if known. + # @returns [Object | Nil] the body, or `nil` if the request has no body. + # @raises [BadRequest] if the `content-length` is invalid or the message framing is invalid, see {read_body}. def read_request_body(method, headers, version = nil) # 2. Any 2xx (Successful) response to a CONNECT request implies that # the connection will become a tunnel immediately after the empty @@ -1017,6 +1023,8 @@ def read_request_body(method, headers, version = nil) # # Framing errors (e.g. `transfer-encoding` in an HTTP/1.0 message, `chunked` applied more than once, or a request whose transfer coding is anything other than exactly `chunked`) are rejected and mark the connection as non-persistent, since the remainder of the stream can no longer be delimited reliably. # + # Rejecting HTTP/1.0 responses with `transfer-encoding` follows RFC 9112 Section 6.1; browsers instead ignore the header and read until the connection is closed. If interoperability with such servers ever requires relaxing this, the correct relaxation is close-delimited reading via {read_remainder_body}, never chunked decoding, since an HTTP/1.0 peer cannot be relied upon to have applied the coding. + # # @parameter headers [Hash] the headers of the message. # @parameter remainder [Boolean] whether to read the remainder of the body, i.e. the message is a response which may be delimited by closing the connection. # @parameter version [String | Nil] the HTTP version of the message, if known. From c5cf73f89306605be4cdb03c74bf7639ad32809f Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:18:18 +1300 Subject: [PATCH 06/10] Make HTTP/1.0 persistence assertion meaningful and pin `read_body` framing checks without a version. --- test/protocol/http1/connection.rb | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/test/protocol/http1/connection.rb b/test/protocol/http1/connection.rb index 41f014c..0132262 100644 --- a/test/protocol/http1/connection.rb +++ b/test/protocol/http1/connection.rb @@ -181,7 +181,7 @@ end it "fails with HTTP/1.0 request with transfer-encoding and content-length" do - client.stream.write "GET / HTTP/1.0\r\nHost: localhost\r\nContent-Length: 11\r\nTransfer-Encoding: chunked\r\n\r\nHello World" + client.stream.write "GET / HTTP/1.0\r\nHost: localhost\r\nConnection: keep-alive\r\nContent-Length: 11\r\nTransfer-Encoding: chunked\r\n\r\nHello World" client.stream.close expect do @@ -626,6 +626,31 @@ end end + with "#read_body" do + # Callers using the original signature do not pass a version, so framing must still be validated without it. + it "reads chunked request body without a version" do + body = server.read_body({"transfer-encoding" => ["chunked"]}, false) + + expect(body).to be_a(Protocol::HTTP1::Body::Chunked) + expect(server).to be(:persistent) + end + + it "fails with non-chunked request transfer-encoding without a version" do + expect do + server.read_body({"transfer-encoding" => ["gzip"]}, false) + end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /must be chunked/) + + expect(server).not.to be(:persistent) + end + + it "reads non-chunked response transfer-encoding until closed without a version" do + body = client.read_body({"transfer-encoding" => ["gzip"]}, true) + + expect(body).to be_a(Protocol::HTTP1::Body::Remainder) + expect(client).not.to be(:persistent) + end + end + with "#write_chunked_body" do let(:chunks) {["Hello", "World"]} let(:body) {::Protocol::HTTP::Body::Buffered.wrap(chunks)} From ca8a5bbd0366fd6cde407e2d8ef7e2c238ab29d2 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Thu, 5 Nov 2026 23:01:36 +1300 Subject: [PATCH 07/10] Shorten the framing documentation. --- lib/protocol/http1/connection.rb | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 3510dfd..35a7d7a 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -34,7 +34,7 @@ module HTTP1 # HTTP/1.x request line parser: TOKEN = /[!#$%&'*+\-\.\^_`|~0-9a-zA-Z]+/.freeze VALID_METHOD = /\A#{TOKEN}\z/.freeze - # The HTTP-version is case-sensitive (RFC 9112 Section 2.3). Only the HTTP/1.x family is accepted; any minor version is treated as HTTP/1.1 (RFC 9110 Section 2.5): + # The HTTP-version is case-sensitive, and only HTTP/1.x is accepted: HTTP_VERSION = /HTTP\/1\.\d/.freeze VALID_HTTP_VERSION = /\A#{HTTP_VERSION}\z/.freeze REQUEST_LINE = /\A(#{TOKEN}) ([^\s]+) (#{HTTP_VERSION})\z/.freeze @@ -441,7 +441,7 @@ def read_request_line # @yields {|host, method, path, version, headers, body| ...} if a block is given. # @returns [Tuple(String, String, String, String, HTTP::Headers, Protocol::HTTP1::Body) | Nil] the host, method, path, version, headers, and body of the request, or `nil` if the connection is closed. # @raises [ProtocolError] if the connection is not in the idle state. - # @raises [BadRequest] if the request line, headers, or body framing are invalid, e.g. an unsupported HTTP version, a malformed header, or a `transfer-encoding` that cannot be used to delimit the body. + # @raises [BadRequest] if the request line, headers, or body framing are invalid. def read_request open! @@ -503,7 +503,7 @@ def interim_status?(status) # @returns [Tuple(String, Integer, String, HTTP::Headers, Protocol::HTTP1::Body)] the version, status, reason, headers, and body of the response. # @raises [ProtocolError] if the connection is not in the open or half-closed local state. # @raises [EOFError] if the connection is closed. - # @raises [BadRequest] if the status line, headers, or body framing are invalid, e.g. an unsupported HTTP version, a malformed header, or a `transfer-encoding` that cannot be used to delimit the body. + # @raises [BadRequest] if the status line, headers, or body framing are invalid. def read_response(method) unless @state == :open or @state == :half_closed_local raise ProtocolError, "Cannot read response in state: #{@state}!" @@ -985,7 +985,7 @@ def read_response_body(method, status, headers, version = nil) # Read the body of the request. # # - The `CONNECT` method is used to establish a tunnel, so the body is read until the connection is closed. - # - The `upgrade` header is used to upgrade the connection to a different protocol (typically WebSockets), so the body is read until the connection is closed. It is ignored in HTTP/1.0 requests, as required by RFC 9110 Section 7.8. + # - The `upgrade` header is used to upgrade the connection to a different protocol (typically WebSockets), so the body is read until the connection is closed. It is ignored in HTTP/1.0 requests. # - Otherwise, the body is read according to {read_body}. # # @parameter method [String] the HTTP method. @@ -1003,9 +1003,7 @@ def read_request_body(method, headers, version = nil) return read_tunnel_body end - # A successful upgrade response implies that the connection will become a tunnel immediately after the empty line that concludes the header fields. - # - # A server that receives an Upgrade header field in an HTTP/1.0 request MUST ignore that Upgrade field (RFC 9110 Section 7.8), so the body is framed according to the remaining headers instead. + # A successful upgrade response implies that the connection will become a tunnel immediately after the empty line that concludes the header fields. An HTTP/1.0 request cannot upgrade (RFC 9110 Section 7.8): if headers[UPGRADE] and version != HTTP10 return read_upgrade_body end @@ -1021,9 +1019,7 @@ def read_request_body(method, headers, version = nil) # - Otherwise, if the `content-length` is present, the body is read until the content length is reached. # - Otherwise, if `remainder` is true, the body is read until the connection is closed. # - # Framing errors (e.g. `transfer-encoding` in an HTTP/1.0 message, `chunked` applied more than once, or a request whose transfer coding is anything other than exactly `chunked`) are rejected and mark the connection as non-persistent, since the remainder of the stream can no longer be delimited reliably. - # - # Rejecting HTTP/1.0 responses with `transfer-encoding` follows RFC 9112 Section 6.1; browsers instead ignore the header and read until the connection is closed. If interoperability with such servers ever requires relaxing this, the correct relaxation is close-delimited reading via {read_remainder_body}, never chunked decoding, since an HTTP/1.0 peer cannot be relied upon to have applied the coding. + # Invalid framing is rejected and marks the connection as non-persistent, since the rest of the stream can no longer be delimited. # # @parameter headers [Hash] the headers of the message. # @parameter remainder [Boolean] whether to read the remainder of the body, i.e. the message is a response which may be delimited by closing the connection. From 5c458c7d4e8531bac81640dfafdee5ac34cecbfc Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 6 Nov 2026 09:11:52 +1300 Subject: [PATCH 08/10] Use static exception messages rather than interpolating received data. --- lib/protocol/http1/connection.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 35a7d7a..bc7414d 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -480,7 +480,7 @@ def read_response_line version, status, reason = read_line.split(/\s+/, 3) unless version&.match?(VALID_HTTP_VERSION) - raise BadRequest, "Invalid response version: #{version.inspect}" + raise BadRequest, "Invalid response version!" end status = Integer(status) @@ -1059,7 +1059,7 @@ def read_body(headers, remainder = false, version = nil) # framing cannot be trusted. if transfer_encoding.count(CHUNKED) > 1 @persistent = false - raise BadRequest, "Message contains chunked transfer encoding more than once: #{transfer_encoding.inspect}" + raise BadRequest, "Message contains chunked transfer encoding more than once!" end if remainder @@ -1086,7 +1086,7 @@ def read_body(headers, remainder = false, version = nil) return read_chunked_body(headers) else @persistent = false - raise BadRequest, "Request transfer encoding must be chunked: #{transfer_encoding.inspect}" + raise BadRequest, "Request transfer encoding must be chunked!" end end end From a3b648b27ddb8ec424503d3bc77664766ccda92b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 6 Nov 2026 09:19:46 +1300 Subject: [PATCH 09/10] Rename the `remainder` argument of `read_body` to `request`, matching `write_body`. --- lib/protocol/http1/connection.rb | 34 +++++++++++++++---------------- test/protocol/http1/connection.rb | 16 +++++++-------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index bc7414d..d027d51 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -979,7 +979,7 @@ def read_response_body(method, status, headers, version = nil) return read_tunnel_body end - return read_body(headers, true, version) + return read_body(headers, false, version) end # Read the body of the request. @@ -1010,23 +1010,23 @@ def read_request_body(method, headers, version = nil) # 6. If this is a request message and none of the above are true, then # the message body length is zero (no message body is present). - return read_body(headers, false, version) + return read_body(headers, true, version) end # Read the body of the message. # # - The `transfer-encoding` header is used to determine if the body is chunked. # - Otherwise, if the `content-length` is present, the body is read until the content length is reached. - # - Otherwise, if `remainder` is true, the body is read until the connection is closed. + # - Otherwise, if the message is a response, the body is read until the connection is closed. # # Invalid framing is rejected and marks the connection as non-persistent, since the rest of the stream can no longer be delimited. # # @parameter headers [Hash] the headers of the message. - # @parameter remainder [Boolean] whether to read the remainder of the body, i.e. the message is a response which may be delimited by closing the connection. + # @parameter request [Boolean] whether the message is a request, which cannot be delimited by closing the connection. # @parameter version [String | Nil] the HTTP version of the message, if known. # @returns [Object] the body. # @raises [BadRequest] if the message framing is invalid. - def read_body(headers, remainder = false, version = nil) + def read_body(headers, request = false, version = nil) # 3. If a Transfer-Encoding header field is present and the chunked # transfer coding (Section 4.1) is the final encoding, the message # body length is determined by reading and decoding the chunked @@ -1062,17 +1062,7 @@ def read_body(headers, remainder = false, version = nil) raise BadRequest, "Message contains chunked transfer encoding more than once!" end - if remainder - if transfer_encoding.last == CHUNKED - return read_chunked_body(headers) - else - # If a Transfer-Encoding header field is present in a response and - # the chunked transfer coding is not the final encoding, the - # message body length is determined by reading the connection until - # it is closed by the server. - return read_remainder_body - end - else + if request # A request body may only use the chunked transfer coding on its # own: we do not decode any other transfer coding, and a server # that receives a transfer coding it does not understand SHOULD @@ -1088,6 +1078,16 @@ def read_body(headers, remainder = false, version = nil) @persistent = false raise BadRequest, "Request transfer encoding must be chunked!" end + else + if transfer_encoding.last == CHUNKED + return read_chunked_body(headers) + else + # If a Transfer-Encoding header field is present in a response and + # the chunked transfer coding is not the final encoding, the + # message body length is determined by reading the connection until + # it is closed by the server. + return read_remainder_body + end end end @@ -1106,7 +1106,7 @@ def read_body(headers, remainder = false, version = nil) end # http://tools.ietf.org/html/rfc2068#section-19.7.1.1 - if remainder + unless request # 7. Otherwise, this is a response message without a declared message # body length, so the message body length is determined by the # number of octets received prior to the server closing the diff --git a/test/protocol/http1/connection.rb b/test/protocol/http1/connection.rb index 0132262..4597a5f 100644 --- a/test/protocol/http1/connection.rb +++ b/test/protocol/http1/connection.rb @@ -629,7 +629,7 @@ with "#read_body" do # Callers using the original signature do not pass a version, so framing must still be validated without it. it "reads chunked request body without a version" do - body = server.read_body({"transfer-encoding" => ["chunked"]}, false) + body = server.read_body({"transfer-encoding" => ["chunked"]}, true) expect(body).to be_a(Protocol::HTTP1::Body::Chunked) expect(server).to be(:persistent) @@ -637,14 +637,14 @@ it "fails with non-chunked request transfer-encoding without a version" do expect do - server.read_body({"transfer-encoding" => ["gzip"]}, false) + server.read_body({"transfer-encoding" => ["gzip"]}, true) end.to raise_exception(Protocol::HTTP1::BadRequest, message: be =~ /must be chunked/) expect(server).not.to be(:persistent) end it "reads non-chunked response transfer-encoding until closed without a version" do - body = client.read_body({"transfer-encoding" => ["gzip"]}, true) + body = client.read_body({"transfer-encoding" => ["gzip"]}, false) expect(body).to be_a(Protocol::HTTP1::Body::Remainder) expect(client).not.to be(:persistent) @@ -771,7 +771,7 @@ "upgrade" => be == ["text"] ) - body = client.read_body(headers, true) + body = client.read_body(headers, false) expect(body.join).to be == "Hello World!" end end @@ -793,7 +793,7 @@ "connection" => be == ["close"], ) - body = client.read_body(headers, true) + body = client.read_body(headers, false) expect(body.join).to be == "Hello World!" end end @@ -813,7 +813,7 @@ headers = client.read_headers expect(headers).to be(:empty?) - body = client.read_body(headers, true) + body = client.read_body(headers, false) expect(body.join).to be == "Hello World!" end @@ -848,7 +848,7 @@ headers = client.read_headers expect(headers).to be == [["connection", "keep-alive"], ["content-length", "0"]] - body = client.read_body(headers, true) + body = client.read_body(headers, false) expect(body).to be_nil end @@ -860,7 +860,7 @@ headers = client.read_headers expect(headers).to be == [["connection", "keep-alive"], ["content-length", "0"]] - body = client.read_body(headers, true) + body = client.read_body(headers, false) expect(body).to be_nil end From 366c75dca3bf4717db01fb4a8203b636fa746329 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 6 Nov 2026 09:20:18 +1300 Subject: [PATCH 10/10] Note the `read_body` argument rename in the release notes. --- releases.md | 1 + 1 file changed, 1 insertion(+) diff --git a/releases.md b/releases.md index 125a18d..3d7f979 100644 --- a/releases.md +++ b/releases.md @@ -6,6 +6,7 @@ - Reject requests whose `transfer-encoding` is anything other than exactly `chunked` (e.g. `gzip, chunked`), since no other transfer coding is decoded and the application would otherwise receive a still-encoded body. Reject any message that applies `chunked` more than once, including across multiple `transfer-encoding` headers. - Only accept `HTTP/1.x` versions (case-sensitive) in request lines (`InvalidRequest`) and response status lines (`BadRequest`). Previously versions such as `HTTP/2.0`, `HTTP/0.9`, `HTTP/1x1` or `http/1.1` were accepted and treated as HTTP/1.1. - Ignore the `upgrade` header in HTTP/1.0 requests, as required by RFC 9110 Section 7.8, so the request body is framed by the remaining headers rather than being read until the connection is closed. + - `Protocol::HTTP1::Connection#read_body` takes `request` instead of `remainder` as its second argument, matching `write_body`. Callers passing `true` to read a response body should now pass `false`. ## v0.41.0