Skip to content

Commit

Permalink
qa: updates psr-7 integration test version
Browse files Browse the repository at this point in the history
Updates to 1.2.0, which adds the tests we wrote for mitigating ZF2015-05, with a few changes:

- When creating the string representation of the URL, we DO NOT normalize the path to remove multiple leading slashes.
  In its absolute form, this is not necessary.
- All normalization is done via `getPath()`; this mitigates the common XSS scenario.
- It adds a test to validate that when using origin-form during a `RequestInterface::getRequestTarget()` call, it will use the results of `getPath()`, as this is a scenario where the XSS could also occur.

I have removed one test from `UriTest`, as it contradicts the first point above.
Since the scenario is covered in the PSR-7 integration tests, we are covered.

See php-http/psr7-integration-tests#54 for more details.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
  • Loading branch information
weierophinney committed Dec 14, 2022
1 parent cf1dc9e commit b17abb2
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 50 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"ext-libxml": "*",
"http-interop/http-factory-tests": "^0.9.0",
"laminas/laminas-coding-standard": "^2.4.0",
"php-http/psr7-integration-tests": "^1.1.1",
"php-http/psr7-integration-tests": "^1.2",
"phpunit/phpunit": "^9.5.26",
"psalm/plugin-phpunit": "^0.18.0",
"vimeo/psalm": "^5.0.0"
Expand Down
140 changes: 101 additions & 39 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 14 additions & 3 deletions src/Uri.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public function __toString(): string
$this->uriString = static::createUriString(
$this->scheme,
$this->getAuthority(),
$this->getPath(), // Absolute URIs should use a "/" for an empty path
$this->path, // Absolute URIs should use a "/" for an empty path
$this->query,
$this->fragment
);
Expand Down Expand Up @@ -185,7 +185,18 @@ public function getPort(): ?int
*/
public function getPath(): string
{
return $this->path;
if ('' === $this->path) {
// No path
return $this->path;
}

if ($this->path[0] !== '/') {
// Relative path
return $this->path;
}

// Ensure only one leading slash, to prevent XSS attempts.
return '/' . ltrim($this->path, '/');
}

/**
Expand Down Expand Up @@ -557,7 +568,7 @@ private function filterPath(string $path): string
{
$path = $this->filterInvalidUtf8($path);

$path = preg_replace_callback(
return preg_replace_callback(
'/(?:[^' . self::CHAR_UNRESERVED . ')(:@&=\+\$,\/;%]+|%(?![A-Fa-f0-9]{2}))/u',
[$this, 'urlEncodeChar'],
$path
Expand Down
7 changes: 0 additions & 7 deletions test/UriTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -575,13 +575,6 @@ public function testFragmentIsNotDoubleEncoded(): void
$this->assertSame($expected, $uri->getFragment());
}

public function testProperlyTrimsLeadingSlashesToPreventXSS(): void
{
$url = 'http://example.org//zend.com';
$uri = new Uri($url);
$this->assertSame('http://example.org/zend.com', (string) $uri);
}

/** @return non-empty-array<string, array{'withScheme'|'withUserInfo'|'withHost'|'withPath'|'withQuery'|'withFragment', mixed}> */
public function invalidStringComponentValues(): array
{
Expand Down

0 comments on commit b17abb2

Please sign in to comment.