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

Support for "request parameters" (query string + form data) in ServerWebExchange [SPR-15000] #19567

Closed
spring-projects-issues opened this issue Dec 9, 2016 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Dec 9, 2016

Daniel Fernández opened SPR-15000 and commented

Scenario

A web application using Spring 5.0.0.BUILD-SNAPSHOT, Spring Boot 2.0.0.BUILD-SNAPSHOT and Spring Web Reactive provides an HTML form that executes different @Controller methods when submitted depending on the presence of specific parameters (managed by means of the params property at @RequestMapping).

For instance:

<form action="/doit" method="post">
   ...
   <button type="submit" name="thingone">Do thing one</button>
   <button type="submit" name="thingtwo">Do thing two</button>
 </form>

And then at the @Controller:

    @RequestMapping(value="/doit")
    public String doThingNone(final FormBean seedStarter, final BindingResult bindingResult) {
        ...
    }

    @RequestMapping(value="/doit", params={"thingone"})
    public String doThingOne(final FormBean seedStarter, final BindingResult bindingResult) {
        ...
    }
    
    @RequestMapping(value="/doit", params={"thingtwo"})
    public String doThingTwo(final FormBean seedStarter, final BindingResult bindingResult) {
        ...
    }

Observed results

-Submitting a form like this form will result in HTTP 400.-
The controller method that is selected for execution is the wrong one, doThingNone(...), as if the params attribute in other @RequestMapping methods didn't apply)

Example repository

I'm creating a Spring Web Reactive-enabled version of Thymeleaf's STSM example application at the thymeleaf/thymeleafexamples-stsm-reactive repository on GitHub: https://github.com/thymeleaf/thymeleafexamples-stsm-reactive/tree/SPR-15000 (tag: SPR-15000)

It can be executed with mvn -U clean compile spring-boot:run and accessed pointing a browser to http://localhost:8080

Once there, clicking on any of the Add row or Add Seed Starter buttons will provoke -an HTTP 400- the wrong controller selection behaviour.


Affects: 5.0 M3

Reference URL: https://github.com/thymeleaf/thymeleafexamples-stsm-reactive/tree/SPR-15000

Issue Links:

Referenced from: commits 6119415

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 9, 2016

Rossen Stoyanchev commented

I'm not getting a 400. Instead the request gets to showSeedstarters method that is mapped to the same URL "/seedstartermng" but without params. I'm not sure why I get a different behavior but I do see one potential explanation.

In the Servlet API "request parameters" is both query params and form data and there is no way to distinguish one from the other. On the reactive side there is a getFormData method on ServerWebExchange and getQueryParams on ServerHttpRequest so they're distinct options. For data binding we combine both but the params condition only matches to query params and the same is true for @RequestParam. I guess we need to decide on the semantics we want.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 9, 2016

Daniel Fernández commented

Apparently, the reason I'm getting an HTTP 400 and you aren't is related to our difference in time zones. The first field in the form of the example application is a Date, and given the DateFormatter is not being applied because of https://github.com/bclozel/spring-boot-web-reactive/issues/55, we both see the result of executing Date#toString().

The interesting part is, submitting the form with that Date as it is displayed for me (with time zone CET) provokes some kind of binding error that results in the HTTP 400. However, if I simply replace at that input the word CET with EST (your time zone), it works and I get your same behaviour.

I still need to investigate further in order to know if this is a new bug or instead something provoked by me somehow, but in the meantime I modified the title and description of this ticket in order to properly focus on the @RequestMapping(params) thing...

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 11, 2016

Daniel Fernández commented

Okay, I finally found the reason for my HTTP 400, and it is indirectly related to the fact that the wrong controller method is being selected.

The fact is that when my form was sent as Fri Dec 9 18:05:07 CET 2016, the form binding operation resulted in a TypeMismatchException. This was because, not being my DateFormatter registered as explained in my last comment, a default ObjectToObjectConverter was being used, which was executing the java.util.Date(String) constructor. This constructor, which was deprecated just before the dinosaurs were wiped out, uses a very basic –and 90% wrong– way of parsing String objects into Date ones, which allows the time zones in the United States to be present in the String, and also GMT and UTC. But no luck for continental Europeans like me living in CET :)

So I was getting a TypeMismatchException while you –in EST– weren't. But now, why did I get an HTTP 400 for that?

Well, that was because given the issue explained in this ticket, the wrong @RequestMapping method was being selected for execution, specifically the one with no params attribute. Which looks like this:

@RequestMapping({"/","/seedstartermng"})
public String showSeedstarters(final SeedStarter seedStarter) {
    seedStarter.setDatePlanted(Calendar.getInstance().getTime());
    return "seedstartermng";
}

And note there is no final BindingResult bindingResult argument following that final SeedStarter seedStarter because that controller method was not meant to receive form submissions. So Spring was inferring that I had no interest in being notified of binding errors and therefore all it had to do was to send an HTTP 400 error. I verified that, adding such BindingResult argument that controller method gets executed –even if it is the wrong one–, and bindingresult.hasErrors() returns true as it should.

Ea, mistery solved :)

As a last comment, regarding the semantics of the params attribute in @RequestMapping and the @RequestParam annotation: from my point of view, it would be great if you could make these behave the same in Web MVC and Web Reactive. Having these behave differently in the MVC and Reactive sides would perhaps be a bit surprising for me as a dev...

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 15, 2016

Rossen Stoyanchev commented

The params condition and @RequestParam now consider both query params and form data.

I used the sample application to ensure it adds and removes rows. I saw error messages appear in the UI after there is at least one row. Also while removing there are exceptions in the controller method. In both cases I think those errors are sample-related. In any case the params condition routes to the expected controller method.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 16, 2016

Daniel Fernández commented

I can confirm it works for me too, thanks a lot!

Those errors in the example app are normal now because the needed Formatter objects (for dates and varieties) have not been auto-registered (see https://github.com/bclozel/spring-boot-web-reactive/issues/55).

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 16, 2016

Rossen Stoyanchev commented

And thanks for the report, as well as many others!

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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants