Skip Menu |

This queue is for tickets about the CGI-Simple CPAN distribution.

Report information
The Basics
Id: 34310
Status: resolved
Priority: 0/
Queue: CGI-Simple

People
Owner: Nobody in particular
Requestors: RSAVAGE [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.103
Fixed in: (no value)



Subject: CGI::Simple::Cookie
Line 34 of CGI::Simple::Cookie says: $pair =~ s/^\s+|\s+$//; # trim leading trailing whitespace but this is missing a /g. It trims either leading or trailing spaces, but not both. Compare with line 93 of CGI::Cookie which does trim both: s/\s*(.*?)\s*/$1/;
Subject: CGI::Simple::Cookie: should support max_age
Perhaps this should be a separate bug report, but here's another issue to fix in CGI::Session::Cookie. The Cookie RFC ( http://www.w3.org/Protocols/rfc2109/rfc2109 ) recommends using "Max-Age", not "Expires" to set the expiration date. ("Expires" remains as a legacy option and uses a different date format. ) "max-age" should be added as a constructor option and a "max_age()" getter/setter method should also be added. This will also increase compatibility with CGI.pm, which added Max-Age support somewhere in the 2.8x series. ( Related to this: a new version of a CGI::Session is about to come out which is expected to switch from using "expires" to "Max-age" to be more RFC-compliant, fixing an issue with Safari. Until CGI::Simple is updated, it will not be a compatible replacement. ) Thanks! Mark
Subject: CGI::Simple::Cookie (4 patches)
By comparing the current CGI::Simple::Cookie to CGI.pm, I found several bugs that had been fixed in CGI.pm since the fork in 2001, and not in CGI::Simple. In each of the 4 cases, I also updated an updated test to illustrate the problem and fix. You can review and apply the pages in the order from to 1 to 4, or the "combined_cookie.patch" represents all the work in one file. ( None of these address "max_age" support, where CGI::Simple still lags CGI.pm. ) Mark
Sat Dec 6 17:19:14 EST 2008 Mark Stosberg <mark@summersault.com> * Support cookies which have an equals sign in the value. Ported from CGI.pm diff -rN -u old-CGI-Simple-1.106/Changes new-CGI-Simple-1.106/Changes --- old-CGI-Simple-1.106/Changes 2008-12-06 18:04:31.000000000 -0500 +++ new-CGI-Simple-1.106/Changes 2008-12-06 18:04:31.000000000 -0500 @@ -144,6 +144,6 @@ - Accept a comma as well as semi-colon as a cookie separator. This is consistent with CGI.pm as well as RFC 2965, which states: "A server SHOULD also accept comma (,) as the separator between cookie-values for future compatibility." (Mark Stosberg) - + - Support cookies which have an equals sign in the value. Ported from CGI.pm (Mark Stosberg) diff -rN -u old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm --- old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:04:31.000000000 -0500 +++ new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:04:31.000000000 -0500 @@ -32,7 +32,7 @@ my @pairs = split "[;,] ?", $raw_cookie; for my $pair ( @pairs ) { $pair =~ s/^\s+|\s+$//g; # trim leading trailing whitespace - my ( $key, $value ) = split "=", $pair; + my($key,$value) = split("=",$pair,2); next unless defined $value; my @values = map { unescape( $_ ) } split /[&;]/, $value; $key = unescape( $key ); diff -rN -u old-CGI-Simple-1.106/t/020.cookie.t new-CGI-Simple-1.106/t/020.cookie.t --- old-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:04:31.000000000 -0500 +++ new-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:04:31.000000000 -0500 @@ -17,7 +17,7 @@ } my @test_cookie = ( - 'foo=123, bar=qwerty; baz=wibble ; qux=a1', + 'foo=123, bar=qwerty; baz=wib=ble ; qux=a1', 'foo=123; bar=qwerty; baz=wibble;', 'foo=vixen; bar=cow; baz=bitch; qux=politician', 'foo=a%20phrase; bar=yes%2C%20a%20phrase; baz=%5Ewibble; qux=%27', @@ -44,7 +44,7 @@ is( $result{foo}->value, '123', "cookie foo is correct" ); is( $result{bar}->value, 'qwerty', "cookie bar is correct" ); - is( $result{baz}->value, 'wibble', "cookie baz is correct" ); + is( $result{baz}->value, 'wib=ble', "cookie baz is correct" ); is( $result{qux}->value, 'a1', "cookie qux is correct" ); }
Sat Dec 6 17:28:09 EST 2008 Mark Stosberg <mark@summersault.com> * Support cookies in which one of multiple values is empty. Ported from CGI.pm diff -rN -u old-CGI-Simple-1.106/Changes new-CGI-Simple-1.106/Changes --- old-CGI-Simple-1.106/Changes 2008-12-06 18:05:09.000000000 -0500 +++ new-CGI-Simple-1.106/Changes 2008-12-06 18:05:09.000000000 -0500 @@ -145,5 +145,6 @@ as well as RFC 2965, which states: "A server SHOULD also accept comma (,) as the separator between cookie-values for future compatibility." (Mark Stosberg) - Support cookies which have an equals sign in the value. Ported from CGI.pm (Mark Stosberg) + - Support cookies in which one of multiple values is empty. Ported from CGI.pm (Mark Stosberg) diff -rN -u old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm --- old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:05:09.000000000 -0500 +++ new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:05:09.000000000 -0500 @@ -33,9 +33,13 @@ for my $pair ( @pairs ) { $pair =~ s/^\s+|\s+$//g; # trim leading trailing whitespace my($key,$value) = split("=",$pair,2); - next unless defined $value; - my @values = map { unescape( $_ ) } split /[&;]/, $value; - $key = unescape( $key ); + next if !defined($value); + my @values = (); + if ($value ne '') { + @values = map unescape($_),split(/[&;]/,$value.'&dmy'); + pop @values; + } + $key = unescape($key); # A bug in Netscape can cause several cookies with same name to # appear. The FIRST one in HTTP_COOKIE is the most recent version. diff -rN -u old-CGI-Simple-1.106/t/020.cookie.t new-CGI-Simple-1.106/t/020.cookie.t --- old-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:05:09.000000000 -0500 +++ new-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:05:09.000000000 -0500 @@ -17,7 +17,7 @@ } my @test_cookie = ( - 'foo=123, bar=qwerty; baz=wib=ble ; qux=a1', + 'foo=123, bar=qwerty; baz=wib=ble ; qux=1&2&', 'foo=123; bar=qwerty; baz=wibble;', 'foo=vixen; bar=cow; baz=bitch; qux=politician', 'foo=a%20phrase; bar=yes%2C%20a%20phrase; baz=%5Ewibble; qux=%27', @@ -45,7 +45,8 @@ is( $result{foo}->value, '123', "cookie foo is correct" ); is( $result{bar}->value, 'qwerty', "cookie bar is correct" ); is( $result{baz}->value, 'wib=ble', "cookie baz is correct" ); - is( $result{qux}->value, 'a1', "cookie qux is correct" ); + my @values = $result{qux}->value; + is_deeply ( \@values, [1,2,''], "multiple values are supported including empty values." ); } #-----------------------------------------------------------------------------
Sat Dec 6 16:38:08 EST 2008 Mark Stosberg <mark@summersault.com> * - CGI::Simple::Cookie, fixed bug when cookie had both leading and trailing white space diff -rN -u old-CGI-Simple-1.106/Changes new-CGI-Simple-1.106/Changes --- old-CGI-Simple-1.106/Changes 2008-12-06 18:02:52.000000000 -0500 +++ new-CGI-Simple-1.106/Changes 2008-12-06 18:02:52.000000000 -0500 @@ -139,4 +139,6 @@ 1.107 2008-12-06 - Fixed bug when calling unescapeHTML on HTML that wasn't properly escaped in the first place. Thanks to M-Uchino and Mark Stosberg. + - CGI::Simple::Cookie, fixed bug when cookie had both leading and trailing white space + (RT#34314, Ron Savage and Mark Stosberg) diff -rN -u old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm --- old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:02:52.000000000 -0500 +++ new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:02:52.000000000 -0500 @@ -31,7 +31,7 @@ my %results; my @pairs = split "; ?", $raw_cookie; for my $pair ( @pairs ) { - $pair =~ s/^\s+|\s+$//; # trim leading trailing whitespace + $pair =~ s/^\s+|\s+$//g; # trim leading trailing whitespace my ( $key, $value ) = split "=", $pair; next unless defined $value; my @values = map { unescape( $_ ) } split /[&;]/, $value; diff -rN -u old-CGI-Simple-1.106/t/020.cookie.t new-CGI-Simple-1.106/t/020.cookie.t --- old-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:02:52.000000000 -0500 +++ new-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:02:52.000000000 -0500 @@ -17,7 +17,7 @@ } my @test_cookie = ( - 'foo=123; bar=qwerty; baz=wibble; qux=a1', + 'foo=123; bar=qwerty; baz=wibble ; qux=a1', 'foo=123; bar=qwerty; baz=wibble;', 'foo=vixen; bar=cow; baz=bitch; qux=politician', 'foo=a%20phrase; bar=yes%2C%20a%20phrase; baz=%5Ewibble; qux=%27',
Sat Dec 6 17:28:09 EST 2008 Mark Stosberg <mark@summersault.com> * Support cookies in which one of multiple values is empty. Ported from CGI.pm Sat Dec 6 17:19:14 EST 2008 Mark Stosberg <mark@summersault.com> * Support cookies which have an equals sign in the value. Ported from CGI.pm Sat Dec 6 16:52:03 EST 2008 Mark Stosberg <mark@summersault.com> * - Accept a comma as well as semi-colon as a cookie separator. This is consistent with CGI.pm as well as RFC 2965, which states: "A server SHOULD also accept comma (,) as the separator between cookie-values for future compatibility." Sat Dec 6 16:38:08 EST 2008 Mark Stosberg <mark@summersault.com> * - CGI::Simple::Cookie, fixed bug when cookie had both leading and trailing white space diff -rN -u old-CGI-Simple-1.106/Changes new-CGI-Simple-1.106/Changes --- old-CGI-Simple-1.106/Changes 2008-12-06 18:05:52.000000000 -0500 +++ new-CGI-Simple-1.106/Changes 2008-12-06 18:05:52.000000000 -0500 @@ -139,4 +139,12 @@ 1.107 2008-12-06 - Fixed bug when calling unescapeHTML on HTML that wasn't properly escaped in the first place. Thanks to M-Uchino and Mark Stosberg. + - CGI::Simple::Cookie, fixed bug when cookie had both leading and trailing white space + (RT#34314, Ron Savage and Mark Stosberg) + - Accept a comma as well as semi-colon as a cookie separator. This is consistent with CGI.pm + as well as RFC 2965, which states: "A server SHOULD also accept comma (,) + as the separator between cookie-values for future compatibility." (Mark Stosberg) + - Support cookies which have an equals sign in the value. Ported from CGI.pm (Mark Stosberg) + - Support cookies in which one of multiple values is empty. Ported from CGI.pm (Mark Stosberg) + diff -rN -u old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm --- old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:05:52.000000000 -0500 +++ new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:05:52.000000000 -0500 @@ -29,13 +29,17 @@ my ( $self, $raw_cookie ) = @_; return () unless $raw_cookie; my %results; - my @pairs = split "; ?", $raw_cookie; + my @pairs = split "[;,] ?", $raw_cookie; for my $pair ( @pairs ) { - $pair =~ s/^\s+|\s+$//; # trim leading trailing whitespace - my ( $key, $value ) = split "=", $pair; - next unless defined $value; - my @values = map { unescape( $_ ) } split /[&;]/, $value; - $key = unescape( $key ); + $pair =~ s/^\s+|\s+$//g; # trim leading trailing whitespace + my($key,$value) = split("=",$pair,2); + next if !defined($value); + my @values = (); + if ($value ne '') { + @values = map unescape($_),split(/[&;]/,$value.'&dmy'); + pop @values; + } + $key = unescape($key); # A bug in Netscape can cause several cookies with same name to # appear. The FIRST one in HTTP_COOKIE is the most recent version. diff -rN -u old-CGI-Simple-1.106/t/020.cookie.t new-CGI-Simple-1.106/t/020.cookie.t --- old-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:05:52.000000000 -0500 +++ new-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:05:52.000000000 -0500 @@ -17,7 +17,7 @@ } my @test_cookie = ( - 'foo=123; bar=qwerty; baz=wibble; qux=a1', + 'foo=123, bar=qwerty; baz=wib=ble ; qux=1&2&', 'foo=123; bar=qwerty; baz=wibble;', 'foo=vixen; bar=cow; baz=bitch; qux=politician', 'foo=a%20phrase; bar=yes%2C%20a%20phrase; baz=%5Ewibble; qux=%27', @@ -44,8 +44,9 @@ is( $result{foo}->value, '123', "cookie foo is correct" ); is( $result{bar}->value, 'qwerty', "cookie bar is correct" ); - is( $result{baz}->value, 'wibble', "cookie baz is correct" ); - is( $result{qux}->value, 'a1', "cookie qux is correct" ); + is( $result{baz}->value, 'wib=ble', "cookie baz is correct" ); + my @values = $result{qux}->value; + is_deeply ( \@values, [1,2,''], "multiple values are supported including empty values." ); } #-----------------------------------------------------------------------------
Sat Dec 6 16:52:03 EST 2008 Mark Stosberg <mark@summersault.com> * - Accept a comma as well as semi-colon as a cookie separator. This is consistent with CGI.pm as well as RFC 2965, which states: "A server SHOULD also accept comma (,) as the separator between cookie-values for future compatibility." diff -rN -u old-CGI-Simple-1.106/Changes new-CGI-Simple-1.106/Changes --- old-CGI-Simple-1.106/Changes 2008-12-06 18:04:04.000000000 -0500 +++ new-CGI-Simple-1.106/Changes 2008-12-06 18:04:04.000000000 -0500 @@ -141,4 +141,9 @@ Thanks to M-Uchino and Mark Stosberg. - CGI::Simple::Cookie, fixed bug when cookie had both leading and trailing white space (RT#34314, Ron Savage and Mark Stosberg) + - Accept a comma as well as semi-colon as a cookie separator. This is consistent with CGI.pm + as well as RFC 2965, which states: "A server SHOULD also accept comma (,) + as the separator between cookie-values for future compatibility." (Mark Stosberg) + + diff -rN -u old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm --- old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:04:04.000000000 -0500 +++ new-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm 2008-12-06 18:04:04.000000000 -0500 @@ -29,7 +29,7 @@ my ( $self, $raw_cookie ) = @_; return () unless $raw_cookie; my %results; - my @pairs = split "; ?", $raw_cookie; + my @pairs = split "[;,] ?", $raw_cookie; for my $pair ( @pairs ) { $pair =~ s/^\s+|\s+$//g; # trim leading trailing whitespace my ( $key, $value ) = split "=", $pair; diff -rN -u old-CGI-Simple-1.106/t/020.cookie.t new-CGI-Simple-1.106/t/020.cookie.t --- old-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:04:04.000000000 -0500 +++ new-CGI-Simple-1.106/t/020.cookie.t 2008-12-06 18:04:04.000000000 -0500 @@ -17,7 +17,7 @@ } my @test_cookie = ( - 'foo=123; bar=qwerty; baz=wibble ; qux=a1', + 'foo=123, bar=qwerty; baz=wibble ; qux=a1', 'foo=123; bar=qwerty; baz=wibble;', 'foo=vixen; bar=cow; baz=bitch; qux=politician', 'foo=a%20phrase; bar=yes%2C%20a%20phrase; baz=%5Ewibble; qux=%27',
Subject: Re: [rt.cpan.org #34310] CGI::Simple::Cookie (4 patches)
Date: Sun, 07 Dec 2008 11:29:13 +1100
To: bug-Cgi-Simple [...] rt.cpan.org
From: Ron Savage <ron [...] savage.net.au>
Hi Mark That's a nice bit of work, although scary at the same time. Here's the output of my attempt to patch: ron@zoe:/tmp$ patch -p0 < combined_cookie.patch patching file old-CGI-Simple-1.106/Changes Hunk #1 FAILED at 139. 1 out of 1 hunk FAILED -- saving rejects to file old-CGI-Simple-1.106/Changes.rej patching file old-CGI-Simple-1.106/lib/CGI/Simple/Cookie.pm patching file old-CGI-Simple-1.106/t/020.cookie.t I can't see why this failed: Contents of Changes.rej (pity about the wrap): *************** *** 139,142 **** 1.107 2008-12-06 - Fixed bug when calling unescapeHTML on HTML that wasn't properly escaped in the first place. Thanks to M-Uchino and Mark Stosberg. --- 139,150 ---- 1.107 2008-12-06 - Fixed bug when calling unescapeHTML on HTML that wasn't properly escaped in the first place. Thanks to M-Uchino and Mark Stosberg. + - CGI::Simple::Cookie, fixed bug when cookie had both leading and trailing white space + (RT#34314, Ron Savage and Mark Stosberg) + - Accept a comma as well as semi-colon as a cookie separator. This is consistent with CGI.pm + as well as RFC 2965, which states: "A server SHOULD also accept comma (,) + as the separator between cookie-values for future compatibility." (Mark Stosberg) + - Support cookies which have an equals sign in the value. Ported from CGI.pm (Mark Stosberg) + - Support cookies in which one of multiple values is empty. Ported from CGI.pm (Mark Stosberg) + -- Ron Savage ron@savage.net.au http://savage.net.au/index.html
Subject: Re: [rt.cpan.org #34310] CGI::Simple::Cookie (4 patches)
Date: Mon, 22 Dec 2008 10:48:12 +1100
To: bug-Cgi-Simple [...] rt.cpan.org
From: Ron Savage <ron [...] savage.net.au>
Hi Mark Have you noticed that when you go to CPAN, 0 bugs are noted for CGI::Simple? http://search.cpan.org/~andya/CGI-Simple-1.106/ I've reported this to RT's admin. -- Ron Savage ron@savage.net.au http://savage.net.au/index.html
On Thu Mar 20 23:24:48 2008, RSAVAGE wrote: Show quoted text
> Line 34 of CGI::Simple::Cookie says: > $pair =~ s/^\s+|\s+$//; # trim leading trailing whitespace > but this is missing a /g. It trims either leading or trailing spaces, > but not both. > Compare with line 93 of CGI::Cookie which does trim both: > s/\s*(.*?)\s*/$1/;
Fixed in #4446, thanks.
Patches applied as r4447. Thanks very much Mark!