From aef71d0168394b2c842711b8d859afe502c6f4be Mon Sep 17 00:00:00 2001 From: Chris Kankiewicz Date: Mon, 6 Apr 2020 10:53:08 -0700 Subject: [PATCH] Fixed unable to navigate to directories with URL special characters Resolves issue #338 --- app/src/ViewFunctions/Breadcrumbs.php | 2 +- app/src/ViewFunctions/Url.php | 35 +++++++++++++++++++++++-- app/views/components/file.twig | 2 +- tests/ViewFunctions/BreadcrumbsTest.php | 20 ++++++++++++++ tests/ViewFunctions/UrlTest.php | 16 +++++++++-- 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/app/src/ViewFunctions/Breadcrumbs.php b/app/src/ViewFunctions/Breadcrumbs.php index 22e5010..f061263 100644 --- a/app/src/ViewFunctions/Breadcrumbs.php +++ b/app/src/ViewFunctions/Breadcrumbs.php @@ -41,7 +41,7 @@ class Breadcrumbs extends ViewFunction return $crumb !== '.'; })->reduce(function (Collection $carry, string $crumb) { return $carry->put($crumb, ltrim( - $carry->last() . DIRECTORY_SEPARATOR . $crumb, DIRECTORY_SEPARATOR + $carry->last() . DIRECTORY_SEPARATOR . urlencode($crumb), DIRECTORY_SEPARATOR )); }, new Collection)->map(function (string $path): string { return sprintf('?dir=%s', $path); diff --git a/app/src/ViewFunctions/Url.php b/app/src/ViewFunctions/Url.php index 1bb8b4d..484d9a5 100644 --- a/app/src/ViewFunctions/Url.php +++ b/app/src/ViewFunctions/Url.php @@ -2,11 +2,26 @@ namespace App\ViewFunctions; +use App\Support\Str; + class Url extends ViewFunction { /** @var string The function name */ protected $name = 'url'; + /** @var string The directory separator */ + protected $directorySeparator; + + /** + * Create a new Url object. + * + * @param string The directory separator + */ + public function __construct(string $directorySeparator = DIRECTORY_SEPARATOR) + { + $this->directorySeparator = $directorySeparator; + } + /** * Return the URL for a given path. * @@ -19,9 +34,25 @@ class Url extends ViewFunction $path = preg_replace('/^.?(\/|\\\)+/', '', $path); if (is_file($path)) { - return $path; + return $this->escape($path); } - return empty($path) ? '' : sprintf('?dir=%s', $path); + return empty($path) ? '' : sprintf('?dir=%s', $this->escape($path)); + } + + /** + * Escape URL characters in path segments. + * + * @param string $path + * + * @return string + */ + protected function escape(string $path): string + { + return Str::explode($path, $this->directorySeparator)->map( + function (string $segment): string { + return urlencode($segment); + } + )->implode($this->directorySeparator); } } diff --git a/app/views/components/file.twig b/app/views/components/file.twig index 28e22f2..136ac43 100644 --- a/app/views/components/file.twig +++ b/app/views/components/file.twig @@ -20,7 +20,7 @@ diff --git a/tests/ViewFunctions/BreadcrumbsTest.php b/tests/ViewFunctions/BreadcrumbsTest.php index 6e2604e..b8ec3c4 100644 --- a/tests/ViewFunctions/BreadcrumbsTest.php +++ b/tests/ViewFunctions/BreadcrumbsTest.php @@ -26,6 +26,26 @@ class BreadcrumbsTest extends TestCase $this->assertEquals(new Collection, $breadcrumbs('.')); } + public function test_it_url_encodes_directory_names(): void + { + $breadcrumbs = new Breadcrumbs($this->container); + + $this->assertEquals(Collection::make([ + 'foo' => '?dir=foo', + 'bar+baz' => '?dir=foo/bar%2Bbaz', + ]), $breadcrumbs('foo/bar+baz')); + + $this->assertEquals(Collection::make([ + 'foo' => '?dir=foo', + 'bar#baz' => '?dir=foo/bar%23baz', + ]), $breadcrumbs('foo/bar#baz')); + + $this->assertEquals(Collection::make([ + 'foo' => '?dir=foo', + 'bar&baz' => '?dir=foo/bar%26baz', + ]), $breadcrumbs('foo/bar&baz')); + } + public function test_it_can_parse_breadcrumbs_from_a_subdirectory(): void { $_SERVER['SCRIPT_NAME'] = '/some/dir/index.php'; diff --git a/tests/ViewFunctions/UrlTest.php b/tests/ViewFunctions/UrlTest.php index 9a6b670..c7d2393 100644 --- a/tests/ViewFunctions/UrlTest.php +++ b/tests/ViewFunctions/UrlTest.php @@ -11,7 +11,6 @@ class UrlTest extends TestCase { $url = new Url; - // Forward slashes $this->assertEquals('', $url('/')); $this->assertEquals('', $url('./')); $this->assertEquals('?dir=some/path', $url('some/path')); @@ -19,8 +18,12 @@ class UrlTest extends TestCase $this->assertEquals('?dir=some/path', $url('./some/path')); $this->assertEquals('?dir=some/file.test', $url('some/file.test')); $this->assertEquals('?dir=some/file.test', $url('./some/file.test')); + } + + public function test_it_can_return_a_url_for_a_directory_using_back_slashes(): void + { + $url = new Url('\\'); - // Back slashes $this->assertEquals('', $url('\\')); $this->assertEquals('', $url('.\\')); $this->assertEquals('?dir=some\path', $url('some\path')); @@ -40,4 +43,13 @@ class UrlTest extends TestCase $this->assertEquals('subdir/alpha.scss', $url('subdir/alpha.scss')); $this->assertEquals('subdir/alpha.scss', $url('./subdir/alpha.scss')); } + + public function test_it_url_encodes_directory_names(): void + { + $url = new Url; + + $this->assertEquals('?dir=foo/bar%2Bbaz', $url('foo/bar+baz')); + $this->assertEquals('?dir=foo/bar%23baz', $url('foo/bar#baz')); + $this->assertEquals('?dir=foo/bar%26baz', $url('foo/bar&baz')); + } }