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

Jetty 12 - Cherry-Pick of Servlet PathMapping improvements around RegexPathSpec #8693

Merged
merged 8 commits into from
Oct 7, 2022

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Oct 6, 2022

This PR is a result of a merge analysis done due to PR #8690

This PR is cherry-pick of the following PathMapping related commits from Jetty 10/11.

gregw and others added 3 commits October 6, 2022 15:42
…xtHandler (#7614)

Added protected method to ServletHandler to allow other servlet mappings (eg regex) in embedded/extended usage

Signed-off-by: Greg Wilkins <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Fix 7891 regex pathInfo

+ Use the pathSpec methods to set servletPath and pathInfo when possible

Signed-off-by: Greg Wilkins <[email protected]>
* Cherry-pick of Improvements to PathSpec.
* From commit: 5b4d1dd
* Fixing ConstraintSecurityHandler usage of PathMappings
* Fixing bad INCLUDE logic from cherry-pick in ServletHandler.doScope()
* Cleanup of non ServletPathSpec behaviors in ServletPathMapping class
* Skip optional group name/info lookup if regex fails.
* Prevent NPE on static servletPathMappings
* Update WebSocketMappings to use new PathMappings.getMatched(String)

Signed-off-by: Joakim Erdfelt <[email protected]>
@joakime joakime requested a review from sbordet October 6, 2022 20:53
@joakime joakime self-assigned this Oct 6, 2022
@joakime joakime requested a review from sbordet October 7, 2022 10:53
@joakime joakime marked this pull request as draft October 7, 2022 12:44
@joakime
Copy link
Contributor Author

joakime commented Oct 7, 2022

This is not even close to being ready.

The change in 12.0.x for MappedServlet is quite dramatic (from how it was in 10.0.x or 11.0.x)
It's not handling the MatchedPath properly ATM.

@joakime joakime marked this pull request as ready for review October 7, 2022 15:06
@joakime joakime merged commit 98c3805 into jetty-12.0.x Oct 7, 2022
@joakime joakime deleted the fix/jetty-12-servlet-pathmapping-regex branch October 7, 2022 15:36
return null;
}

public ServletPathMapping getServletPathMapping(String pathInContext, MatchedPath matchedPath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gregw this is the new method that isn't being used consistently that I'm concerned about.

There is also a getServletPathMapping(String pathInContext) above this method that I couldn't get rid of (we should be able to get rid of it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants