fixed domain and path detection bugs in Mojo::UserAgent::CookieJar

mojolicious · Jan 12, 2013 · 28e228e · 28e228e
1 parent 5a328eb
commit 28e228e
Show file tree

Hide file tree

Showing 6 changed files with 151 additions and 7 deletions.
diff --git a/Changes b/Changes
@@ -1,4 +1,9 @@
 
+3.78  2013-01-13
+  - Added to_dir method to Mojo::Path.
+  - Improved tests.
+  - Fixed domain and path detection bugs in Mojo::UserAgent::CookieJar.
+
 3.77  2013-01-12
   - Added support for advanced binary WebSocket message tests to Test::Mojo.
   - Added binary and text events to Mojo::Transaction::WebSocket.

diff --git a/lib/Mojo/Path.pm b/lib/Mojo/Path.pm
@@ -85,6 +85,12 @@ sub parse {
 
 sub to_abs_string { $_[0]->leading_slash ? "$_[0]" : "/$_[0]" }
 
+sub to_dir {
+  my $clone = shift->clone;
+  pop @{$clone->parts} unless $clone->trailing_slash;
+  return $clone->trailing_slash(@{$clone->parts} ? 1 : 0);
+}
+
 sub to_route {
   my $self = shift;
   return '/' . join('/', @{$self->parts}) . ($self->trailing_slash ? '/' : '');
@@ -237,6 +243,15 @@ Turn path into an absolute string.
   # "/i/%E2%99%A5/mojolicious"
   Mojo::Path->new('i/%E2%99%A5/mojolicious')->to_abs_string;
 
+=head2 to_dir
+
+  my $dir = $route->to_dir;
+
+Clone path and remove everything after the right-most slash.
+
+  # "/foo/bar/"
+  Mojo::Path->new('/foo/bar/index.html');
+
 =head2 to_route
 
   my $route = $path->to_route;

diff --git a/lib/Mojo/UserAgent/CookieJar.pm b/lib/Mojo/UserAgent/CookieJar.pm
@@ -2,6 +2,7 @@ package Mojo::UserAgent::CookieJar;
 use Mojo::Base -base;
 
 use Mojo::Cookie::Request;
+use Mojo::Path;
 
 has max_cookie_size => 4096;
 
@@ -41,18 +42,27 @@ sub extract {
   my ($self, $tx) = @_;
   my $url = $tx->req->url;
   for my $cookie (@{$tx->res->cookies}) {
-    $cookie->domain($url->host) unless $cookie->domain;
-    $cookie->path($url->path)   unless $cookie->path;
-    $self->add($cookie);
+
+    # Validate domain
+    my $host = lc $url->host;
+    my $domain = lc($cookie->domain // $host);
+    next unless $host eq $domain || $host =~ /\Q.$domain\E$/;
+    $cookie->domain($domain);
+
+    # Validate path
+    my $path = $cookie->path // $url->path->to_dir->to_abs_string;
+    $path =~ s!/$!!;
+    next unless $url->path->contains(Mojo::Path->new($path)->to_route);
+    $self->add($cookie->path($path));
   }
 }
 
 sub find {
   my ($self, $url) = @_;
 
   # Look through the jar
-  return unless my $domain = $url->host;
-  my $path = $url->path->to_string || '/';
+  return unless my $domain = lc($url->host // '');
+  my $path = $url->path->to_abs_string;
   my @found;
   while ($domain =~ /[^.]+\.[^.]+|localhost$/) {
     next unless my $old = $self->{jar}{$domain};
@@ -68,7 +78,8 @@ sub find {
 
       # Taste cookie
       next if $cookie->secure && $url->protocol ne 'https';
-      next unless $path =~ /^\Q@{[$cookie->path]}/;
+      my $cpath = $cookie->path;
+      next unless $cpath eq '/' || $path eq $cpath || $path =~ m!^\Q$cpath/!;
       my $name  = $cookie->name;
       my $value = $cookie->value;
       push @found, Mojo::Cookie::Request->new(name => $name, value => $value);

diff --git a/lib/Mojolicious.pm b/lib/Mojolicious.pm
@@ -40,7 +40,7 @@ has static   => sub { Mojolicious::Static->new };
 has types    => sub { Mojolicious::Types->new };
 
 our $CODENAME = 'Rainbow';
-our $VERSION  = '3.77';
+our $VERSION  = '3.78';
 
 sub AUTOLOAD {
   my $self = shift;

diff --git a/t/mojo/cookiejar.t b/t/mojo/cookiejar.t
@@ -2,6 +2,7 @@ use Mojo::Base -strict;
 
 use Test::More;
 use Mojo::Cookie::Response;
+use Mojo::Transaction::HTTP;
 use Mojo::URL;
 use Mojo::UserAgent::CookieJar;
 
@@ -73,6 +74,12 @@ is $cookies[1], undef, 'no second cookie';
 is $cookies[0]->name,  'foo', 'right name';
 is $cookies[0]->value, 'bar', 'right value';
 is $cookies[1], undef, 'no second cookie';
+@cookies = $jar->find(Mojo::URL->new('http://kraih.com/foo/bar'));
+is $cookies[0]->name,  'foo', 'right name';
+is $cookies[0]->value, 'bar', 'right value';
+is $cookies[1], undef, 'no second cookie';
+@cookies = $jar->find(Mojo::URL->new('http://kraih.com/foobar'));
+is $cookies[0], undef, 'no cookies';
 
 # "localhost"
 $jar = Mojo::UserAgent::CookieJar->new;
@@ -302,5 +309,107 @@ $jar->add(
 is $cookies[0]->name,  'foo', 'right name';
 is $cookies[0]->value, 'bar', 'right value';
 is $cookies[1], undef, 'no second cookie';
+@cookies = $jar->find(Mojo::URL->new('http://kraih.com/foo%28bar/baz'));
+is $cookies[0]->name,  'foo', 'right name';
+is $cookies[0]->value, 'bar', 'right value';
+is $cookies[1], undef, 'no second cookie';
+
+# Extract and inject cookies without domain and path
+$jar = Mojo::UserAgent::CookieJar->new;
+my $tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://mojolicio.us/perldoc/Mojolicious');
+$tx->res->cookies(
+  Mojo::Cookie::Response->new(name => 'foo', value => 'without'));
+$jar->extract($tx);
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://mojolicio.us/perldoc');
+$jar->inject($tx);
+is $tx->req->cookie('foo')->name,  'foo',     'right name';
+is $tx->req->cookie('foo')->value, 'without', 'right value';
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://mojolicio.us/perldoc');
+$jar->inject($tx);
+is $tx->req->cookie('foo')->name,  'foo',     'right name';
+is $tx->req->cookie('foo')->value, 'without', 'right value';
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://mojolicio.us/whatever');
+$jar->inject($tx);
+is $tx->req->cookie('foo'), undef, 'no cookie';
+
+# Extract and inject cookies with domain and path
+$jar = Mojo::UserAgent::CookieJar->new;
+$tx  = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://LABS.Kraih.Com/perldoc/Mojolicious');
+$tx->res->cookies(
+  Mojo::Cookie::Response->new(
+    name   => 'foo',
+    value  => 'with',
+    domain => 'labs.KRAIH.com',
+    path   => '/perldoc'
+  ),
+  Mojo::Cookie::Response->new(
+    name   => 'bar',
+    value  => 'with',
+    domain => 'KRAIH.com',
+    path   => '/'
+  )
+);
+$jar->extract($tx);
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://labs.kraih.COM/perldoc');
+$jar->inject($tx);
+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';
+$tx = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://labs.kraih.COM/Perldoc');
+$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';
+
+# Extract cookies with invalid domain
+$jar = Mojo::UserAgent::CookieJar->new;
+$tx  = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://labs.kraih.com/perldoc/Mojolicious');
+$tx->res->cookies(
+  Mojo::Cookie::Response->new(
+    name   => 'foo',
+    value  => 'invalid',
+    domain => 'a.s.kraih.com'
+  ),
+  Mojo::Cookie::Response->new(
+    name   => 'foo',
+    value  => 'invalid',
+    domain => 'mojolicio.us'
+  )
+);
+$jar->extract($tx);
+is_deeply [$jar->all], [], 'no cookies';
+
+# Extract cookies with invalid path
+$jar = Mojo::UserAgent::CookieJar->new;
+$tx  = Mojo::Transaction::HTTP->new;
+$tx->req->url->parse('http://labs.kraih.com/perldoc/Mojolicious');
+$tx->res->cookies(
+  Mojo::Cookie::Response->new(
+    name  => 'foo',
+    value => 'invalid',
+    path  => '/perldoc/index.html'
+  ),
+  Mojo::Cookie::Response->new(
+    name  => 'foo',
+    value => 'invalid',
+    path  => '/perldocMojolicious'
+  ),
+  Mojo::Cookie::Response->new(
+    name  => 'foo',
+    value => 'invalid',
+    path  => '/perldoc.Mojolicious'
+  )
+);
+$jar->extract($tx);
+is_deeply [$jar->all], [], 'no cookies';
 
 done_testing();
diff --git a/t/mojo/path.t b/t/mojo/path.t
@@ -6,11 +6,14 @@ use Mojo::Path;
 # Basic functionality
 my $path = Mojo::Path->new;
 is $path->parse('/path')->to_string, '/path', 'right path';
+is $path->to_dir, '/', 'right directory';
 is $path->parts->[0], 'path', 'right part';
 is $path->parts->[1], undef,  'no part';
 ok $path->leading_slash, 'has leading slash';
 ok !$path->trailing_slash, 'no trailing slash';
 is $path->parse('path/')->to_string, 'path/', 'right path';
+is $path->to_dir, 'path/', 'right directory';
+is $path->to_dir->to_abs_string, '/path/', 'right directory';
 is $path->parts->[0], 'path', 'right part';
 is $path->parts->[1], undef,  'no part';
 ok !$path->leading_slash, 'no leading slash';
@@ -29,6 +32,7 @@ is "$path", '/AZaz09-._~!$&\'()*+,;=:@', 'right result';
 
 # Unicode
 is $path->parse('/foo/♥/bar')->to_string, '/foo/%E2%99%A5/bar', 'right path';
+is $path->to_dir, '/foo/%E2%99%A5/', 'right directory';
 is $path->parts->[0], 'foo', 'right part';
 is $path->parts->[1], '♥', 'right part';
 is $path->parts->[2], 'bar', 'right part';