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

Media type matching is potentially too restrictive #106

Open
tonicsoft opened this issue Jul 18, 2018 · 2 comments
Open

Media type matching is potentially too restrictive #106

tonicsoft opened this issue Jul 18, 2018 · 2 comments
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed

Comments

@tonicsoft
Copy link

tonicsoft commented Jul 18, 2018

Suppose an HTTP client submits a request to the server with header "Accept: /" or even no accept header at all. Clearly, a json response is acceptable here. In both cases jersey will set the MediaType of the request to "/" (in the absense of any @produces annotation on the resource method).

The code eventually comes to jackson-jaxrs-json-provider-2.9.6-sources.jar:com.fasterxml.jackson.jaxrs.json.JacksonJsonProvider:hasMatchingMediaType which returns false for MediaType.WILDCARD_TYPE.

I think that in this case it should return true. And by extension it should return true for application/* media types.

In other words, mediaType = MediaType.WILDCARD_TYPE should be handled in the same way as mediaType = null.

protected boolean hasMatchingMediaType(MediaType mediaType)

@cowtowncoder cowtowncoder added need-test-case To work on issue, a reproduction (ideally unit test) needed 2.11 labels Jan 5, 2020
@cowtowncoder
Copy link
Member

Should be easy to add in 2.11 (too risky for inclusion in patches, like 2.10.3). But would be great would be a test case to show expected usage -- do clients actually send wildcard matches? Or, rather, does Jersey (et al) map empty/missing Accept header as wild card?

Also: should the same change be made for all format providers, not just json? I assume so?

My main concern here is actually that behavioral change may break some users, so there is also question of downsides, potential breakages.

@cowtowncoder
Copy link
Member

Would need a reproduction and/or PR here; will leave open for now but may close in near future.

@cowtowncoder cowtowncoder removed the 2.11 label Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed
Projects
None yet
Development

No branches or pull requests

2 participants