diff --git a/lib/protocol/http/header/accept.rb b/lib/protocol/http/header/accept.rb index ec5cdf4..bbad9ff 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 = / - (?: # Start non-capturing group - "[^"\\]*" # Match quoted strings (no escaping of quotes within) - | # OR - [^,"]+ # 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) # Match until a comma or end of string + (?=,|\z) # Match until a comma or end of string /x ParseError = Class.new(Error) @@ -114,8 +114,24 @@ def parse_media_range(value) type = match[:type] subtype = match[:subtype] parameters = {} + parameters_string = match[:parameters] + offset = 0 - match[:parameters].scan(PARAMETER) do |key, value, 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/) + raise ParseError, "Invalid quality factor: #{value.inspect}" + end + + key = "q" + end + if quoted_value value = QuotedString.unquote(quoted_value) end @@ -123,6 +139,14 @@ def parse_media_range(value) parameters[key] = value end + 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 != "*") + 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/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. diff --git a/test/protocol/http/header/accept.rb b/test/protocol/http/header/accept.rb index 20630a0..2c09b0a 100644 --- a/test/protocol/http/header/accept.rb +++ b/test/protocol/http/header/accept.rb @@ -84,6 +84,72 @@ 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;invalid", + "text/html;invalid;q=0.5", + "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 "ignores empty list members" do + expect(subject.parse(", text/html,").media_ranges).to have_attributes( + length: be == 1, + ) + 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"])