Skip to content

ForwardedHeader should accept multiple elements #712

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
schabe77 opened this issue May 16, 2018 · 3 comments
Closed

ForwardedHeader should accept multiple elements #712

schabe77 opened this issue May 16, 2018 · 3 comments

Comments

@schabe77
Copy link

schabe77 commented May 16, 2018

Hello,

https://tools.ietf.org/html/rfc7239 states that it's possible to have more than one element in the "Forwarded"-Header.
Each elements can consist of multiple field-values, separated by semicolon.
Elements are seperated by comma (see examples at https://tools.ietf.org/html/rfc7239#section-4).

But org.springframework.hateoas.mvc.ForwardedHeader.of(String) only splits by semicolon.
That's why parsing Headers like
ForwardedHeader.of("for=192.0.2.43, for=198.51.100.17");
fails with
java.lang.IllegalArgumentException: At least one forwarded element needs to be present!
at org.springframework.util.Assert.isTrue(Assert.java:116)
at org.springframework.hateoas.mvc.ForwardedHeader.of(ForwardedHeader.java:68)

Multiple Forward-Elements are used by opera mini, for example. Opera users may use opera servers as proxy to reduce data usage. These Proxies add a Forward-Header with multiple Values.

@mle-enso
Copy link

mle-enso commented May 16, 2018

I can confirm this behaviour:

This one works (in conjunction with a dummy controller at path "/dummy"):

    @Test
    public void buildUrlWithOneForInForwardedHeader() {
        // given
        MockHttpServletRequest request = new MockHttpServletRequest();
        request.addHeader("Forwarded", "for=111.0.0.0; by=host=example.com");
        RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request));

        // when
        String url = linkTo(methodOn(DummyController.class).getDummy()).withSelfRel().getHref();

        // then
        assertThat(url).isEqualTo("http://example.com/dummy");
    }

…and this one doesn't (throws the exception mentioned above)…

    @Test
    public void buildUrlWithTwoForInForwardedHeader() {
        // given
        MockHttpServletRequest request = new MockHttpServletRequest();
        request.addHeader("Forwarded", "for=111.0.0.0, for=222.0.0.0; by=host=example.com");
        RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request));

        // when
        String url = linkTo(methodOn(DummyController.class).getDummy()).withSelfRel().getHref();

        // then
        assertThat(url).isEqualTo("http://example.com/dummy");
    }

@gregturn
Copy link
Contributor

I appreciate you investigating this along with finding the spec reference. I'll keep that in mind as we look to properly integrate with Spring MVC's updates in 5.1 regarding Forwarded header handling. It's quite possible we will be able to delegate completely, making this a moot point. We'll see.

@gregturn
Copy link
Contributor

In light of #713, I'm closing this issue.

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

No branches or pull requests

3 participants