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

Issue #5057 - Included root context path #5058

Merged
merged 3 commits into from
Jul 20, 2020
Merged

Issue #5057 - Included root context path #5058

merged 3 commits into from
Jul 20, 2020

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Jul 16, 2020

Root context path in include should be empty string.

@gregw
Fixes #5057 Included root context path
ea0ce64 
Root context path in include should be empty string.
@gregw gregw linked an issue Jul 16, 2020 that may be closed by this pull request
javax.servlet.include.context_path attribute on root context. should be empty string, but is "/" #5057
Closed
joakime
joakime approved these changes Jul 16, 2020
View reviewed changes
Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

Looks pretty straight forward to me.
+1

@joakime joakime added Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc) labels Jul 16, 2020
@joakime joakime changed the title Jetty 9.4.x #5057 included context path Issue #5057 - Included root context path Jul 16, 2020
janbartel
janbartel requested changes Jul 20, 2020
View reviewed changes
Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

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

Just clarify the difference with ContextHandler.Context.getContextPath() (ie should one use the other?) and then I'm +1.

gregw added 2 commits July 20, 2020 13:54
@gregw
Merge branch 'jetty-9.4.x' into jetty-9.4.x-5057-IncludedContextPath
4feed0a
@gregw
Issue #5057
1fbe474 
merged context path methods as result of review.
ServletContent.getContextPath now returns the encoded contextPath (if anybody is silly enough to have one).
@gregw gregw merged commit 65de149 into jetty-9.4.x Jul 20, 2020
lachlan-roberts added a commit that referenced this pull request Jul 23, 2020
@lachlan-roberts
disable test until merge of #5058 is fixed
cd3f495 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts mentioned this pull request Jul 23, 2020
Merged
@joakime joakime deleted the jetty-9.4.x-5057-IncludedContextPath branch August 14, 2020 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joakime joakime joakime approved these changes

@janbartel janbartel janbartel approved these changes

@lachlan-roberts lachlan-roberts lachlan-roberts approved these changes

Assignees
No one assigned
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

javax.servlet.include.context_path attribute on root context. should be empty string, but is "/"
4 participants
@gregw @joakime @janbartel @lachlan-roberts