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

HiddenHttpMethodFilter breaks HandlerFunction with BodyExtractors.toFormData [SPR-16551] #21094

Closed
spring-projects-issues opened this issue Mar 4, 2018 · 4 comments
Assignees
Labels
in: web type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 4, 2018

Michael Simons opened SPR-16551 and commented

I want to use org.springframework.web.filter.reactive.HiddenHttpMethodFilter. In a Spring Boot application, I can just add this as a Bean, it's recognized and works as expected. Incoming POST-requests are routed accordingly. But: I cannot access form data in all cases: As long as I use annotated controller methods taking in a ServerWebExchange and using getFormData or a Mono of a model attribute as shown here everything works well.

If I want to use a HandlerFunction however, it collides while using the form data. The HiddenMethodFilter works, but I cannot access form data any more. Using a handler function likes this

final HandlerFunction<ServerResponse> handlerFunction = req -> req
              .body(toFormData())
              .map(MultiValueMap::toSingleValueMap)
              .flatMap(formData -> ok().body(Mono.just("You posted " + formData.get("content")), String.class))
              .switchIfEmpty(ServerResponse.badRequest().build());

breaks with an java.lang.IllegalStateException: Only one connection receive subscriber allowed in the .body(toFormData())-call

19:33:36.576 [reactor-http-nio-2] ERROR org.springframework.web.server.adapter.HttpWebHandlerAdapter - Failed to handle request [POST http://localhost:8080/stuff]
java.lang.IllegalStateException: Only one connection receive subscriber allowed.
	at reactor.ipc.netty.channel.FluxReceive.startReceiver(FluxReceive.java:276)
	at reactor.ipc.netty.channel.FluxReceive.subscribe(FluxReceive.java:124)
	at reactor.core.publisher.FluxMap.subscribe(FluxMap.java:62)
	at reactor.ipc.netty.ByteBufFlux.subscribe(ByteBufFlux.java:242)

I have attached a demo project with the full code.

The test will work as soon as the hidden method filter is removed and the routing is changed to match POST or the test client uses PUT.


Affects: 5.0.4

Attachments:

Issue Links:

  • #21824 Document the need for consistent access to form data through ServerWebExchange

Referenced from: commits c563179

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2018

Rossen Stoyanchev commented

Indeed the initialization in ServerWebExchange related to getFormData ends up using the getBody method even before anyone actually tries get form data through it. The HiddenHttpMethodFilter only applies a mapping, so that doesn't count. We need to use Mono.defer either in DefaultServerWebExchange or in FormHttpMessageReader.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2018

Rossen Stoyanchev commented

So I misjudged apparently. The reason it works in annotated controllers is because that goes through ServerWebExchange.getFormData which caches the parsed form data. BodyExtractors.toFormData on the other hand reads directly from the request, so effectively both HiddenHttpMethodFilter and BodyExtractor.toFormData try to consume the body.

I'm not sure what the answer is. On one level, for a HandlerFunction that already gets the form data, it's trivial to check for the presence of a value. On the other hand, even if we accept this, since WebFluxConfigurationSupport supports annotated controllers and functional endpoints, simply configuring HiddenHttpMethodFilter for annotated controllers, would break functional endpoints.

I think there is a good argument to be made for exposing the ServerWebExchange.getFormData and getMultipartData through ServerRequest, as an extra optional layer on top of BodyExtractors. It allows repeated access to the data, parsed and cached. That's consistent with annotated controllers where you can avoid using getFormData and getMultipartData and use @RequestBody instead, for example in order to get a Flux<Part> (one at a time) instead of a Map (all parsed and cached).

Arjen Poutsma?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 8, 2018

Arjen Poutsma commented

That sounds good to me. I will add these methods next week.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 23, 2018

Arjen Poutsma commented

Fixed in c563179

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

No branches or pull requests

2 participants