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

Legacy "application/graphql" is not supported if charset is set #1036

Closed
gjinajgledis opened this issue Jul 24, 2024 · 5 comments
Closed

Legacy "application/graphql" is not supported if charset is set #1036

gjinajgledis opened this issue Jul 24, 2024 · 5 comments
Assignees
Labels
in: web Issues related to web handling status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@gjinajgledis
Copy link

Recent changes to support application/graphql seems like do not cater for the case when the content-type value is application/graphql;charset=UTF-8

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 24, 2024
@bclozel
Copy link
Member

bclozel commented Jul 24, 2024

Unfortunately, you're not giving us much to go on here. Can you elaborate?

  • are you talking about request or response content-type?
  • what do you mean by "does not cater for the case", are you getting an exception, an HTTP error status?
  • For which version this was working, and what version are you upgrading to?

Ideally, a minimal sample application should help us answer most of those.

Please note that "application/graphql" is supported in Spring for GraphQL, but not officially by the GraphQL community. See #375 and #563

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Jul 24, 2024
@gjinajgledis
Copy link
Author

Sorry Brian for not giving enough information.
According to this closed issue support for application/graphql for request body was added by this commit . So, in version 1.3.1 spring graphql started to support for the first time this media type.
But when I perform this request using Spring mockmvc:
mvc.perform(MockMvcRequestBuilders.post(PATH_GRAPHQL) .content("{ hello }") .contentType("application/graphql") .accept(MediaType.APPLICATION_JSON))
I still get this error: org.springframework.web.HttpMediaTypeNotSupportedException
Is this something expected or could be potentially an issue?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 25, 2024
@bclozel
Copy link
Member

bclozel commented Jul 25, 2024

This is covered by a test case in the same commit you're pointing at. Also, your initial comment mentioned "application/graphql;charset=UTF-8" but your code snippet doesn't. Maybe you can share a sample application?

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 25, 2024
@gjinajgledis
Copy link
Author

Hey Brian,
Please have a look to this sample application .
You will find inside AuthorIntegrationTest two test cases and one of them which is sending a application/graphql media type request fails.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 25, 2024
@bclozel bclozel changed the title ApplicationGraphQl Fallback does not support content-type header application/graphql;charset=UTF-8 Legacy "application/graphql" content type is not supported if charset is set Jul 26, 2024
@bclozel bclozel changed the title Legacy "application/graphql" content type is not supported if charset is set Legacy "application/graphql" is not supported if charset is set Jul 26, 2024
@bclozel bclozel added type: bug A general bug in: web Issues related to web handling and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jul 26, 2024
@bclozel bclozel self-assigned this Jul 26, 2024
@bclozel bclozel added this to the 1.3.3 milestone Jul 26, 2024
@bclozel bclozel added the for: backport-to-1.2.x Marks an issue as a candidate for backport to 1.2.x label Jul 26, 2024
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-1.2.x Marks an issue as a candidate for backport to 1.2.x labels Jul 26, 2024
@bclozel
Copy link
Member

bclozel commented Jul 26, 2024

Thanks for the sample.
MockMvc is adding automatically the charset information to the outgoing content type. This means the server is effectively receiving application/graphql;charset=UTF-8. The fallback introduced in #948 was incomplete since it checked for exact matches and did not compare media types.

We'll fix this in 1.2.x and 1.3.x but please note that you should get rid of "application/graphql" as soon as possible in favor of "application/json" (client requests) and "application/json"/"application/graphql-response+json" (server responses).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues related to web handling status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants