Skip to content
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

Equals checks to MediaType.ALL should not be affected the presence of parameters [SPR-17550] #22082

Closed
spring-issuemaster opened this issue Nov 29, 2018 · 2 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Nov 29, 2018

Raffaele Litto opened SPR-17550 and commented

I discovered this while trying to create an annotation for non-json Controllers with @RequestMapping(accept="!application/json")
When the RequestMappingInfo invokes

ProducesRequestCondition produces = this.producesCondition.getMatchingCondition(request);

the code checks if the accepted media type contains MEDIA_ALL. But in my case the header contained a quality argument:

text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8

and, after parsing, resulted in a mediatype / with quality 0.8 this failed to match with the check at the end of the ProducesRequestCondition

		else if (acceptedMediaTypes.contains(MediaType.ALL)) {
			return EMPTY_CONDITION;
		}

From what I see there are multiple places where there is an attempt to match MediaType ignoring the fact that they might have a quality on them.
Either there should be an overload of .equals and .hashcode to ignore the quality, or create an ad-hoc method to compare them without considering the quality and use it every time they check for presence inside a collection


Affects: 4.3.20, 5.1.3

Referenced from: commits 4b24bcb

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 3, 2018

Rossen Stoyanchev commented

Indeed such equals checks against MediaType.ALL are really intended to be:

mediaTypes.stream().anyMatch(type -> type.isCompatibleWith(MediaType.ALL))
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 2, 2019

Rossen Stoyanchev commented

Actually isCompatibleWith isn't quite the same thing. I've addressed this by adding a new equalsTypeAndSubtype method to MediaType.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.