Skip to content

Commit 5f23957

Browse files
colinnewelloalders
authored andcommitted
Fix issue #32 by checking for " as well as ; when splitting.
Fixes Set-Cookie (v1) style cookie parsing with regards to quoting. Note that this also subtly changes the way we deal values we return as it strips the " when it previously didn't. These are the logical values so I believe this is corect but it is a change of behaviour. This may well affect issue #21 as we'll strip the " on read meaning we should hopefully do the right thing now.
1 parent 85dcdef commit 5f23957

File tree

3 files changed

+70
-2
lines changed

3 files changed

+70
-2
lines changed

lib/HTTP/Cookies.pm

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ sub extract_cookies
227227
my $param;
228228
my $expires;
229229
my $first_param = 1;
230-
for $param (split(/;\s*/, $set)) {
230+
for $param (@{_split_text($set)}) {
231231
next unless length($param);
232232
my($k,$v) = split(/\s*=\s*/, $param, 2);
233233
if (defined $v) {
@@ -617,6 +617,50 @@ sub _normalize_path # so that plain string compare can be used
617617
$_[0] =~ s/([\0-\x20\x7f-\xff])/sprintf("%%%02X",ord($1))/eg;
618618
}
619619

620+
# deals with splitting values by ; and the fact that they could
621+
# be in quotes which can also have escaping.
622+
sub _split_text {
623+
my $val = shift;
624+
my @vals = grep { $_ ne '' } split(/([;\\"])/, $val);
625+
my @chunks;
626+
# divide it up into chunks to be processed.
627+
my $in_string = 0;
628+
my @current_string;
629+
for(my $i = 0; $i < @vals; $i++) {
630+
my $chunk = $vals[$i];
631+
if($in_string) {
632+
if($chunk eq '\\') {
633+
# don't care about next char probably.
634+
# having said that, probably need to be appending to the chunks
635+
# just dropping this.
636+
$i++;
637+
if($i < @vals) {
638+
push @current_string, $vals[$i];
639+
}
640+
} elsif($chunk eq '"') {
641+
$in_string = 0;
642+
}
643+
else {
644+
push @current_string, $chunk;
645+
}
646+
} else {
647+
if($chunk eq '"') {
648+
$in_string = 1;
649+
}
650+
elsif($chunk eq ';') {
651+
push @chunks, join('', @current_string);
652+
@current_string = ();
653+
}
654+
else {
655+
push @current_string, $chunk;
656+
}
657+
}
658+
}
659+
push @chunks, join('', @current_string) if @current_string;
660+
s/^\s+// for @chunks;
661+
return \@chunks;
662+
}
663+
620664
1;
621665

622666
__END__

t/cookies.t

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ $c->extract_cookies($res);
642642
$req = HTTP::Request->new(GET => "http://www.example.com/foo");
643643
$c->add_cookie_header($req);
644644
#print $req->as_string;
645-
ok($req->header("Cookie"), "foo=\"bar\"");
645+
ok($req->header("Cookie"), "foo=bar");
646646

647647
# Test cookies that expire far into the future [RT#50147]
648648
$c = HTTP::Cookies->new;

t/issue32.t

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
use strict;
2+
use warnings;
3+
use Test::More;
4+
5+
use HTTP::Cookies;
6+
use HTTP::Request;
7+
use HTTP::Response;
8+
9+
my $req = HTTP::Request->new(GET => "http://example.com");
10+
my $resp = HTTP::Response->new(200, 'OK', ['Set-Cookie', q!a="b;c;\\"d"; expires=Fri, 06-Nov-2025 08:58:34 GMT; domain=example.com; path=/!]);
11+
$resp->request($req);
12+
13+
my $c = HTTP::Cookies->new;
14+
$c->extract_cookies($resp);
15+
is $c->as_string, 'Set-Cookie3: a="b;c;\"d"; path="/"; domain=example.com; path_spec; expires="2025-11-06 08:58:34Z"; version=0' . "\n";
16+
17+
# test the implementation of the split function in isolation.
18+
# should probably name the function better too.
19+
my $simple = 'b;c;d';
20+
is_deeply HTTP::Cookies::_split_text($simple), [qw/b c d/], "Parse $simple";
21+
my $complex = '"b;c;\\"d";blah=32;foo="/"';
22+
is_deeply HTTP::Cookies::_split_text($complex), ['b;c;"d','blah=32','foo=/'], "Parse $complex";
23+
24+
done_testing;

0 commit comments

Comments
 (0)