fixed a few more RFC 6265 compliance bugs

mojolicious · Sep 9, 2013 · 5fa8130 · 5fa8130
1 parent 798edfb
commit 5fa8130
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 20 deletions.
diff --git a/Changes b/Changes
@@ -1,7 +1,8 @@
 
 4.35  2013-09-09
   - Added origin attribute to Mojo::Cookie::Response.
-  - Fixed RFC 6265 compliance bug in Mojo::UserAgent::CookieJar.
+  - Fixed RFC 6265 compliance bugs in Mojo::Cookie::Request,
+    Mojo::Cookie::Response and Mojo::UserAgent::CookieJar.
 
 4.34  2013-09-08
   - Fixed portability bug in SO_REUSEPORT tests.

diff --git a/lib/Mojo/Collection.pm b/lib/Mojo/Collection.pm
@@ -32,7 +32,7 @@ sub new {
 sub c { __PACKAGE__->new(@_) }
 
 sub compact {
-  shift->grep(sub {length});
+  shift->grep(sub { length($_ // '') });
 }
 
 sub each {

diff --git a/lib/Mojo/Cookie/Request.pm b/lib/Mojo/Cookie/Request.pm
@@ -19,7 +19,7 @@ sub parse {
 
 sub to_string {
   my $self = shift;
-  return '' unless my $name = $self->name;
+  return '' unless length(my $name = $self->name // '');
   my $value = $self->value // '';
   $value = $value =~ /[,;" ]/ ? quote($value) : $value;
   return "$name=$value";

diff --git a/lib/Mojo/Cookie/Response.pm b/lib/Mojo/Cookie/Response.pm
@@ -55,7 +55,7 @@ sub to_string {
   my $self = shift;
 
   # Name and value (Netscape)
-  return '' unless my $name = $self->name;
+  return '' unless length(my $name = $self->name // '');
   my $value = $self->value // '';
   $value = $value =~ /[,;" ]/ ? quote($value) : $value;
   my $cookie = "$name=$value";

diff --git a/lib/Mojo/UserAgent/CookieJar.pm b/lib/Mojo/UserAgent/CookieJar.pm
@@ -19,12 +19,13 @@ sub add {
     next if length($cookie->value // '') > $size;
 
     # Replace cookie
-    my $domain = lc($cookie->domain // $cookie->origin);
+    my $origin = $cookie->origin;
+    next unless my $domain = $cookie->domain // $origin;
     $domain =~ s/^\.//;
-    my $path = $cookie->path;
-    my $name = $cookie->name;
-    my $jar  = $self->{jar}{$domain} ||= [];
-    @$jar = (grep({$_->path ne $path || $_->name ne $name} @$jar), $cookie);
+    next unless my $path = $cookie->path;
+    next unless length(my $name = $cookie->name // '');
+    my $jar = $self->{jar}{lc $domain} ||= [];
+    @$jar = (grep({_compare($_, $path, $name, $origin)} @$jar), $cookie);
   }
 
   return $self;
@@ -69,7 +70,7 @@ sub find {
     # Grab cookies
     my $new = $self->{jar}{$domain} = [];
     for my $cookie (@$old) {
-      next unless $cookie->domain || $host eq $cookie->origin;
+      if (my $origin = $cookie->origin) { next unless $host eq $origin }
 
       # Check if cookie has expired
       my $expires = $cookie->expires;
@@ -98,6 +99,12 @@ sub inject {
   $req->cookies($self->find($req->url));
 }
 
+sub _compare {
+  my ($cookie, $path, $name, $origin) = @_;
+  return 1 if $cookie->path ne $path || $cookie->name ne $name;
+  return ($cookie->origin // '') ne ($origin // '');
+}
+
 sub _path { $_[0] eq '/' || $_[0] eq $_[1] || $_[1] =~ m!^\Q$_[0]/! }
 
 1;

diff --git a/t/mojo/cookie.t b/t/mojo/cookie.t
@@ -4,11 +4,15 @@ use Test::More;
 use Mojo::Cookie::Request;
 use Mojo::Cookie::Response;
 
+# Missing name
+is(Mojo::Cookie::Request->new,  '', 'right format');
+is(Mojo::Cookie::Response->new, '', 'right format');
+
 # Request cookie as string
 my $cookie = Mojo::Cookie::Request->new;
-$cookie->name('foo');
+$cookie->name('0');
 $cookie->value('ba =r');
-is $cookie->to_string, 'foo="ba =r"', 'right format';
+is $cookie->to_string, '0="ba =r"', 'right format';
 
 # Request cookie without value as string
 $cookie = Mojo::Cookie::Request->new;
@@ -152,7 +156,7 @@ is $cookie->to_string, 'foo=; path=/test', 'right format';
 
 # Full response cookie as string
 $cookie = Mojo::Cookie::Response->new;
-$cookie->name('foo');
+$cookie->name('0');
 $cookie->value('ba r');
 $cookie->domain('example.com');
 $cookie->path('/test');
@@ -161,7 +165,7 @@ $cookie->expires(1218092879);
 $cookie->secure(1);
 $cookie->httponly(1);
 is $cookie->to_string,
-  'foo="ba r"; expires=Thu, 07 Aug 2008 07:07:59 GMT; domain=example.com;'
+  '0="ba r"; expires=Thu, 07 Aug 2008 07:07:59 GMT; domain=example.com;'
   . ' path=/test; secure; Max-Age=60; HttpOnly', 'right format';
 
 # Empty response cookie

diff --git a/t/mojo/cookiejar.t b/t/mojo/cookiejar.t
@@ -6,8 +6,15 @@ use Mojo::Transaction::HTTP;
 use Mojo::URL;
 use Mojo::UserAgent::CookieJar;
 
-# Session cookie
+# Missing values
 my $jar = Mojo::UserAgent::CookieJar->new;
+$jar->add(Mojo::Cookie::Response->new(domain => 'example.com'));
+$jar->add(Mojo::Cookie::Response->new(name   => 'foo'));
+$jar->add(Mojo::Cookie::Response->new(name => 'foo', domain => 'example.com'));
+$jar->add(Mojo::Cookie::Response->new(domain => 'example.com', path => '/'));
+is_deeply [$jar->all], [], 'no cookies';
+
+# Session cookie
 $jar->add(
   Mojo::Cookie::Response->new(
     domain => 'example.com',
@@ -321,6 +328,36 @@ $tx->req->url->parse('http://mojolicio.us/whatever');
 $jar->inject($tx);
 is $tx->req->cookie('foo'), undef, 'no cookie';
 
+# Extract and inject cookies with same name (with and without domain)
+$jar = Mojo::UserAgent::CookieJar->new;
+$tx  = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://example.com/test');
+$tx->res->cookies(
+  Mojo::Cookie::Response->new(name => 'foo', value => 'without'),
+  Mojo::Cookie::Response->new(
+    name   => 'foo',
+    value  => 'with',
+    domain => 'example.com'
+  )
+);
+$jar->extract($tx);
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://example.com/test');
+$jar->inject($tx);
+@cookies = $tx->req->cookie('foo');
+is $cookies[0]->name,  'foo',     'right name';
+is $cookies[0]->value, 'without', 'right value';
+is $cookies[1]->name,  'foo',     'right name';
+is $cookies[1]->value, 'with',    'right value';
+is $cookies[2], undef, 'no third cookie';
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://www.example.com/test');
+$jar->inject($tx);
+@cookies = $tx->req->cookie('foo');
+is $cookies[0]->name,  'foo',  'right name';
+is $cookies[0]->value, 'with', 'right value';
+is $cookies[1], undef, 'no second cookie';
+
 # Extract and inject cookies for "localhost" (valid and invalid)
 $jar = Mojo::UserAgent::CookieJar->new;
 $tx  = Mojo::Transaction::HTTP->new;
@@ -363,7 +400,7 @@ $tx->res->cookies(
     path   => '/'
   ),
   Mojo::Cookie::Response->new(
-    name   => 'baz',
+    name   => '0',
     value  => 'with',
     domain => '.xn--bcher-kva.cOm',
     path   => '/%70erldoc/Mojolicious/'
@@ -377,16 +414,16 @@ is $tx->req->cookie('foo')->name,  'foo',  'right name';
 is $tx->req->cookie('foo')->value, 'with', 'right value';
 is $tx->req->cookie('bar')->name,  'bar',  'right name';
 is $tx->req->cookie('bar')->value, 'with', 'right value';
-is $tx->req->cookie('baz')->name,  'baz',  'right name';
-is $tx->req->cookie('baz')->value, 'with', 'right value';
+is $tx->req->cookie('0')->name,    '0',    'right name';
+is $tx->req->cookie('0')->value,   'with', 'right value';
 $tx = Mojo::Transaction::HTTP->new;
 $tx->req->url->parse('http://bücher.COM/perldoc/Mojolicious/Lite');
 $jar->inject($tx);
 is $tx->req->cookie('foo'), undef, 'no cookie';
 is $tx->req->cookie('bar')->name,  'bar',  'right name';
 is $tx->req->cookie('bar')->value, 'with', 'right value';
-is $tx->req->cookie('baz')->name,  'baz',  'right name';
-is $tx->req->cookie('baz')->value, 'with', 'right value';
+is $tx->req->cookie('0')->name,    '0',    'right name';
+is $tx->req->cookie('0')->value,   'with', 'right value';
 $tx = Mojo::Transaction::HTTP->new;
 $tx->req->url->parse('http://labs.bücher.COM/Perldoc');
 $jar->inject($tx);