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.

Sign up for GitHub

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

Fix ServletContext.getResourcePaths() #9974

Merged
merged 4 commits into from
Jun 28, 2023
Merged

Fix ServletContext.getResourcePaths() #9974

merged 4 commits into from
Jun 28, 2023

Conversation

lorban
Copy link
Contributor

@lorban lorban commented Jun 26, 2023 •
edited
Loading

The ServletContextApi.getResourceAsStream(), ServletContextApi.getResource() and ServletContextApi.getResourcePaths() are canonicalizing the path parameter while they should be normalizing it instead.

Fixes #9972

@lorban
#9972 add reproducer
89af30a 
Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban changed the base branch from jetty-10.0.x to jetty-12.0.x June 26, 2023 17:05
@lorban
#9972 tentative fix
f0c81bf 
Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban requested review from gregw and joakime June 26, 2023 17:19
@lorban lorban self-assigned this Jun 26, 2023
@lorban lorban added the Bug For general bugs on Jetty side label Jun 26, 2023
@lorban
fix checkstyle
34a089e 
Signed-off-by: Ludovic Orban <[email protected]>
gregw
gregw requested changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

I think we have to at least normalize out ../../../../etc/passwd arguments. See my comments for more info.

@joakime joakime mentioned this pull request Jun 26, 2023
Closed
@lorban
#9972 proper fix
59a554c 
Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban marked this pull request as ready for review June 27, 2023 12:47
@lorban
Copy link
Contributor Author

lorban commented Jun 27, 2023

@gregw thanks for the details. I changed the canonicalization into a normalization.

Unfortunately, this uncovered a JDK bug for which @joakime added a reproducer: #9978

@lorban lorban requested a review from gregw June 27, 2023 12:49
gregw
gregw approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

LGTM

@lorban lorban merged commit c002adb into jetty-12.0.x Jun 28, 2023
@lorban lorban deleted the fix/jetty-12-9972-ServletContext-getResourcePaths branch June 28, 2023 06:50
@joakime joakime mentioned this pull request Jul 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw approved these changes

@joakime joakime Awaiting requested review from joakime

Assignees

@lorban lorban

Labels
Bug For general bugs on Jetty side
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

getResourcePaths fails when a META-INF resource has reserved characters in its filename
2 participants
@lorban @gregw