Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update PSR-7 integration tests #128

Merged

Conversation

weierophinney
Copy link
Member

Q A
Documentation no
Bugfix yes
BC Break slight behavior change
New Feature no
RFC no
QA yes

Description

Updates to 1.2.0 of the PSR-7 integration test suite, which adds the tests we wrote for mitigating ZF2015-05, and requires 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.

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>
@weierophinney weierophinney added this to the 2.23.0 milestone Dec 14, 2022
@weierophinney weierophinney linked an issue Dec 14, 2022 that may be closed by this pull request
Since I was able to return earlier in `filterPath()`, I was able to remove a number of dead conditionals.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Flagged by Psalm.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks @weierophinney!

@Ocramius Ocramius self-assigned this Dec 14, 2022
@Ocramius Ocramius merged commit a738cec into laminas:2.23.x Dec 14, 2022
@weierophinney weierophinney deleted the feature/psr-7-integration-tests branch April 6, 2023 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PSR-7 Integration Tests Failing
3 participants