DispatcherServlet gets wrong request handler depending on `Accept` header

[SeongEon-Jo]

Affects: springframework version 5.3.29

I developed Controller class like below.

@RestController
@RequiredArgsConstructor
public class OrderController {

    @GetMapping(value = "/orders/{no}", produces = "application/json")
    public Long getOrder(@PathVariable Long no) {
        return no;
    }

    @GetMapping(value = "/orders/title", produces = "text/plain")
    public String getOrderTitle(@RequestParam(name = "order_no") List<Long> orderNoList) {
        return "title";
    }
}

Based on this, I ran test like below.

@Test
    void test() throws Exception {
        ResultActions perform = mockMvc.perform(get("/orders/title")
                .accept("application/json")
                .param("order_no", "1, 2, 3"));

        perform.andExpect(status().isNotAcceptable());
    }

It failed because actual status was 400 bad request.

I also found a log and it said like below.

MockHttpServletRequest:
      HTTP Method = GET
      Request URI = /orders/title
       Parameters = {order_no=[1, 2, 3]}
          Headers = [Accept:"application/json"]
             Body = null
    Session Attrs = {}

Handler:
             Type = com.example.demo.controller.OrderController
           Method = com.example.demo.controller.OrderController#getOrder(Long) // note this

Async:
    Async started = false
     Async result = null

Resolved Exception:
             Type = org.springframework.web.method.annotation.MethodArgumentTypeMismatchException

...

java.lang.AssertionError: Status expected:<406> but was:<400>
Expected :406
Actual   :400

It looks weird because GET /orders/title should invoke OrderController#getOrderTitle().

Servlet seems to fail to distinguish variable argument like {no} from static argument like title in URI.

Please note that it has something to do with Accept header (relevant to produces parameter in @GetMapping) because it gets the right handler if I remove all produces parameter in @GetMapping.

Is is a bug? or am I using it in a wrong way?

[mdeinum]

Everything in the @RequestMapping as the name implies is used for mapping the request. So in this case it tries to find a handler method which can handle the /orders/title request which will return application/json. For this it will find the @GetMapping(value = "/orders/{no}", produces = "application/json") because it fits. There is no limit on the {no} and thus it will match /orders/title. Which then will fail as it cannot create a Long from the String title.

The @GetMapping(value = "/orders/title", produces = "text/plain") doesn't apply as it doesn't match the Accept header application/json.

So the @GetMapping(value = "/orders/title", produces = "text/plain") checks just 1 of the boxes (the URL), whereas the @GetMapping(value = "/orders/{no}", produces = "application/json") checks 2 boxes, URL and content-type. It will try to get the best match, which is the one with 2 boxes.

When removing the produces this will eliminate the check for the Accept header and will only match the URL. As both match it will then try to find the best matching URL, which is a direct match on the full URL instead of a pattern.

From a mapping perspective this works as designed (IMHO).

To limit the path variable pattern to only numbers you can utilize a Regular Expression which will be taken into account during mapping. Something like @GetMapping(value = "/orders/{no:\\d+}", produces = "application/json") will limit it and should give the behavior desired.

[SeongEon-Jo]

Well.. I understood what you mean. Thanks for explanation!

But then, is there any reason to check URL firstly based on a pattern, instead of direct match on the full URL?

/orders/title and /orders/{no} are totally different end point even though they are under the same pattern.

An order like below seems to be reasonable to me.

1. Check direct matched URL
2. If exists, check Accept header value
3. if does not exists, check pattern matched URL and so forth

[bclozel]

Thanks for getting in touch, but it feels like this is a question that would be better suited to Stack Overflow. As mentioned in the guidelines for contributing, we prefer to use the issue tracker only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (so that other people can find it) or add some more details if you feel this is a genuine bug.

An order like below seems to be reasonable to me.

This would be a breaking change and would make things harder to reason about. It's your responsibility to create handler mappings that are easy to disambiguate. If you want more control over the mapping process I'd suggest using the functional web framework instead.