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

MvcUriComponentsBuilder javadocs inaccurately reflects usage of forwarded headers #34615

Closed
dforrest-es opened this issue Mar 18, 2025 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: documentation A documentation task
Milestone

Comments

@dforrest-es
Copy link

In version 5.1.x, the ServletUriComponentsBuilder methods changed to remove the usage of forwarded headers:

However, the method and class documentation in MvcUriComponentsBuilder was not updated and still indicates the forwarded headers are used. (see instances of Note: in the javadocs) Yet MvcUriComponentsBuilder internally uses ServletUriComponentsBuilder for URI components resolution:

private static UriComponentsBuilder getBaseUrlToUse(@Nullable UriComponentsBuilder baseUrl) {
return baseUrl == null ?
ServletUriComponentsBuilder.fromCurrentServletMapping() :
baseUrl.cloneBuilder();
}

This affects 5.1.x -> 7.0.x.


Simple test:

@Test
void testMvcUriComponentsBuilder() {
    MockHttpServletRequest request = new MockHttpServletRequest();
    request.setServerName("localhost");
    request.setServerPort(8080);
    request.setScheme("http");

    request.addHeader("X-Forwarded-Proto", "https");
    request.addHeader("X-Forwarded-Host", "test.example.com");
    request.addHeader("X-Forwarded-Port", "443");

    RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request, new MockHttpServletResponse()));

    @Controller
    @RequestMapping("/test")
    class InnerController {}

    String expected = MvcUriComponentsBuilder.fromController(InnerController.class).toUriString();
    assertEquals(expected, "https://test.example.com");
}

Error:

expected: <http://localhost:8080/test> but was: <https://test.example.com>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 18, 2025
@bclozel bclozel added type: documentation A documentation task in: web Issues in web modules (web, webmvc, webflux, websocket) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 18, 2025
@bclozel bclozel added this to the 6.2.x milestone Mar 18, 2025
@rstoyanchev rstoyanchev self-assigned this Mar 19, 2025
@rstoyanchev
Copy link
Contributor

Thanks for pointing this out.

@rstoyanchev rstoyanchev modified the milestones: 6.2.x, 6.2.5 Mar 19, 2025
@rstoyanchev rstoyanchev added the for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x label Mar 19, 2025
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x labels Mar 19, 2025
rstoyanchev added a commit that referenced this issue Mar 19, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
related to forwarded headers

Closes gh-34615
@dforrest-es
Copy link
Author

@rstoyanchev Not a problem. Note that each method's javadocs also has this problem, not just the class level.

@rstoyanchev
Copy link
Contributor

Shame, I did a couple of text searches on the whole codebase, and somehow missed the ones in the same class! I've created #34625.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

4 participants