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

Jump to bottom

Improve alias checking in PathResource #6473

Closed
gregw opened this issue Jun 26, 2021 · 1 comment · Fixed by #6474
Closed

Improve alias checking in PathResource #6473

gregw opened this issue Jun 26, 2021 · 1 comment · Fixed by #6474
Labels
Enhancement
Projects
Jetty 9.4.43 FROZEN Jetty 10.0.6/11.0.6 FROZEN

Comments

@gregw
Copy link
Contributor

gregw commented Jun 26, 2021 •
edited

Target Jetty version(s)
9.10,11

Enhancement Description

Currently multiple calls to canonicalPath are done, which is inefficient repeat scanning. Include normalization in alias checks, as was previously done in FileResource, so that only a single normalization pass is needed.
@gregw gregw changed the title Improve alias checking in Improve alias checking in PathResource Jun 26, 2021
@gregw gregw mentioned this issue Jun 26, 2021
Merged
@gregw gregw linked a pull request Jun 26, 2021 that will close this issue
Jetty 9.4.x #6473 normalize only once #6474
Merged
@gregw gregw added this to To do in Jetty 9.4.43 FROZEN via automation Jun 26, 2021
@gregw gregw added this to To do in Jetty 10.0.6/11.0.6 FROZEN via automation Jun 26, 2021
lachlan-roberts added a commit that referenced this issue Jun 27, 2021
@lachlan-roberts
Issue #6473 - normalize only once
ab49940 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Jun 28, 2021
@lachlan-roberts
Issue #6473 - canonicalPath refactor & fix alias check in PathResource
c2d5a82 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
gregw added a commit that referenced this issue Jun 28, 2021
@gregw
Issue #6473 - canonicalPath refactor & fix alias check in PathResource (
122a78a 
#6474)

Reduce multiple canonicalPath calls with single alias check in PathResource
Revert to decoding and the normalizing URLs so that subsequent canonicalPath calls are noops. 
Co-authored-by: Lachlan Roberts <lachlan@webtide.com>
sbordet added a commit that referenced this issue Jun 28, 2021
@sbordet
Issue #6473 - Improve alias checking in PathResource.
ee4dd2a 
* Reverted %-escape handling for URI query parts.
* Performing canonicalization in ServletContext.getResource(),
  and improving alias checking in ContextHandler.getResource().
* Performing canonicalization checks in Resource.addPath() to avoid
  navigation above of the root.
* Test added and fixed.
* Various cleanups.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
lachlan-roberts added a commit that referenced this issue Jun 29, 2021
@lachlan-roberts
Issue #6473 - canonicalPath refactor & fix alias check in PathResource
92a74d6 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
gregw pushed a commit that referenced this issue Jun 29, 2021
@sbordet @gregw
Issue #6473 - Improve alias checking in PathResource.
2e75feb 
* Reverted %-escape handling for URI query parts.
* Performing canonicalization in ServletContext.getResource(),
  and improving alias checking in ContextHandler.getResource().
* Performing canonicalization checks in Resource.addPath() to avoid
  navigation above of the root.
* Test added and fixed.
* Various cleanups.
* Improved javadoc and comments

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
lachlan-roberts pushed a commit that referenced this issue Jun 29, 2021
@sbordet @lachlan-roberts
Issue #6473 - Improve alias checking in PathResource.
4cf9ca7 
* Reverted %-escape handling for URI query parts.
* Performing canonicalization in ServletContext.getResource(),
  and improving alias checking in ContextHandler.getResource().
* Performing canonicalization checks in Resource.addPath() to avoid
  navigation above of the root.
* Test added and fixed.
* Various cleanups.
* Improved javadoc and comments

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This was referenced Jun 29, 2021
Merged
Merged
sbordet added a commit that referenced this issue Jun 29, 2021
@sbordet @gregw
Issue #6473 - Improve alias checking in PathResource. (#6477)
f045b5a 
* Issue #6473 - Improve alias checking in PathResource.

* Reverted %-escape handling for URI query parts.
* Performing canonicalization in ServletContext.getResource(),
  and improving alias checking in ContextHandler.getResource().
* Performing canonicalization checks in Resource.addPath() to avoid
  navigation above of the root.
* Test added and fixed.
* Various cleanups.
* Improved javadoc and comments

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
sbordet pushed a commit that referenced this issue Jun 29, 2021
@lachlan-roberts @gregw
Issue #6473 - canonicalPath refactor & fix alias check in PathResourc…
3c32afa 
…e (Jetty-10) (#6478)

Issue #6473 - canonicalPath refactor & fix alias check in PathResource

* Reverted %-escape handling for URI query parts.
* Performing canonicalization in ServletContext.getResource(),
  and improving alias checking in ContextHandler.getResource().
* Performing canonicalization checks in Resource.addPath() to avoid
  navigation above of the root.
* Test added and fixed.
* Various cleanups.
* Improved javadoc and comments
* Compliance mode HttpURI uses UriCompliance.Violation

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw moved this from To do to Done in Jetty 9.4.43 FROZEN Jul 1, 2021
@gregw gregw moved this from To do to Done in Jetty 10.0.6/11.0.6 FROZEN Jul 1, 2021
@lachlan-roberts
Copy link
Contributor

This is done in both the 9.4.43 project and the 10.0.6/11.0.6 project so I think this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement
Projects
No open projects
Jetty 10.0.6/11.0.6 FROZEN
  
Done
Jetty 9.4.43 FROZEN
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Jetty 9.4.x #6473 normalize only once jetty/jetty.project
2 participants
@gregw @lachlan-roberts