Skip to content

Fix header parsing, allowing for \t characters between VCHAR. #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions lib/protocol/http1/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ module HTTP1

# HTTP/1.x header parser:
FIELD_NAME = TOKEN
FIELD_VALUE = /[^\000-\037]*/.freeze
HEADER = /\A(#{FIELD_NAME}):\s*(#{FIELD_VALUE})\s*\z/.freeze
WS = /[ \t]/ # Whitespace.
OWS = /#{WS}*/ # Optional whitespace.
VCHAR = /[!-~]/ # Match visible characters from ASCII 33 to 126.
FIELD_VALUE = /#{VCHAR}+(?:#{WS}+#{VCHAR}+)*/.freeze
HEADER = /\A(#{FIELD_NAME}):#{OWS}(?:(#{FIELD_VALUE})#{OWS})?\z/.freeze

VALID_FIELD_NAME = /\A#{FIELD_NAME}\z/.freeze
VALID_FIELD_VALUE = /\A#{FIELD_VALUE}\z/.freeze
Expand Down Expand Up @@ -484,7 +487,7 @@ def read_headers
break if line.empty?

if match = line.match(HEADER)
fields << [match[1], match[2]]
fields << [match[1], match[2] || ""]
else
raise BadHeader, "Could not parse header: #{line.inspect}"
end
Expand Down
106 changes: 106 additions & 0 deletions test/protocol/http1/connection/headers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# frozen_string_literal: true

# Released under the MIT License.
# Copyright, 2025, by Samuel Williams.

require "protocol/http1/connection"
require "connection_context"

describe Protocol::HTTP1::Connection do
include_context ConnectionContext

let(:headers) {Array.new}

before do
client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\n#{headers.join("\r\n")}\r\n\r\n"
client.stream.close
end

with "header that contains tab characters" do
let(:headers) {[
"user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) \t\t\tChrome/55.0.2883.95 Safari/537.36"
]}

it "can parse the header" do
authority, method, target, version, headers, body = server.read_request

expect(headers).to have_keys(
"user-agent" => be == "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) \t\t\tChrome/55.0.2883.95 Safari/537.36"
)
end
end

with "header that contains obsolete folding whitespace" do
let(:headers) {[
"user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko)\n\tChrome/55.0.2883.95 Safari/537.36"
]}

it "rejects the request" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadHeader)
end
end

with "header that contains invalid characters" do
let(:headers) {[
"user-agent: Mozilla\x00Hacker Browser"
]}

it "rejects the request" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadHeader)
end
end

with "header that contains invalid high characters" do
let(:headers) {[
"user-agent: Mozilla\x7FHacker Browser"
]}

it "rejects the request" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadHeader)
end
end

with "header that has empty value" do
let(:headers) {[
"user-agent: "
]}

it "can parse the header" do
authority, method, target, version, headers, body = server.read_request

expect(headers).to have_keys(
"user-agent" => be == ""
)
end
end

with "header that has invalid name" do
let(:headers) {[
"invalid name: value"
]}

it "rejects the request" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadHeader)
end
end

with "header that has empty name" do
let(:headers) {[
": value"
]}

it "rejects the request" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadHeader)
end
end
end
40 changes: 40 additions & 0 deletions test/protocol/http1/parser.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# frozen_string_literal: true

# Released under the MIT License.
# Copyright, 2025, by Samuel Williams.

require "protocol/http1/connection"

describe Protocol::HTTP1 do
describe "REQUEST_LINE" do
it "parses in linear time" do
skip_unless_method_defined(:linear_time?, Regexp.singleton_class)

expect(Regexp).to be(:linear_time?, Protocol::HTTP1::REQUEST_LINE)
end
end

describe "HEADER" do
it "parses in linear time" do
skip_unless_method_defined(:linear_time?, Regexp.singleton_class)

expect(Regexp).to be(:linear_time?, Protocol::HTTP1::HEADER)
end
end

describe "VALID_FIELD_NAME" do
it "parses in linear time" do
skip_unless_method_defined(:linear_time?, Regexp.singleton_class)

expect(Regexp).to be(:linear_time?, Protocol::HTTP1::VALID_FIELD_NAME)
end
end

describe "VALID_FIELD_VALUE" do
it "parses in linear time" do
skip_unless_method_defined(:linear_time?, Regexp.singleton_class)

expect(Regexp).to be(:linear_time?, Protocol::HTTP1::VALID_FIELD_VALUE)
end
end
end
Loading