Fix incorrect HttpHeaders handling in RequestHeaderParameterProcessor #1300
Conversation
spencergibb
added
bug
| MethodMetadata data = context.getMethodMetadata(); | ||
|
|
||
| if (Map.class.isAssignableFrom(parameterType)) { | ||
| if (Map.class.isAssignableFrom(parameterType) || HttpHeaders.class.isAssignableFrom(parameterType)) { |
There was a problem hiding this comment.
I don't think we then need the Map.class.isAssignableFrom(parameterType) check here anymore then since from this point forward we are only compatible with Spring Boot 4 so no need for backward compatibility
There was a problem hiding this comment.
The Map.class.isAssignableFrom(parameterType) check is necessary for backward compatibility and to support MultiValueMap usage.
There was a problem hiding this comment.
But we don't need to support Boot versions prior to 4.0.x on the main branch, the 4.3.x branch and the 4.2.x branches of Spring Cloud OpenFeign will support the prior Boot versions. That is why I think we can remove the check for Map.
There was a problem hiding this comment.
If you think it’s safe to remove the check, I’m open to it. That said, I’d prefer to keep it — a lot of tests fail when it’s removed, and users may (or probably) rely on MultiValueMap as a workaround instead of HttpHeaders. Keeping it feels like the safer option to avoid subtle breakages. What do you think?
|
Please sign your commit(s) so the DCO check passes as well |
Signed-off-by: Alex <invokevirtual@icloud.com>
4 participants
In Spring 4, HttpHeaders no longer implements the Map interface, which makes the comparison in RequestHeaderParameterProcessor invalid. This PR addresses this issue.