Bug description
I'm having an issue with form data when using content type /. I managed to pinpoint it to this fix on Jackson Databind. What happens now when initializing form data in DefaultServerWebExchange (code) is that JSON mappers also say they can read form data (here) and parsing therefore fails (because they can't). Before this Jackson fix, ObjectMapper answered that it couldn't deserialize MultiValueMap.
Bug reproduce
Here is the simplest dummy app I've managed to build which replicates the issue: https://github.com/Dodomix/form-data-jackson-bug. The test fails with version 3.4.4 of Spring Boot, and passes with 3.4.3 (change in Jackson 2.18.2 -> 2.18.3). I'm opening the report here because the problematic code is in spring-web.
The exact behavior which changed in Jackson:
var objectMapper = new ObjectMapper();
objectMapper.canDeserialize(objectMapper.constructType(MultiValueMap.class));
Expected behavior
All tests pass.
System info
Java: Temurin 24
Comment From: bclozel
Sorry for the delayed response.
I've had a look at your sample and it seems your application does not specify which media type it can consume, and the client sets the "Content-Type" as "/". Given that situation, message codecs can only be iterated over and the first compatible one wins. It looks like Jackson previously had limited support for custom maps and now it doesn't.
I'm only seeing 3 solutions here:
* make our JSON codecs ignore MultiValueMap
on purpose. This doesn't seem fair as MultiValueMap
isn't tied to a particular media type nor technology
* reorder the codecs to consider the form one first. Again, this sounds like a brittle workaround with lots of unintended consequences
* ensure that clients send a proper content-type for content negotiation
Even if you have no control over your clients, this seems like the most sensible solution as processing such requests is bound to create lots of problems. If your application can spot such requests, maybe you can use a custom WebFilter
to mutate incoming requests and set a proper content type.
Thanks for your report!
Comment From: Dodomix
Thank you for your analysis. I would disagree with the conclusion however and I would like this to be reopened. If you look at the code for initializing the form data (link), you can see that it will try to get a reader for content type */*
, because of contentType.isCompatibleWith(MediaType.APPLICATION_FORM_URLENCODED)
, it will try to parse form data with the json reader. This is not the expected behavior.
If we compare it with multipart data initialization (link), it won't get triggered for */*
. Relevant code here is contentType.getType().equalsIgnoreCase("multipart")
.
In my opinion, the correct behavior here would be instead of
HttpMessageReader<MultiValueMap<String, String>> reader = getReader(configurer, contentType, FORM_DATA_TYPE);
to have something like:
HttpMessageReader<MultiValueMap<String, String>> reader = getReader(configurer, MediaType.APPLICATION_FORM_URLENCODED, FORM_DATA_TYPE);
To be completely clear, the problem is that the code concludes that it should parse the body as form data, and then tries to parse it as JSON. If it concludes it should parse the body as form data, it should parse it as form data.
Comment From: bclozel
Apologies for missing that and thanks for the feedback. Now I understand that the Jackson behavior change, the mention of MultiValueMap
and the regression part were red herrings. We are indeed potentially using a codec that is not suited for form data, we should fix that.
I think that the proposed change sounds reasonable. The only downside that can think of would be if an application uses a custom "application/x-www-form-urlencoded"
-like media type and expects a custom converter that supports it. This is quite unlikely in my opinion.
I'll work on a fix targeting the next 6.2.x maintenance release.