Skip to content

Commit

Permalink
fix(http/cookies): allow spaces to be present in the cookie value
Browse files Browse the repository at this point in the history
Fixes crystal-lang#14435.

Move parse_set_cookie example to the correct location.
  • Loading branch information
anton7c3 committed May 31, 2024
1 parent 28fd0dc commit 5d152d1
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 14 deletions.
55 changes: 47 additions & 8 deletions spec/std/http/cookie_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ module HTTP
it "raises on invalid value" do
cookie = HTTP::Cookie.new("x", "")
invalid_values = {
'"', ',', ';', '\\', # invalid printable ascii characters
' ', '\r', '\t', '\n', # non-printable ascii characters
'"', ',', ';', '\\', # invalid printable ascii characters
'\r', '\t', '\n', # non-printable ascii characters
}.map { |c| "foo#{c}bar" }

invalid_values.each do |invalid_value|
Expand Down Expand Up @@ -235,12 +235,6 @@ module HTTP
cookie.to_set_cookie_header.should eq("key=value")
end

it "parse_set_cookie with space" do
cookie = parse_set_cookie("key=value; path=/test")
parse_set_cookie("key=value;path=/test").should eq cookie
parse_set_cookie("key=value; \t\npath=/test").should eq cookie
end

it "parses key=" do
cookie = parse_first_cookie("key=")
cookie.name.should eq("key")
Expand Down Expand Up @@ -285,9 +279,54 @@ module HTTP
first.value.should eq("bar")
second.value.should eq("baz")
end

it "parses cookie with spaces in value" do
parse_first_cookie(%[key=some value]).value.should eq "some value"
parse_first_cookie(%[key="some value"]).value.should eq "some value"
end

it "strips spaces around value only when it's unquoted" do
parse_first_cookie(%[key= some value ]).value.should eq "some value"
parse_first_cookie(%[key=" some value "]).value.should eq " some value "
end
end

describe "parse_set_cookie" do
it "with space" do
cookie = parse_set_cookie("key=value; path=/test")
parse_set_cookie("key=value;path=/test").should eq cookie
parse_set_cookie("key=value; \t\npath=/test").should eq cookie
end

it "parses cookie with spaces in value" do
parse_set_cookie(%[key=some value]).value.should eq "some value"
parse_set_cookie(%[key="some value"]).value.should eq "some value"
end

it "removes leading and trailing whitespaces" do
cookie = parse_set_cookie(%[key= \tvalue \t; \t\npath=/test])
cookie.name.should eq "key"
cookie.value.should eq "value"
cookie.path.should eq "/test"

cookie = parse_set_cookie(%[ key\t =value \n;path=/test])
cookie.name.should eq "key"
cookie.value.should eq "value"
cookie.path.should eq "/test"
end

it "strips spaces around value only when it's unquoted" do
cookie = parse_set_cookie(%[key= value ; \tpath=/test])
cookie.name.should eq "key"
cookie.value.should eq "value"
cookie.path.should eq "/test"

cookie = parse_set_cookie(%[key=" value "; \tpath=/test])
cookie.name.should eq "key"
cookie.value.should eq " value "
cookie.path.should eq "/test"
end

it "parses path" do
cookie = parse_set_cookie("key=value; path=/test")
cookie.name.should eq("key")
Expand Down
22 changes: 16 additions & 6 deletions src/http/cookie.cr
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ module HTTP
private def validate_value(value)
value.each_byte do |byte|
# valid characters for cookie-value per https://tools.ietf.org/html/rfc6265#section-4.1.1
# all printable ASCII characters except ' ', ',', '"', ';' and '\\'
if !byte.in?(0x21...0x7f) || byte.in?(0x22, 0x2c, 0x3b, 0x5c)
# all printable ASCII characters except ',', '"', ';' and '\\'
if !byte.in?(0x20...0x7f) || byte.in?(0x22, 0x2c, 0x3b, 0x5c)
raise IO::Error.new("Invalid cookie value")
end
end
Expand Down Expand Up @@ -196,9 +196,9 @@ module HTTP
module Parser
module Regex
CookieName = /[^()<>@,;:\\"\/\[\]?={} \t\x00-\x1f\x7f]+/
CookieOctet = /[!#-+\--:<-\[\]-~]/
CookieOctet = /[!#-+\--:<-\[\]-~ ]/
CookieValue = /(?:"#{CookieOctet}*"|#{CookieOctet}*)/
CookiePair = /(?<name>#{CookieName})=(?<value>#{CookieValue})/
CookiePair = /\s*(?<name>#{CookieName})\s*=\s*(?<value>#{CookieValue})\s*/
DomainLabel = /[A-Za-z0-9\-]+/
DomainIp = /(?:\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})/
Time = /(?:\d{2}:\d{2}:\d{2})/
Expand Down Expand Up @@ -230,9 +230,11 @@ module HTTP
def parse_cookies(header, &)
header.scan(CookieString).each do |pair|
value = pair["value"]
if value.starts_with?('"')
if value.starts_with?('"') && value.ends_with?('"')
# Unwrap quoted cookie value
value = value.byte_slice(1, value.bytesize - 2)
else
value = value.strip
end
yield Cookie.new(pair["name"], value)
end
Expand All @@ -251,8 +253,16 @@ module HTTP
expires = parse_time(match["expires"]?)
max_age = match["max_age"]?.try(&.to_i64.seconds)

# Unwrap quoted cookie value
cookie_value = match["value"]
if cookie_value.starts_with?('"') && cookie_value.ends_with?('"')
cookie_value = cookie_value.byte_slice(1, cookie_value.bytesize - 2)
else
cookie_value = cookie_value.strip
end

Cookie.new(
match["name"], match["value"],
match["name"], cookie_value,
path: match["path"]?,
expires: expires,
domain: match["domain"]?,
Expand Down

0 comments on commit 5d152d1

Please sign in to comment.