fixed empty path element bug in Mojo::Path

mojolicious · Nov 4, 2011 · b7ea220 · b7ea220
1 parent d5989e2
commit b7ea220
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 62 deletions.
diff --git a/Changes b/Changes
@@ -3,6 +3,7 @@ This file documents the revision history for Perl extension Mojolicious.
 2.23    2011-11-04 00:00:00
         - Updated jQuery to version 1.7.
         - Improved documentation.
+        - Fixed empty path element bug in Mojo::Path.
 
 2.22    2011-11-03 00:00:00
         - Added EXPERIMENTAL --verbose flag to routes command.

diff --git a/lib/Mojo/Path.pm b/lib/Mojo/Path.pm
@@ -11,11 +11,7 @@ use Mojo::URL;
 has [qw/leading_slash trailing_slash/];
 has parts => sub { [] };
 
-sub new {
-  my $self = shift->SUPER::new();
-  $self->parse(@_);
-  return $self;
-}
+sub new { shift->SUPER::new()->parse(@_) }
 
 # DEPRECATED in Smiling Face With Sunglasses!
 sub append {
@@ -32,26 +28,23 @@ sub canonicalize {
   my $self = shift;
 
   # Resolve path
-  my @path;
+  my @parts;
   for my $part (@{$self->parts}) {
 
     # ".."
     if ($part eq '..') {
-
-      # Leading '..' can't be resolved
-      unless (@path && $path[-1] ne '..') { push @path, '..' }
-
-      # Uplevel
-      else { pop @path }
+      unless (@parts && $parts[-1] ne '..') { push @parts, '..' }
+      else                                  { pop @parts }
       next;
     }
 
     # "."
-    next if $part eq '.';
+    next if $part ~~ ['.', ''];
 
-    push @path, $part;
+    # Part
+    push @parts, $part;
   }
-  $self->parts(\@path);
+  $self->parts(\@parts);
 
   return $self;
 }
@@ -86,21 +79,11 @@ sub parse {
   my ($self, $path) = @_;
   $path //= '';
 
-  # Leading and trailing slash
-  $path =~ m#^/# ? $self->leading_slash(1)  : $self->leading_slash(undef);
-  $path =~ m#/$# ? $self->trailing_slash(1) : $self->trailing_slash(undef);
-
-  # Parse
   $path = url_unescape $path;
   utf8::decode $path;
-  my @parts;
-  for my $part (split '/', $path) {
-
-    # Empty parts before the first are garbage
-    next unless length $part or @parts;
-    push @parts, $part;
-  }
-  $self->parts(\@parts);
+  $path =~ s|^/|| ? $self->leading_slash(1)  : $self->leading_slash(undef);
+  $path =~ s|/$|| ? $self->trailing_slash(1) : $self->trailing_slash(undef);
+  $self->parts([split '/', $path, -1]);
 
   return $self;
 }
@@ -114,15 +97,15 @@ sub to_string {
   my $self = shift;
 
   # Escape
-  my @path = map {
+  my @parts = map {
     url_escape(encode('UTF-8', $_),
       "$Mojo::URL::UNRESERVED$Mojo::URL::SUBDELIM\:\@")
   } @{$self->parts};
 
   # Format
-  my $path = join '/', @path;
+  my $path = join '/', @parts;
   $path = "/$path" if $self->leading_slash;
-  $path = "$path/" if @path && $self->trailing_slash;
+  $path = "$path/" if @parts && $self->trailing_slash;
 
   return $path;
 }

diff --git a/t/mojo/path.t b/t/mojo/path.t
@@ -3,7 +3,7 @@ use Mojo::Base -strict;
 
 use utf8;
 
-use Test::More tests => 127;
+use Test::More tests => 173;
 
 # "This is the greatest case of false advertising I’ve seen since I sued the
 #  movie 'The Never Ending Story.'"
@@ -14,13 +14,13 @@ my $path = Mojo::Path->new;
 is $path->parse('/path')->to_string, '/path', 'right path';
 is $path->parts->[0], 'path', 'right part';
 is $path->parts->[1], undef,  'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+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->parts->[0], 'path', 'right part';
 is $path->parts->[1], undef,  'no part';
-is $path->leading_slash,  undef, 'no leading slash';
-is $path->trailing_slash, 1,     'has trailing slash';
+ok !$path->leading_slash, 'no leading slash';
+ok $path->trailing_slash, 'has trailing slash';
 
 # Unicode
 is $path->parse('/foo/♥/bar')->to_string, '/foo/%E2%99%A5/bar',
@@ -29,30 +29,30 @@ is $path->parts->[0], 'foo', 'right part';
 is $path->parts->[1], '♥', 'right part';
 is $path->parts->[2], 'bar', 'right part';
 is $path->parts->[3], undef, 'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
 is $path->parse('/foo/%E2%99%A5/bar')->to_string, '/foo/%E2%99%A5/bar',
   'right path';
 is $path->parts->[0], 'foo', 'right part';
 is $path->parts->[1], '♥', 'right part';
 is $path->parts->[2], 'bar', 'right part';
 is $path->parts->[3], undef, 'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
 
 # Zero in path
 is $path->parse('/path/0')->to_string, '/path/0', 'right path';
 is $path->parts->[0], 'path', 'right part';
 is $path->parts->[1], '0',    'right part';
 is $path->parts->[2], undef,  'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
 
 # Canonicalizing
 $path = Mojo::Path->new(
   '/%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc%2fpasswd');
-is "$path", '/../../../../../../../../../../etc/passwd', 'rigth result';
-is $path->parts->[0],  '..',     'right part';
+is "$path", '//../../../../../../../../../../etc/passwd', 'rigth result';
+is $path->parts->[0],  '',       'right part';
 is $path->parts->[1],  '..',     'right part';
 is $path->parts->[2],  '..',     'right part';
 is $path->parts->[3],  '..',     'right part';
@@ -62,9 +62,10 @@ is $path->parts->[6],  '..',     'right part';
 is $path->parts->[7],  '..',     'right part';
 is $path->parts->[8],  '..',     'right part';
 is $path->parts->[9],  '..',     'right part';
-is $path->parts->[10], 'etc',    'right part';
-is $path->parts->[11], 'passwd', 'right part';
-is $path->parts->[12], undef,    'no part';
+is $path->parts->[10], '..',     'right part';
+is $path->parts->[11], 'etc',    'right part';
+is $path->parts->[12], 'passwd', 'right part';
+is $path->parts->[13], undef,    'no part';
 is $path->canonicalize, '/../../../../../../../../../../etc/passwd',
   'rigth result';
 is $path->parts->[0],  '..',     'right part';
@@ -80,12 +81,12 @@ is $path->parts->[9],  '..',     'right part';
 is $path->parts->[10], 'etc',    'right part';
 is $path->parts->[11], 'passwd', 'right part';
 is $path->parts->[12], undef,    'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
 
 # Canonicalizing (alternative)
 $path = Mojo::Path->new(
-  '/%2ftest%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc%2fpasswd');
+  '%2ftest%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc%2fpasswd');
 is "$path", '/test/../../../../../../../../../etc/passwd', 'rigth result';
 is $path->parts->[0],  'test',   'right part';
 is $path->parts->[1],  '..',     'right part';
@@ -112,11 +113,11 @@ is $path->parts->[7],  '..',     'right part';
 is $path->parts->[8],  'etc',    'right part';
 is $path->parts->[9],  'passwd', 'right part';
 is $path->parts->[10], undef,    'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
 
 # Canonicalizing (with escaped "%")
-$path = Mojo::Path->new('/%2ftest%2f..%252f..%2f..%2f..%2f..%2fetc%2fpasswd');
+$path = Mojo::Path->new('%2ftest%2f..%252f..%2f..%2f..%2f..%2fetc%2fpasswd');
 is "$path", '/test/..%252f../../../../etc/passwd', 'rigth result';
 is $path->parts->[0], 'test',    'right part';
 is $path->parts->[1], '..%2f..', 'right part';
@@ -131,8 +132,8 @@ is $path->parts->[0], '..',     'right part';
 is $path->parts->[1], 'etc',    'right part';
 is $path->parts->[2], 'passwd', 'right part';
 is $path->parts->[3], undef,    'no part';
-is $path->leading_slash,  1,     'has leading slash';
-is $path->trailing_slash, undef, 'no trailing slash';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
 
 # Contains
 $path = Mojo::Path->new('/foo/bar');
@@ -162,3 +163,55 @@ ok !$path->contains('/0/♥'),    'does not contain path';
 ok !$path->contains('/0/0.html'), 'does not contain path';
 ok !$path->contains('/0.html'),   'does not contain path';
 ok !$path->contains('/♥.html'), 'does not contain path';
+
+# Empty path elements
+$path = Mojo::Path->new('/foo//bar/23/');
+is "$path", '/foo//bar/23/';
+is $path->parts->[0], 'foo', 'right part';
+is $path->parts->[1], '',    'right part';
+is $path->parts->[2], 'bar', 'right part';
+is $path->parts->[3], '23',  'right part';
+is $path->parts->[4], undef, 'no part';
+ok $path->leading_slash,  'has leading slash';
+ok $path->trailing_slash, 'has trailing slash';
+$path = Mojo::Path->new('//foo/bar/23/');
+is "$path", '//foo/bar/23/';
+is $path->parts->[0], '',    'right part';
+is $path->parts->[1], 'foo', 'right part';
+is $path->parts->[2], 'bar', 'right part';
+is $path->parts->[3], '23',  'right part';
+is $path->parts->[4], undef, 'no part';
+ok $path->leading_slash,  'has leading slash';
+ok $path->trailing_slash, 'has trailing slash';
+$path = Mojo::Path->new('/foo///bar/23/');
+is "$path", '/foo///bar/23/';
+is $path->parts->[0], 'foo', 'right part';
+is $path->parts->[1], '',    'right part';
+is $path->parts->[2], '',    'right part';
+is $path->parts->[3], 'bar', 'right part';
+is $path->parts->[4], '23',  'right part';
+is $path->parts->[5], undef, 'no part';
+ok $path->leading_slash,  'has leading slash';
+ok $path->trailing_slash, 'has trailing slash';
+$path = Mojo::Path->new('///foo/bar/23/');
+is "$path", '///foo/bar/23/';
+is $path->parts->[0], '',    'right part';
+is $path->parts->[1], '',    'right part';
+is $path->parts->[2], 'foo', 'right part';
+is $path->parts->[3], 'bar', 'right part';
+is $path->parts->[4], '23',  'right part';
+is $path->parts->[5], undef, 'no part';
+ok $path->leading_slash,  'has leading slash';
+ok $path->trailing_slash, 'has trailing slash';
+$path = Mojo::Path->new('///foo/bar/23///');
+is "$path", '///foo/bar/23///';
+is $path->parts->[0], '',    'right part';
+is $path->parts->[1], '',    'right part';
+is $path->parts->[2], 'foo', 'right part';
+is $path->parts->[3], 'bar', 'right part';
+is $path->parts->[4], '23',  'right part';
+is $path->parts->[5], '',    'right part';
+is $path->parts->[6], '',    'right part';
+is $path->parts->[7], undef, 'no part';
+ok $path->leading_slash,  'has leading slash';
+ok $path->trailing_slash, 'has trailing slash';
diff --git a/t/mojo/url.t b/t/mojo/url.t
@@ -118,13 +118,13 @@ is $rel, '', 'right relative version';
 is $rel->to_abs, 'http://kraih.com/', 'right absolute version';
 is $rel->to_abs->to_rel, '', 'right relative version';
 $rel = $url->to_rel(Mojo::URL->new('http://kraih.com/a/'));
-is $rel, '../', 'right relative version';
+is $rel, '..', 'right relative version';
 is $rel->to_abs, 'http://kraih.com/', 'right absolute version';
-is $rel->to_abs->to_rel, '../', 'right relative version';
+is $rel->to_abs->to_rel, '..', 'right relative version';
 $rel = $url->to_rel(Mojo::URL->new('http://kraih.com/a/b/'));
-is $rel, '../../', 'right relative version';
+is $rel, '../..', 'right relative version';
 is $rel->to_abs, 'http://kraih.com/', 'right absolute version';
-is $rel->to_abs->to_rel, '../../', 'right relative version';
+is $rel->to_abs->to_rel, '../..', 'right relative version';
 $url = Mojo::URL->new('http://kraih.com/index.html');
 $rel = $url->to_rel(Mojo::URL->new('http://kraih.com/'));
 is $rel, 'index.html', 'right relative version';
@@ -339,7 +339,7 @@ is $url->to_rel, 'foo//bar/23/', 'right relative version';
 $url = Mojo::URL->new('http://kraih.com//foo//bar/23/');
 $url->base->parse('http://kraih.com/');
 ok $url->is_abs, 'is absolute';
-is $url->to_rel, 'foo//bar/23/', 'right relative version';
+is $url->to_rel, '/foo//bar/23/', 'right relative version';
 $url = Mojo::URL->new('http://kraih.com/foo///bar/23/');
 $url->base->parse('http://kraih.com/');
 ok $url->is_abs, 'is absolute';

diff --git a/t/mojolicious/rebased_lite_app.t b/t/mojolicious/rebased_lite_app.t
@@ -45,8 +45,8 @@ $t->get_ok('/foo')->status_is(200)->content_is(<<EOF);
 <base href="http://kraih.com/rebased/" />
 <link href="/rebased/b.css" media="test" rel="stylesheet" type="text/css" />
 <img alt="Test" src="/rebased/images/test.png" />
-http://kraih.com/rebased/
-/rebased/
+http://kraih.com/rebased
+/rebased
 http://kraih.com/
 EOF