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

WebFlux application server add server.forward- Headers - Strategy = Framework RouterFunction endpoint 404 #25270

Closed
oursy opened this issue Jun 17, 2020 · 7 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@oursy
Copy link

oursy commented Jun 17, 2020

There are two services:

account-service, gateway-server

After the account-service has set the 'server.forward- Headers - Strategy = Framework' property.routerFunction endpoint start 404

The following interfaces are accessed through the Spring Cloud Gateway

http://localhost:8080/account/hi is 404. The generated by RouterFunction endpoint

http://localhost:8080/account/hello return 200 normal, through RequestMapping annotations

Reproducible Example:

forward-proxy-demo.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 17, 2020
@bclozel bclozel self-assigned this Jun 17, 2020
@bclozel
Copy link
Member

bclozel commented Jun 17, 2020

Issue summary

  • a Spring Cloud Gateway app gateway-server is taking requests on port 8080, rewriting them without the /account prefix and routing them to the account-service
  • the account-service is listening for requests on port 9000 and has two endpoints - a WebFlux annotated endpoint @GetMapping("/hello") and a RouterFunction endpoint GET "/hi"

The problem here, with this setup, only the annotated one works and the functional one does not map.

Spring Cloud Gateway and Forwarded headers

First, you've configured your Spring Cloud Gateway application to strip the "/account" prefix from the request path. By doing so, the Gateway app will rewrite the request path from "/account/hello" to "/hello" but also add the "/account" prefix as a "X-Forwarded-Prefix" request header.

Since you've added the ForwardedHeaderTransformer (with the Spring Boot configuration property), you're actually asking Spring Webflux to read those Forwarded headers and transform again the incoming request, so adding back the path prefix you've just asked Gateway to remove. I believe the first issue is that your account-service application should not have that filter enabled. If you need the other forwarded headers, you should configure Spring Cloud Gateway to remove that particular prefix header.

ForwardedHeaderTransformer behavior

So at that point, we know that with this configuration your account-service is mutating requests by looking at the forwarded headers. This is what the filter is doing:

# Before the filter
exchange.getRequest().getPath() == "/hello"
exchange.getRequest().getPath().pathWithinApplication() == "/hello
exchange.getRequest().getPath().contextPath() == ""
exchange.getRequest().getHeaders().getFirst("X-Forwarded-Prefix") == "/account"

# After the filter
exchange.getRequest().getPath() == "/account/hello"
exchange.getRequest().getPath().pathWithinApplication() == "/hello
exchange.getRequest().getPath().contextPath() == "/account"
exchange.getRequest().getHeaders().getFirst("X-Forwarded-Prefix") == ""

So the filters removes the forwarded headers and takes the prefix as the context path for the request.

Mapping behavior for WebFlux annotated/functional endpoints

Now you might wonder why the WebFlux annotated controller "/hello" works and the functional endpoint "/hi" doesn't: the annotated endpoints do not take into account the context path when routing, whereas the function endpoints do; in other words, the annotated endpoints see "/hello" and the functional ones see "/account/hi.

This explains why with this setup the annotated endpoint is properly routed and the functional one is not. One could argue that this setup is flawed in the first place with the ForwardedHeaderTransformer which should not be there. If there's a particular reason for that, could you explain @oursy ?

Now I'm also wondering why there's that mapping behavior difference between annotated controllers and functional endpoints in WebFlux. Why is the context path taken into account in one case and not other? Could you help us here @rstoyanchev and @poutsma ?

@oursy
Copy link
Author

oursy commented Jun 18, 2020

@bclozel Thanks for your quick reply

One could argue that this setup is flawed in the first place with the ForwardedHeaderTransformer which should not be there. If there's a particular reason for that, could you explain

account-service integrates springdoc-openapi library to automatically generate Swagger Documentation.

Configure server.forward-headers-strategy=framework This attribute is for
I want to use Spring Cloud Gateway to unify the Swagger endpoints of services behind the aggregation gateway

The following link explains more information
https://springdoc.org/faq.html#how-can-i-deploy-springdoc-openapi-ui-behind-a-reverse-proxy

@bclozel
Copy link
Member

bclozel commented Jun 18, 2020

I see, thanks!

I'm not super familiar with the unification of APIs behind a gateway, but I'm wondering if in this case you'd want to prefix your application routes with account and avoid rewriting the prefix on the gateway.

Besides the request mapping behavior difference between annotations and functions, I don't think we can do anything about it in Spring Framework. Maybe you could ask a question on StackOverflow or create a new issue on the Spring Cloud Gateway project about that?

@oursy
Copy link
Author

oursy commented Jun 19, 2020

Besides the request mapping behavior difference between annotations and functions

This problem has stopped me now。

temporary solution is
Change function mapping to request mapping

@bclozel
Copy link
Member

bclozel commented Jul 6, 2020

Now that #25279 is solved, I've checked again with the sample application.

There is a major mapping difference between Spring WebFlux annotations and RouterFunctions mapping:

  • Annotations remove the context path when mapping, by using the org.springframework.http.server.RequestPath#pathWithinApplication
  • RouterFunction variants always use the full path since org.springframework.web.reactive.function.server.ServerRequest#pathContainer is returning a PathContainer, and not RequestPath (which extends PathContainer). The context path is not removed before mapping.

Debugging through sample applications show that the DefaultServerRequest implementation is using the underlying RequestPath, it's just not exposing it as is.

@poutsma @rstoyanchev this difference is the only remaining point here; I can see why RouterFunctions could be considered more explicit, and maybe nesting routes should be used instead here in applications. We're not calling out that difference anywhere in our documentation as far as I understand, maybe because contextPath shouldn't even be a concept in the functional variant?

I managed to reproduce this with a new test in RequestPredicatesTests:

@Test
public void pathWithContext() {
	URI uri = URI.create("https://localhost/context/path");
	RequestPredicate predicate = RequestPredicates.path("/p*");
	MockServerHttpRequest mockRequest = MockServerHttpRequest.get(uri.toString()).contextPath("/context").build();
	ServerRequest request = new DefaultServerRequest(MockServerWebExchange.from(mockRequest), emptyList());
	assertThat(predicate.test(request)).isTrue();

	mockRequest = MockServerHttpRequest.head("https://example.com").build();
	request = new DefaultServerRequest(MockServerWebExchange.from(mockRequest), Collections.emptyList());
	assertThat(predicate.test(request)).isFalse();
}

@poutsma
Copy link
Contributor

poutsma commented Jul 7, 2020

This is something that needs to be straightened out in WebFlux.fn. I will pick it up for 5.3, since it will be a breaking change for some people.

@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 7, 2020
@poutsma poutsma added this to the 5.3 M2 milestone Jul 7, 2020
@poutsma poutsma assigned poutsma and unassigned bclozel Jul 7, 2020
@rstoyanchev
Copy link
Contributor

In Spring MVC this already works as expected so we should align for consistency. The context path arguably should not be included in mappings.

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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants