Skip to content

@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

Closed
spring-projects-issues opened this issue Aug 2, 2016 · 7 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

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.

@RestController
@RequestMapping(consumes = MediaType.APPLICATION_JSON_UTF8_VALUE)
public class FooController {
...

Affects: 4.2.6

@spring-projects-issues
Copy link
Collaborator Author

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 consumes=APPLICATION_JSON_UTF8_VALUE in the first place.

@spring-projects-issues
Copy link
Collaborator Author

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.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

GET could have body but the server could ignore it (https://tools.ietf.org/html/rfc2616#section-9.3)

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 {};
}

@spring-projects-issues
Copy link
Collaborator Author

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.

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
@Blacklands
Copy link

Blacklands commented Sep 28, 2020

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 consumes restriction.

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 Content-Type header to them.

@rstoyanchev
Copy link
Contributor

@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.

@Blacklands
Copy link

Blacklands commented Oct 15, 2020

@rstoyanchev Sorry for not mentioning my version - I'm using Spring Boot 2.3.3, which seems to pull in Spring Web 5.2.8.
This still doesn't work for me, so I assume I misunderstand the issue?

If I have a @RestController method annotated with e.g.

@GetMapping(consumes = MediaType.APPLICATION_JSON_VALUE)

and then try to send a GET request without a Content-Type header to that endpoint, I get

[org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver]: Resolved [org.springframework.web.HttpMediaTypeNotSupportedException: Content type '' not supported]

Was this change only for cases where there is also a @RequestBody(required = false) annotation on the method?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants