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

Regression in 4.1.5: UriComponentsBuilder.fromHttpRequest sets port to 80 i.s.o nothing/443 [SPR-12771] #17368

Closed
spring-issuemaster opened this issue Mar 1, 2015 · 11 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Mar 1, 2015

Bart Van Bos opened SPR-12771 and commented

Hi,

I am using spring-hateoas to generate URL links between my REST managed resources. Since the upstep from Spring 4.1.4 -> 4.1.5, I'm having problems with the PORT/SCHEME combination generated.

Example link before the update:

https://example.com/rest/mobile/users/1

Example link after the update:

https://example.com:80/rest/mobile/users/1

As you can see, it automatically append port 80 to the URL. The problem is that my rest back-end is running on a cloud platform heroku, which is using a reverse proxy to terminate the TLS (HTTPS).

browser --- (HTTPS/TLS on :443) --- reverse proxy --- (HTTP on :80) --- Heroku web server

This worked well before the update since the port was not automatically appended and the browser could figure out the port by negotiation. By hard coding it (by taking it form the original request), this auto detecting is not possible anymore.

These are my headers:

Header Name: host, Header Value: example.com
Header Name: connection, Header Value: close
Header Name: user-agent, Header Value: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Header Name: accept, Header Value: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Header Name: accept-language, Header Value: en-US,en;q=0.5
Header Name: accept-encoding, Header Value: gzip, deflate
Header Name: x-request-id, Header Value: d97ed0c8-c91e-492a-b609-e05290f2fb2d
Header Name: x-forwarded-for, Header Value: 84.198.58.199
Header Name: x-forwarded-proto, Header Value: https
Header Name: x-forwarded-port, Header Value: 443
Header Name: via, Header Value: 1.1 vegur

I believe the problem lies in UriComponentsBuilder.fromHttpRequest:

String portHeader = request.getHeaders().getFirst("X-Forwarded-Port");
if (StringUtils.hasText(portHeader)) {
    port = Integer.parseInt(portHeader);
}

...

builder.scheme(scheme);
builder.host(host);
if (scheme.equals("http") && port != 80 || scheme.equals("https") && port != 443) {
    builder.port(port);
}
return builder;

In my case X-Forwarded-Port == 443, so the builder.port should be set to 443 or left blank (not filled with 80 as is now the case).


Affects: 4.1.5

Reference URL: spring-projects/spring-hateoas#301

Issue Links:

  • #14743 Add support for X-Forwarded-Host in ServletUriComponentsBuilder
  • #15346 X-Forwarded-Host handling in ServletUriComponentsBuilder does not treat ports correctly
  • #17410 X-Forwarded-Port with Comma Separated List Not Able to be Parsed in Spring 4.1.x

Referenced from: commits 410fc6d, d64c48f

2 votes, 7 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 1, 2015

Bart Van Bos commented

I have tracked down the introduction of this issue to the following commit
f84c458

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 1, 2015

Bart Van Bos commented

Introduced in f84c458

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 5, 2015

Damien Coraboeuf commented

Hi,

With the following settings in an Nginx SSL reverse proxy:

proxy_set_header X-Forwarded-Host 192.168.59.104;
proxy_set_header X-Forwarded-Port 443;
proxy_set_header X-Forwarded-Proto https;

I would expect the computed URL by the UriComponentsBuilder to be https://192.168.59.104, but it is instead https://192.168.59.104:80, which obviously is the subject on this issue.

In Spring 4.1.2, no problem ; but I see this regression in 4.1.5.

I think the culprit code is this condition:

if (scheme.equals("http") && port != 80 || scheme.equals("https") && port != 443) {

It was not existing in the ServletUriComponentsBuilder code, which the code in UriComponentsBuilder.fromRequest had been copied from.

In the situation indicated above, we fall in the case where the scheme is https, the port 443, and then we do not update the port in the builder. I think the port of the builder should be set if the port is explicitly specified.

Damien.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 5, 2015

Rossen Stoyanchev commented

I think that particular line of code is fine and hasn't changed. The issue is that after refactoring the code to move it from ServletUriComponentsBuilder into the more widely available UriComponentsBuilder, the starting point includes a port value, so this line picks up the port:

UriComponentsBuilder builder = UriComponentsBuilder.fromUri(uri);

I'll try to add a fix soon to try.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 5, 2015

Rossen Stoyanchev commented

There is a fix now available in both branches. If you can please give it a try.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 5, 2015

Damien Coraboeuf commented

Great news. Have you any idea when the 4.1.6 would be delivered? Just to know if I design the workaround in the meantime or if I just wait. Thanks.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 6, 2015

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 6, 2015

Damien Coraboeuf commented

Thanks. I should have known...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 29, 2015

Bart Van Bos commented

Verified as fixed in latest 4.1.6.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 3, 2015

Elnur Abdurrakhimov commented

This hasn't been fixed. I'm still running into it with Spring Boot 1.2 and Spring 4.1.6. Had to revert to Spring Boot 1.1 and Spring 4.0.9 to make my API work again. Please reopen the ticket.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 3, 2015

Elnur Abdurrakhimov commented

I figured the cause. It doesn't work without setting the header X-Forwarded-Port to 443. It did work without this header in Spring 4.0.x though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.