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

Parameter values are null when making a PUT request [SPR-15828] #20383

Closed
spring-issuemaster opened this issue Jul 27, 2017 · 10 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Jul 27, 2017

Petar Tahchiev opened SPR-15828 and commented

Hello,

here's the setup. I have a controller which is mapped to PUT request method. Spring-boot configures HttpPutFormContentFilter and in 4.3.8 the method in HttpPutFormContentRequestWrapper:getParameterValues looks like this:

String[] queryStringValues = super.getParameterValues(name);
List<String> formValues = this.formParameters.get(name);
if (formValues == null) {
     return queryStringValues;
}
        //more

So because super.getParameterValues(name) returns correct value and the formParameters.get(name) returns null, the the correct value from queryStringValues is returned.

Now in 4.3.9 this method looks like this:

String[] queryParam = (super.getQueryString() != null ? super.getParameterValues(name) : null);
List<String> formParam = this.formParameters.get(name);
if (formParam == null) {
     return queryParam;
}
        // more 

Now because the super.getQueryString() is null and the formParam is null too then null is returned.

Why has this been changed? Do I need to change my forms too? I was assuming my code should work in both 4.3.8 and 4.3.9


Affects: 4.3.10, 5.0 RC3

Attachments:

Issue Links:

  • #20475 Spring MVC : ModelAttribute not being populated on a PUT request ("is duplicated by")
  • #20390 Access to request parameters via @RequestParam within form PUT request handlers is broken (HttpPutFormContentFilter) ("is duplicated by")
  • #20308 MockMvc duplicates PUT Parameter value

Referenced from: commits 3524849, af83d23

0 votes, 5 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Juergen Hoeller commented

This looks like a side effect of #20308; however, that's a 4.3.10-only change. Are you sure this happens with 4.3.9 as well? In any case, let's sort this out for 4.3.11, one way or the other.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 28, 2017

Petar Tahchiev commented

Hi Juergen,
yes you are right - my bad. Spring-platform-bom BRUSSELS-S3 uses 4.3.9 (works fine) and spring-platform-bom BRUSSELS-BUILD-SNAPSHOT uses 4.3.10 (fails).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 28, 2017

Rossen Stoyanchev commented

I presume this is when the application is running? In other words just double checking it's not a MockMvc test.

The intent of super.getParameterValues(name) is to get request parameters that did not come from the body which the filter has parsed. The main other source is the query and hence the name of the local variable. The extra check for super.queryString() != null) was added due to #20308 and wasn't meant to change anything but apparently it has side effects. Can you confirm what is the source of the request parameter returned from the call to super.getParmaterValues()? If not the body and not the query string, where is it coming from actually?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 29, 2017

Petar Tahchiev commented

Yes, this happens when the application is running and I submit a PUT form.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 29, 2017

Petar Tahchiev commented

I stopped with breakpoint in HttpPutFormContentRequestWrapper:getParameterValues and super.getParameterValues(name) returns correct value. But this value is only going to be used when the super.getQueryString() != null - and in my case super.getQueryString() is null

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 31, 2017

Rossen Stoyanchev commented

Request parameters are a Servlet API concept (no such thing in HTTP). The are sourced from URI query parameters, from form data in the request body, or parts in the body of a multipart form request. So I am first of all wondering what is the source of correct value, i.e. what does it correspond to in the actual HTTP request that came in? Or does it not correspond to anything and it was added later, e.g. possibly by decorating the request?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 31, 2017

Jonas Havers commented

The values are contained in the request parameters, just like defined in javax.servlet.ServletRequest#getParameterMap(), and can be used, like for normal POST requests.

In the HttpPutFormContentFilter, the HttpInputMessage is a ServletServerHttpRequest with an overridden getBody method which is then passed to the FormHttpMessageConverter. The converter does not convert any values because the inputMessage body is empty and it tries to extract the parameters from the body.

I do not know the reason why the "getBody" method is overridden. In the ServletServerHttpRequest, there is the following conditional implementation:

   @Override
public InputStream getBody() throws IOException {
     if (isFormPost(this.servletRequest)) {
          return getBodyFromServletRequestParameters(this.servletRequest);
     }
     else {
          return this.servletRequest.getInputStream();
     }
}

ServletServerHttpRequest.getBodyFromServletRequestParameters(request)) would work just fine for our PUT requests if the implementation above was not restricted to POST requests. Because the request already passed the HiddenHttpMethodFilter, it is currently wrapped as a "PUT request".

The request wrappers at HttpPutFormContentRequestWrapper#getParameterValues are as follows:

  • org.springframework.web.filter.HiddenHttpMethodFilter.HttpMethodRequestWrapper
  • org.apache.catalina.connector.RequestFacade
  • org.apache.catalina.connector.Request
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 31, 2017

Rossen Stoyanchev commented

Yes based on #20390 one possible explanation for this ticket here is that actual underlying request method is HTTP POST and the body would have already been parsed by the Servlet container. So the assumption that super.getParameterValues() has to return query parameters isn't always true.

Petar Tahchiev can you confirm that your case is a request coming from a browser where the form has the <input type="hidden" name="_method" value="PUT"> and server runs with HiddenHttpMethodFilter and HttpPutFormContentFilter?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 4, 2017

Vedran Pavic commented

We were also affected by this, however while updating from 5.0.0.RC2 to 5.0.0.RC3.

can you confirm that your case is a request coming from a browser where the form has the <input type="hidden" name="_method" value="PUT"> and server runs with HiddenHttpMethodFilter and HttpPutFormContentFilter?

This has been our case - it's a Spring Boot application so both HiddenHttpMethodFilter and HttpPutFormContentFilter are automatically registered while hidden method input is rendered by Thymeleaf.

I've tested the 5.0.0.BUILD-SNAPSHOT and can confirm that it restores the original/expected behavior.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 14, 2017

Rossen Stoyanchev commented

Thanks for checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.