Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-15831

MockMvc doesn't honor params in RequestMapping when no equal sign

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Complete
    • Affects Version/s: 4.3.10, 5.0 RC3
    • Fix Version/s: 5.0 RC4
    • Component/s: Core
    • Labels:
      None
    • Last commented by a User:
      false

      Description

      When two resource methods are defined as following:

          @GetMapping
          public String noParam() {
              return "noParam";
          }
       
          @GetMapping(params = "foo")
          public String paramFoo() {
              return "paramFoo";
          }
      

      and a MockMvc test tries executing the second method using

          mvc.perform(get("/api/bug?foo"))
      

      then the first method is invoked instead. Adding an equal sign at the end fixes the issue, but the equl sign shouldn't be necessary. BTW, in production, sending a request to /api/bug?foo does invoke the second method as expected.

      Here's a repro project: https://github.com/jnizet/param-bug-demo

      Note that the original bug has been detected in a Spring Boot 2.0.0.M2 project, so the bug is still there in the latest version of Spring.

        Activity

        Hide
        rstoya05-aop Rossen Stoyanchev added a comment -

        It looks like Servlet containers (at least current Tomcat and Jetty) do not differentiate between empty value ("foo=") and no value ("foo") and always return an empty value.

        In the Spring Framework however we do and always have. In this case UriComponentsBuilder parses "foo" as a parameter with a null value, MockHttpServletRequestBuilder preserves that distinction, MockHttpServletRequest stores it as a String[] with one null value, and at request time ParamsRequestCondition calls WebUtils.hasSubmitParameter(request, name) which does request.getParameter(name) != null and that is null.

        I am not sure whether the Servlet container behavior has changed over time or this simply hasn't surfaced until now. I could not find anything the Servlet spec and API that talks about empty parameter values.

        We could make an improvement in ParamsRequestCondition to perform the name-only match via request.getParameterMap().contains(name). One could argue that WebUtils.hasSubmitParameter is fine as it is since a submit parameter is a more specific use case for a browser form button.

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - It looks like Servlet containers (at least current Tomcat and Jetty) do not differentiate between empty value ("foo=") and no value ("foo") and always return an empty value. In the Spring Framework however we do and always have. In this case UriComponentsBuilder parses "foo" as a parameter with a null value, MockHttpServletRequestBuilder preserves that distinction, MockHttpServletRequest stores it as a String[] with one null value, and at request time ParamsRequestCondition calls WebUtils.hasSubmitParameter(request, name) which does request.getParameter(name) != null and that is null. I am not sure whether the Servlet container behavior has changed over time or this simply hasn't surfaced until now. I could not find anything the Servlet spec and API that talks about empty parameter values. We could make an improvement in ParamsRequestCondition to perform the name-only match via request.getParameterMap().contains(name) . One could argue that WebUtils.hasSubmitParameter is fine as it is since a submit parameter is a more specific use case for a browser form button.

          People

          • Assignee:
            rstoya05-aop Rossen Stoyanchev
            Reporter:
            jnizet Jean-Baptiste Nizet
            Last updater:
            St├ęphane Nicoll
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              16 weeks, 1 day ago