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?
Comment From: 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.
Comment From: 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.
- Check direct matched URL
- If exists, check Accept header value
- if does not exists, check pattern matched URL and so forth
Comment From: 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.