From cebe8702f997c56cd5264b6ae5cb780d8403ab5c Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:19:04 +1200 Subject: [PATCH 1/8] Harden Accept header parsing --- lib/protocol/http/header/accept.rb | 72 +++++++++++++++++++++++------ test/protocol/http/header/accept.rb | 65 ++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 13 deletions(-) diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index ec5cdf4..1a1b1a5 100644 --- a/lib/protocol/http/header/accept.rb +++ b/lib/protocol/http/header/accept.rb @@ -8,21 +8,13 @@ require_relative "../quoted_string" require_relative "../error" +require "strscan" + module Protocol module HTTP module Header # The `accept-content-type` header represents a list of content-types that the client can accept. class Accept < Split - # Regular expression used to split values on commas, with optional surrounding whitespace, taking into account quoted strings. - SEPARATOR = / - (?: # Start non-capturing group - "[^"\\]*" # Match quoted strings (no escaping of quotes within) - | # OR - [^,"]+ # Match non-quoted strings until a comma or quote - )+ - (?=,|\z) # Match until a comma or end of string - /x - ParseError = Class.new(Error) MEDIA_RANGE = /\A(?#{TOKEN})\/(?#{TOKEN})(?.*)\z/ @@ -73,7 +65,9 @@ def quality_factor # @parameter value [String] a raw header value containing comma-separated media types. # @returns [Accept] a new instance containing the parsed media types. def self.parse(value) - self.new(value.scan(SEPARATOR).map(&:strip)) + raise TypeError, "Expected a String, got: #{value.class}" unless value.is_a?(String) + + self.new << value end # Adds one or more comma-separated values to the header. @@ -82,7 +76,7 @@ def self.parse(value) # # @parameter value [String] a raw header value containing one or more media types separated by commas. def << value - self.concat(value.scan(SEPARATOR).map(&:strip)) + self.concat(split_values(value)) end # Converts the parsed header value into a raw header value. @@ -109,13 +103,57 @@ def media_ranges private + def split_values(value) + return [] if value.empty? + + values = [] + start = 0 + quoted = false + escaped = false + + value.each_char.with_index do |character, index| + if quoted + if escaped + escaped = false + elsif character == "\\" + escaped = true + elsif character == '"' + quoted = false + end + elsif character == '"' + quoted = true + elsif character == "," + values << value[start...index].strip + start = index + 1 + end + end + + raise ParseError, "Unterminated quoted string: #{value.inspect}" if quoted + + values << value[start..].strip + return values + end + def parse_media_range(value) if match = value.match(MEDIA_RANGE) type = match[:type] subtype = match[:subtype] parameters = {} + scanner = StringScanner.new(match[:parameters]) - match[:parameters].scan(PARAMETER) do |key, value, quoted_value| + while scanner.scan(PARAMETER) + key = scanner[:key] + value = scanner[:value] + quoted_value = scanner[:quoted_value] + + if key.casecmp?("q") + if quoted_value || parameters.key?("q") || !value.match?(/\A(?:#{QVALUE})\z/) + raise ParseError, "Invalid quality factor: #{value.inspect}" + end + + key = "q" + end + if quoted_value value = QuotedString.unquote(quoted_value) end @@ -123,6 +161,14 @@ def parse_media_range(value) parameters[key] = value end + unless scanner.eos? + raise ParseError, "Invalid media range parameters: #{scanner.rest.inspect}" + end + + if (type.include?("*") && type != "*") || (subtype.include?("*") && subtype != "*") || (type == "*" && subtype != "*") + raise ParseError, "Invalid wildcards in media range: #{type}/#{subtype}" + end + return MediaRange.new(type, subtype, parameters) else raise ParseError, "Invalid media type: #{value.inspect}" diff --git a/test/protocol/http/header/accept.rb b/test/protocol/http/header/accept.rb index 20630a0..0288b1c 100644 --- a/test/protocol/http/header/accept.rb +++ b/test/protocol/http/header/accept.rb @@ -84,6 +84,71 @@ end end + with 'foo/bar;key="A,B,C", text/plain;note="a,b; c\\"d"' do + it "preserves commas and escapes in quoted parameters" do + expect(header.length).to be == 2 + expect(media_ranges[0].parameters).to have_keys( + "key" => be == "A,B,C", + ) + expect(media_ranges[1].parameters).to have_keys( + "note" => be == 'a,b; c"d', + ) + end + end + + with "invalid media ranges" do + it "rejects malformed parameters" do + [ + ",", + "text/html,", + "text/html;", + "text/html;invalid", + "text/html;charset=", + 'text/html;charset="unterminated', + ].each do |value| + expect{subject.parse(value).media_ranges}.to raise_exception(Protocol::HTTP::Header::Accept::ParseError) + end + end + + it "rejects invalid wildcard forms" do + [ + "*/json", + "text/*+json", + "te*t/plain", + ].each do |value| + expect{subject.parse(value).media_ranges}.to raise_exception(Protocol::HTTP::Header::Accept::ParseError) + end + end + + it "rejects invalid quality factors" do + [ + "text/html;q=1.1", + "text/html;q=0.1234", + "text/html;q=invalid", + 'text/html;q="0.5"', + "text/html;q=0.5;q=0.4", + ].each do |value| + expect{subject.parse(value).media_ranges}.to raise_exception(Protocol::HTTP::Header::Accept::ParseError) + end + end + end + + with ".parse" do + it "accepts an empty string" do + expect(subject.parse("").media_ranges).to be == [] + end + + it "rejects nil" do + expect{subject.parse(nil)}.to raise_exception(TypeError) + end + end + + with "text/html;q=0, text/plain;q=0.123, application/json;q=1.000" do + it "accepts standard quality factors" do + expect(media_ranges.collect(&:quality_factor)).to be == [1.0, 0.123, 0.0] + end + end + with ".coerce" do it "coerces array to Accept" do result = subject.coerce(["text/html", "application/json"]) From 47355af66343c24c76558e2460727945821a7d1b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:22:51 +1200 Subject: [PATCH 2/8] Move Accept type validation into parser --- lib/protocol/http/header/accept.rb | 4 ++-- test/protocol/http/header/accept.rb | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index 1a1b1a5..eae4199 100644 --- a/lib/protocol/http/header/accept.rb +++ b/lib/protocol/http/header/accept.rb @@ -65,8 +65,6 @@ def quality_factor # @parameter value [String] a raw header value containing comma-separated media types. # @returns [Accept] a new instance containing the parsed media types. def self.parse(value) - raise TypeError, "Expected a String, got: #{value.class}" unless value.is_a?(String) - self.new << value end @@ -104,6 +102,8 @@ def media_ranges private def split_values(value) + raise TypeError, "Expected a String, got: #{value.class}" unless value.is_a?(String) + return [] if value.empty? values = [] diff --git a/test/protocol/http/header/accept.rb b/test/protocol/http/header/accept.rb index 0288b1c..f38610d 100644 --- a/test/protocol/http/header/accept.rb +++ b/test/protocol/http/header/accept.rb @@ -143,6 +143,12 @@ end end + with "#<<" do + it "rejects nil" do + expect{subject.new << nil}.to raise_exception(TypeError) + end + end + with "text/html;q=0, text/plain;q=0.123, application/json;q=1.000" do it "accepts standard quality factors" do expect(media_ranges.collect(&:quality_factor)).to be == [1.0, 0.123, 0.0] From 25d204955a6832153fc2826f609356bad5fa75ce Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:30:11 +1200 Subject: [PATCH 3/8] Preserve regex-based Accept parsing --- lib/protocol/http/header/accept.rb | 58 +++++++++++------------------ test/protocol/http/header/accept.rb | 8 +++- 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index eae4199..24db94e 100644 --- a/lib/protocol/http/header/accept.rb +++ b/lib/protocol/http/header/accept.rb @@ -8,13 +8,21 @@ require_relative "../quoted_string" require_relative "../error" -require "strscan" - module Protocol module HTTP module Header # The `accept-content-type` header represents a list of content-types that the client can accept. class Accept < Split + # Regular expression used to split values on commas, with optional surrounding whitespace, taking into account quoted strings. + SEPARATOR = / + (?: + "(?:[^"\\]|\\.)*" # Match quoted strings, including quoted pairs. + | + [^,"]+ # Match non-quoted strings until a comma or quote. + )+ + (?=,|\z) + /x + ParseError = Class.new(Error) MEDIA_RANGE = /\A(?#{TOKEN})\/(?#{TOKEN})(?.*)\z/ @@ -104,34 +112,7 @@ def media_ranges def split_values(value) raise TypeError, "Expected a String, got: #{value.class}" unless value.is_a?(String) - return [] if value.empty? - - values = [] - start = 0 - quoted = false - escaped = false - - value.each_char.with_index do |character, index| - if quoted - if escaped - escaped = false - elsif character == "\\" - escaped = true - elsif character == '"' - quoted = false - end - elsif character == '"' - quoted = true - elsif character == "," - values << value[start...index].strip - start = index + 1 - end - end - - raise ParseError, "Unterminated quoted string: #{value.inspect}" if quoted - - values << value[start..].strip - return values + value.scan(SEPARATOR).map(&:strip) end def parse_media_range(value) @@ -139,12 +120,15 @@ def parse_media_range(value) type = match[:type] subtype = match[:subtype] parameters = {} - scanner = StringScanner.new(match[:parameters]) + parameters_string = match[:parameters] + offset = 0 - while scanner.scan(PARAMETER) - key = scanner[:key] - value = scanner[:value] - quoted_value = scanner[:quoted_value] + parameters_string.scan(PARAMETER) do |key, value, quoted_value| + parameter_match = Regexp.last_match + unless parameter_match.begin(0) == offset + raise ParseError, "Invalid media range parameters: #{parameters_string[offset..].inspect}" + end + offset = parameter_match.end(0) if key.casecmp?("q") if quoted_value || parameters.key?("q") || !value.match?(/\A(?:#{QVALUE})\z/) @@ -161,8 +145,8 @@ def parse_media_range(value) parameters[key] = value end - unless scanner.eos? - raise ParseError, "Invalid media range parameters: #{scanner.rest.inspect}" + unless offset == parameters_string.length + raise ParseError, "Invalid media range parameters: #{parameters_string[offset..].inspect}" end if (type.include?("*") && type != "*") || (subtype.include?("*") && subtype != "*") || (type == "*" && subtype != "*") diff --git a/test/protocol/http/header/accept.rb b/test/protocol/http/header/accept.rb index f38610d..7ad0af1 100644 --- a/test/protocol/http/header/accept.rb +++ b/test/protocol/http/header/accept.rb @@ -99,8 +99,6 @@ with "invalid media ranges" do it "rejects malformed parameters" do [ - ",", - "text/html,", "text/html;", "text/html;invalid", "text/html;charset=", @@ -141,6 +139,12 @@ it "rejects nil" do expect{subject.parse(nil)}.to raise_exception(TypeError) end + + it "ignores empty list members" do + expect(subject.parse(", text/html,").media_ranges).to have_attributes( + length: be == 1, + ) + end end with "#<<" do From d5386c819f8ceb73c5e9367f8b3b96d967f70a27 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:33:29 +1200 Subject: [PATCH 4/8] Remove redundant Accept input validation --- lib/protocol/http/header/accept.rb | 8 +------- test/protocol/http/header/accept.rb | 10 ---------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index 24db94e..07e62fa 100644 --- a/lib/protocol/http/header/accept.rb +++ b/lib/protocol/http/header/accept.rb @@ -82,7 +82,7 @@ def self.parse(value) # # @parameter value [String] a raw header value containing one or more media types separated by commas. def << value - self.concat(split_values(value)) + self.concat(value.scan(SEPARATOR).map(&:strip)) end # Converts the parsed header value into a raw header value. @@ -109,12 +109,6 @@ def media_ranges private - def split_values(value) - raise TypeError, "Expected a String, got: #{value.class}" unless value.is_a?(String) - - value.scan(SEPARATOR).map(&:strip) - end - def parse_media_range(value) if match = value.match(MEDIA_RANGE) type = match[:type] diff --git a/test/protocol/http/header/accept.rb b/test/protocol/http/header/accept.rb index 7ad0af1..431bd79 100644 --- a/test/protocol/http/header/accept.rb +++ b/test/protocol/http/header/accept.rb @@ -136,10 +136,6 @@ expect(subject.parse("").media_ranges).to be == [] end - it "rejects nil" do - expect{subject.parse(nil)}.to raise_exception(TypeError) - end - it "ignores empty list members" do expect(subject.parse(", text/html,").media_ranges).to have_attributes( length: be == 1, @@ -147,12 +143,6 @@ end end - with "#<<" do - it "rejects nil" do - expect{subject.new << nil}.to raise_exception(TypeError) - end - end - with "text/html;q=0, text/plain;q=0.123, application/json;q=1.000" do it "accepts standard quality factors" do expect(media_ranges.collect(&:quality_factor)).to be == [1.0, 0.123, 0.0] From 5c735f54ceba3cc507882ceec9de9735154453b2 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:35:11 +1200 Subject: [PATCH 5/8] Restore Accept separator annotations --- lib/protocol/http/header/accept.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index 07e62fa..919daa3 100644 --- a/lib/protocol/http/header/accept.rb +++ b/lib/protocol/http/header/accept.rb @@ -15,12 +15,12 @@ module Header class Accept < Split # Regular expression used to split values on commas, with optional surrounding whitespace, taking into account quoted strings. SEPARATOR = / - (?: - "(?:[^"\\]|\\.)*" # Match quoted strings, including quoted pairs. - | - [^,"]+ # Match non-quoted strings until a comma or quote. + (?: # Start non-capturing group + "(?:[^"\\]|\\.)*" # Match quoted strings, including quoted pairs + | # OR + [^,"]+ # Match non-quoted strings until a comma or quote )+ - (?=,|\z) + (?=,|\z) # Match until a comma or end of string /x ParseError = Class.new(Error) From 0f125673080aeae93df7137e6ca62f579f3005c1 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:42:27 +1200 Subject: [PATCH 6/8] Add Accept parser release note --- releases.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/releases.md b/releases.md index 79cef02..d982394 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - Improve `Accept` header parsing for quoted pairs, malformed parameters, invalid wildcards, and invalid quality factors. + ## v0.64.0 - Add `Protocol::HTTP::Request#rewind!` and `#retry!` for preparing requests to be sent again. From 848aed76a89add3a014b7fbb653622ffdf7973bf Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 16:51:20 +1200 Subject: [PATCH 7/8] Preserve direct Accept parsing --- lib/protocol/http/header/accept.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index 919daa3..bbad9ff 100644 --- a/lib/protocol/http/header/accept.rb +++ b/lib/protocol/http/header/accept.rb @@ -73,7 +73,7 @@ def quality_factor # @parameter value [String] a raw header value containing comma-separated media types. # @returns [Accept] a new instance containing the parsed media types. def self.parse(value) - self.new << value + self.new(value.scan(SEPARATOR).map(&:strip)) end # Adds one or more comma-separated values to the header. From 2f44ad1a7359e4735fc5c93036a01056559c1257 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 28 Jul 2026 17:00:13 +1200 Subject: [PATCH 8/8] Cover malformed Accept parameter gaps --- test/protocol/http/header/accept.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/protocol/http/header/accept.rb b/test/protocol/http/header/accept.rb index 431bd79..2c09b0a 100644 --- a/test/protocol/http/header/accept.rb +++ b/test/protocol/http/header/accept.rb @@ -101,6 +101,7 @@ [ "text/html;", "text/html;invalid", + "text/html;invalid;q=0.5", "text/html;charset=", 'text/html;charset="unterminated', ].each do |value|