-
Notifications
You must be signed in to change notification settings - Fork 38.5k
@RequestMapping consumes type annotation filters out "GET" requests without Content-Type [SPR-14556] #19124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Rossen Stoyanchev commented Even though it is impractical, technically the HTTP spec does allow GET to have a body. I can see what you're aiming for but type-level conditions are inherited and I don't think we can guess even in this case what the intent is. In 4.3 we added support for HTTP method specific, composed annotations. Previously specifying a consumes condition meant a very long mapping. It's now much more concise and arguably it's better to have consumes only where it applies: @PostMapping(path="/foo", consumes=APPLICATION_JSON_UTF8_VALUE) Another point is that consumes and produces aren't meant to appear everywhere. Think of them as deal breakers when you actually have more than one method mapped on the same URL but each expecting a different content type and doing something different depending on that. So I would check if you even need |
Victor Herraiz Posada commented We do need consumes, there is another controller that expect a different content type for the same endpoints. As you said, GET could have body but the server could ignore it (https://tools.ietf.org/html/rfc2616#section-9.3) and only process the request URI. Specs are not clear at that point. What we trying to implement is a controller that reads and writes only with JSON. But current implementation force us to annotate every method. Another solution could be implement MediaType.NONE or something similar that match requests without body and add that to "consumes" array. This should match also PUT request without body. |
Rossen Stoyanchev commented
Right but as a framework we can't assume that. The application decides. Since we are talking about avoiding duplication in the mappings and you have a specific scenario with JSON-only vs other controller for the same URLs then I think the best solution is to create your own custom annotation. They're trivial to create, will give you a concise annotation to use, and this is exactly their purpose. For example: @JsonPostMapping(path="/foo") where JsonPostMapping is declared like this (you could add other attributes, see PostMapping for example): @Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@Documented
@RequestMapping(method = RequestMethod.POST, consumes=APPLICATION_JSON_UTF8_VALUE)
public @interface JsonPostMapping {
@AliasFor(annotation = RequestMapping.class)
String[] value() default {};
@AliasFor(annotation = RequestMapping.class)
String[] path() default {};
} |
Victor Herraiz Posada commented That solution is great. We have some constrains on the spring version at the moment, but we will use it for sure. Anyway, it could be interesting to have something that filter out requests with body. "MediaType.NONE" or something with better naming should be consider in another requirement in my opinion. Thank you for your comments. |
I just ran into the same problem and was very confused, until I found this issue here. I was wondering if it would be possible to add an option to the framework (disabled by default) for a "strict" handing of GET requests - when enabled, they are assumed to not have a body, and are thus not subject to to the I understand that it's technically allowed and so Spring has to support it, but I'd argue that it's enough of an edge case that an option to be more strict about it would make sense. The overwhelming majority of cases is probably assuming GET requests have no body, and thus also not adding a |
@Blacklands I don't know what version you're on but there was a change in 5.2 for the consumes condition to match if there is no body, see #22010. |
@rstoyanchev Sorry for not mentioning my version - I'm using If I have a
and then try to send a
Was this change only for cases where there is also a |
Victor Herraiz Posada opened SPR-14556 and commented
"Get" requests should not use content type header, therefore they should not be filter by the consumes property (see example below).
The type annotation is quite useful for to add a "general" constrain to any POST o PUT, but it could not be use if there is any GET method in the same controller.
Affects: 4.2.6
The text was updated successfully, but these errors were encountered: