Skip to content

Default TCP port included in ControllerLinkBuilder - HTTP:80 vs HTTS:443 #301

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
boeboe opened this issue Feb 23, 2015 · 10 comments
Closed

Comments

@boeboe
Copy link

boeboe commented Feb 23, 2015

Hi,

Last weekend (feb 20), Spring 4.1.5 was release. After fetching this new version I've got an issue with the links generated by Spring-hateoas.

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.

Does someone has an idea on how to get around this problem? Is there a way to configure the URL building strategy (port appending on default 80/443 ports in particular) globally in spring-hateoas?

Best regards,
Bart

PS: when I deploy my REST back-end locally I use localhost:9090, which worked and still works well. So in some cases I need the TCP port to be appended, but in case of 80/443 http/https I'd like the port to be left out of the URL.

@odrotbohm
Copy link
Member

So isn't that rather something you'd want to report in the Spring Framework JIRA? We're basically using ServletUriComponentsBuilder as is.

I can see some changes made in the release for SPR-12723 but I can't quite see why the changes @jhoeller would cause a change in behavior. Looking at your example it seems that the request contains an explicit port 80 despite the fact its on HTTPS. According to the code in SUCB this will always have lead to the port explicitly added as it's not equal to 443.

We apply some custom handling for X-Forwarded headers, that might mess things up here. What do your request headers look like?

@boeboe
Copy link
Author

boeboe commented Feb 27, 2015

This 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
Header Name: connect-time, Header Value: 2
Header Name: x-request-start, Header Value: 1425043521547
Header Name: total-route-time, Header Value: 0

The output from ServletUriComponentsBuilder:

https://example.com:80/rest/mobile/test/header

@odrotbohm
Copy link
Member

That looks weird. Care to create a test case that shows the lookup through ServerUriComponentsBuilder failing?

@jiwhiz
Copy link

jiwhiz commented Feb 27, 2015

@olivergierke I have a running example for you. Just upgraded to Spring Boot 1.2.2 and my website is not working any more. If you open https://www.jiwhiz.com by Chrome, and see the network traffic in Developer Tools, you can find the first api call to https://www.jiwhiz.com/api/public is 200 OK, but all hyperlinks in the response will have port 80, such as

{
  "authenticated": false,
  "_links": {
    "self": {
      "href": "https://www.jiwhiz.com:80/api/public"
    },
    "blogs": {
      "href": "https://www.jiwhiz.com:80/api/public/blogs{?page,size,sort}",
      "templated": true
    },
    "blog": {
      "href": "https://www.jiwhiz.com:80/api/public/blogs/{blogId}",
      "templated": true
    },
    "currentUser": {
      "href": "https://www.jiwhiz.com:80/api/currentUser"
    },
    "latestBlog": {
      "href": "https://www.jiwhiz.com:80/api/public/latestBlog"
    },
    "recentBlogs": {
      "href": "https://www.jiwhiz.com:80/api/public/recentBlogs"
    },
    "recentComments": {
      "href": "https://www.jiwhiz.com:80/api/public/recentComments"
    },
    "tagCloud": {
      "href": "https://www.jiwhiz.com:80/api/public/tagClouds"
    },
    "profile": {
      "href": "https://www.jiwhiz.com:80/api/public/profiles/{userId}",
      "templated": true
    },
    "postComment": {
      "href": "https://www.jiwhiz.com:80/api/user/blogs/{blogId}/comments",
      "templated": true
    }
  }
}

My application is deployed to PWS and I'm using CloudFlare as proxy for SSL. You can see all the source code at https://github.com/jiwhiz/JiwhizBlogWeb

Appreciate if you can help me fix this issue. Otherwise I will revert back to Spring Boot 1.2.1.RELEASE.

@boeboe
Copy link
Author

boeboe commented Mar 1, 2015

I believe the problem lies in UriComponentsBuilder.fromHttpRequest:

public static UriComponentsBuilder fromHttpRequest(HttpRequest request) {
    URI uri = request.getURI();
    UriComponentsBuilder builder = UriComponentsBuilder.fromUri(uri);

    String scheme = uri.getScheme();
    String host = uri.getHost();
    int port = uri.getPort();

    String hostHeader = request.getHeaders().getFirst("X-Forwarded-Host");
    if (StringUtils.hasText(hostHeader)) {
        String[] hosts = StringUtils.commaDelimitedListToStringArray(hostHeader);
        String hostToUse = hosts[0];
        if (hostToUse.contains(":")) {
            String[] hostAndPort = StringUtils.split(hostToUse, ":");
            host  = hostAndPort[0];
            port = Integer.parseInt(hostAndPort[1]);
        }
        else {
            host = hostToUse;
            port = -1;
        }
    }

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

    String protocolHeader = request.getHeaders().getFirst("X-Forwarded-Proto");
    if (StringUtils.hasText(protocolHeader)) {
        scheme = protocolHeader;
    }

    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" equals 443, hence is assigned to variable port. The builder never sets the port however because scheme == https && port == 443 (not !443).

I have reported a bug in Spring project

https://jira.spring.io/browse/SPR-12771

The commit that broke this use case.

@toedter
Copy link

toedter commented Mar 29, 2015

Thanks for reporting this, I have exactly the same behavior in my spring boot app (deployed to heroku).

@boeboe
Copy link
Author

boeboe commented Mar 29, 2015

Fixed in latest Spring 4.1.6 RELEASE.

@elnur
Copy link

elnur commented Jul 4, 2015

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.

@odrotbohm
Copy link
Member

There's not much we can do about this here as indicated as it's been a bug in Spring Framework accidentally adding this. Do you have a small sample app somewhere to show that the issue persists?

@elnur
Copy link

elnur commented Jul 4, 2015

I figured the reason and posted it to Spring's JIRA. Turns out you have to pass X-Forwarded-Port which wasn't required before and hence it's a regression. But adding the header fixed the problem for me. Thanks.

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

No branches or pull requests

5 participants