Skip to content

Conversation

@frankbille
Copy link
Contributor

If we don't and we also use a port in the proxyTo address, we will end up with a host:port:port address causing an exception.

…header.

If we don't and we also use a port in the proxyTo address, we will end up with a host:port:port address causing an exception.
@Laures
Copy link
Contributor

Laures commented Jun 26, 2013

can somebody explain why spring-hateoas cares for the x-forwarded header at all? shouldn't this be handled by the servlet container?

@frankbille
Copy link
Contributor Author

Not really, this seems like a hack, but it caused a problem for me. If it can be simplified and have the servlet container handle it it would be better for me. I just don't have the overview of it is possible.

@Laures
Copy link
Contributor

Laures commented Jun 26, 2013

i can give you an example for jetty. if you set forwarded=true on the configured http-connector jetty will set the x-forwarded-proto header value in the current httpservlet request. for tomcat there is the RemoteIpValve that does something similar.

as a framework i would expect the current servlet request to contain the uri/port as it is seen by the client.

EDIT: of cause both configs only have an effect when the header is set

@odrotbohm
Copy link
Member

Thanks for the patch. I've filed SPR-10718 to get this fixed in the framework going forward. To apply the fix against Spring HATEOAS, would you mind singing the CLA and report the reference number back?

odrotbohm pushed a commit that referenced this pull request Jul 3, 2013
ControllerLinkBuilder now correctly uses the X-Fowarded-Host header by inspecting it for a port being set and configuring the discovered one on the ServletUriComponentsBuilder created.

Also added that the first host listed in the header is used.
@frankbille
Copy link
Contributor Author

No problem. I agree with Laures, that it is annoying that we have to consider the proxy in front of the server, but f.ex. on Google App Engine, it doesn't handle it for us. So either the core Spring framework or it's modules has to.

I signed the CLA, and got this confirmation number: 58420130704035452

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