Skip to content

Validate server.servlet.path does not contain unusual patterns (eg /*) #13292

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

Closed
candrews opened this issue May 29, 2018 · 8 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@candrews
Copy link
Contributor

candrews commented May 29, 2018

In application.properties:

server.servlet.path=/*
EndpointRequest.to(HealthEndpoint.class).matches(request)

Make a request for /actuator/health

I'd expect for the matcher to match, but it doesn't. Looking inside EndpointRequest, the delegate field has a OrRequestMatcher instance composed of an AntPathRequestMatcher with pattern set to /*/actuator/health/** - the pattern should be /actuator/health/**

@candrews
Copy link
Contributor Author

This issue is related to #12934

@candrews candrews changed the title EndpointRequest creates a matcher starting with /*; doesn't match requests EndpointRequest doesn't work correctly when server.servlet.path=/* May 29, 2018
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 29, 2018
@candrews
Copy link
Contributor Author

The RequestMatcherFactory created in org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest.AbstractRequestMatcher.createDelegate(WebApplicationContext) has servletPath=/* (as expected) but it isn't handled correctly in org.springframework.boot.actuate.autoconfigure.security.servlet.EndpointRequest.RequestMatcherFactory.antPath(String...)

looks like a bit more work like 469372c is necessary

@philwebb
Copy link
Member

@candrews The server.servlet.path property is for the main path of the dispatcher servlet. Using /* doesn't seem valid to me. What path are you actually trying to map your dispatcher servlet to?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label May 29, 2018
@candrews
Copy link
Contributor Author

I'm not sure why my application has server.servlet.path=/* set - the history isn't telling me. However, google shows me a few pages instructing users to use this configuration for JBoss. https://victal.eti.br/deploying-springboot-applications-on-jboss-7.html is one such example. So I think I'm probably not the only person with this configuration.

Setting server.servlet.path=/ (which is the default) doesn't seem to break anything. So if you do nothing with this issue, it won't bother me.

If server.servlet.path=/* really is invalid, perhaps Spring Boot can have a validator on that field raising an error if this invalid value is provided?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 29, 2018
@philwebb philwebb changed the title EndpointRequest doesn't work correctly when server.servlet.path=/* Validate server.servlet.path does not contain unusal patterns (eg /*) May 29, 2018
@philwebb philwebb added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels May 29, 2018
@philwebb philwebb added this to the Backlog milestone May 29, 2018
@philwebb
Copy link
Member

Good idea, I'll re-purpose this issue to add validation.

@mbhave
Copy link
Contributor

mbhave commented Sep 6, 2018

We have some tests that rely on this behavior at the moment. I'm not quite sure what we want to allow for spring.mvc.servlet.path.

@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Sep 11, 2018
@dsyer dsyer changed the title Validate server.servlet.path does not contain unusal patterns (eg /*) Validate server.servlet.path does not contain unusual patterns (eg /*) Sep 12, 2018
@mbhave mbhave removed the for: team-attention An issue we'd like other members of the team to review label Sep 13, 2018
@philwebb philwebb self-assigned this Oct 4, 2018
@snicoll snicoll modified the milestones: 2.1.x, 2.1.0.RC1 Oct 5, 2018
@mbhave
Copy link
Contributor

mbhave commented Oct 5, 2018

* is a valid character in urls so maybe we should make this check less restrictive. Also, when determining url mappings, it seems like this should be getPath().endsWith("/*") so that a servlet path of /foo*bar will be mapped to foo*bar/*. Reopening the issue to see what the rest of the team thinks.

@mbhave mbhave reopened this Oct 5, 2018
@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Oct 5, 2018
@philwebb
Copy link
Member

We discussed this on the call and decided to keep it as strict as possible until someone complains that they really need *.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants