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

Select return content type based on Accept header #919

Merged
merged 10 commits into from
Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -458,14 +458,14 @@ follows:
builder.Services.AddResponseCompression(options =>
{
options.EnableForHttps = true; // may lead to CRIME and BREACH attacks
options.MimeTypes = new[] { "application/json", "application/graphql+json" };
options.MimeTypes = new[] { "application/json", "application/graphql-response+json" };
})

// place this first/early in the pipeline
app.UseResponseCompression();
```

In order to compress GraphQL responses, the `application/graphql+json` content type must be
In order to compress GraphQL responses, the `application/graphql-response+json` content type must be
added to the `MimeTypes` option. You may choose to enable other content types as well.

Please note that enabling response compression over HTTPS can lead to CRIME and BREACH
Expand Down Expand Up @@ -661,7 +661,7 @@ A list of methods are as follows:
| `HandleWebSocketSubProtocolNotSupportedAsync` | Writes a '400 Invalid WebSocket sub-protocol.' message to the output. |

Below is a sample of custom middleware to change the response content type to `application/json`,
rather than the default of `application/graphql+json`:
rather than the default of `application/graphql-response+json`:

```csharp
class MyMiddleware<TSchema> : GraphQLHttpMiddleware<TSchema>
Expand Down
4 changes: 2 additions & 2 deletions docs/migration/migration7.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# Migrating from v6 to v7
# Migrating from v6 to v7.1

## Major changes and new features

#### GraphQL Middleware

- Configuration simplified to a single line of code
- Single middleware to support GET, POST and WebSocket connections (configurable)
- Media type of 'application/graphql+json' is accepted and returned as recommended by the draft spec (configurable via virtual method)
- Media type of 'application/graphql-response+json' is accepted and returned as recommended by the draft spec (configurable via virtual method)
- Batched requests will execute in parallel within separate service scopes (configurable)
- Authorization rules can be set on endpoints, regardless of schema configuration
- Mutation requests are disallowed over GET connections, as required by the spec
Expand Down
32 changes: 31 additions & 1 deletion src/Transports.AspNetCore/GraphQLHttpMiddleware.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma warning disable CA1716 // Identifiers should not match keywords

using Microsoft.AspNetCore.Http;

namespace GraphQL.Server.Transports.AspNetCore;

/// <inheritdoc/>
Expand Down Expand Up @@ -54,7 +56,9 @@ public class GraphQLHttpMiddleware : IUserContextBuilder
private const string MEDIATYPE_GRAPHQLJSON = "application/graphql+json";
private const string MEDIATYPE_JSON = "application/json";
private const string MEDIATYPE_GRAPHQL = "application/graphql";
internal const string CONTENTTYPE_JSON = "application/json; charset=utf-8";
internal const string CONTENTTYPE_GRAPHQLJSON = "application/graphql+json; charset=utf-8";
internal const string CONTENTTYPE_GRAPHQLRESPONSEJSON = "application/graphql-response+json; charset=utf-8";

/// <summary>
/// Initializes a new instance.
Expand Down Expand Up @@ -444,6 +448,16 @@ protected virtual async Task<ExecutionResult> ExecuteRequestAsync(HttpContext co
ValueTask<IDictionary<string, object?>?> IUserContextBuilder.BuildUserContextAsync(HttpContext context, object? payload)
=> BuildUserContextAsync(context, payload);

private static readonly (string MediaType, string ResponseType)[] _validMediaTypes = new[]
{
("application/graphql-response+json", CONTENTTYPE_GRAPHQLRESPONSEJSON),
("application/json", CONTENTTYPE_JSON),
("application/graphql+json", CONTENTTYPE_GRAPHQLJSON),
("*/*", CONTENTTYPE_GRAPHQLRESPONSEJSON),
("application/*", CONTENTTYPE_GRAPHQLRESPONSEJSON),
("application/*+json", CONTENTTYPE_GRAPHQLRESPONSEJSON),
};

/// <summary>
/// Selects a response content type string based on the <see cref="HttpContext"/>.
/// Defaults to <see cref="CONTENTTYPE_GRAPHQLJSON"/>. Override this value for compatibility
Expand All @@ -454,7 +468,23 @@ protected virtual async Task<ExecutionResult> ExecuteRequestAsync(HttpContext co
/// <see cref="WriteJsonResponseAsync{TResult}(HttpContext, HttpStatusCode, TResult)"/>.
/// </summary>
protected virtual string SelectResponseContentType(HttpContext context)
=> CONTENTTYPE_GRAPHQLJSON;
{
var acceptHeaders = context.Request.GetTypedHeaders().Accept;
if (acceptHeaders != null)
{
for (int i = 0; i < acceptHeaders.Count; i++)
{
for (int j = 0; j < _validMediaTypes.Length; j++)
{
var (mediaType, responseType) = _validMediaTypes[j];
if (acceptHeaders[i].MediaType.Equals(mediaType, StringComparison.OrdinalIgnoreCase))
return responseType;
}
}
}

return CONTENTTYPE_GRAPHQLRESPONSEJSON;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that if you only have a single misbehaving client, then you would need to subclass the GraphQLHttpMiddleware and recreate the logic above with a different fall-back. It would still be really good to be able to just override the default response content type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

}

/// <summary>
/// Writes the specified object (usually a GraphQL response represented as an instance of <see cref="ExecutionResult"/>) as JSON to the HTTP response stream.
Expand Down
2 changes: 1 addition & 1 deletion tests/Samples.Tests/TestServerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static async Task VerifyGraphQLPostAsync(
{
using var client = server.CreateClient();
var body = System.Text.Json.JsonSerializer.Serialize(new { query });
var content = new StringContent(body, System.Text.Encoding.UTF8, "application/graphql+json");
var content = new StringContent(body, System.Text.Encoding.UTF8, "application/json");
using var request = new HttpRequestMessage(HttpMethod.Post, url);
request.Content = content;
if (jwtToken != null)
Expand Down
23 changes: 23 additions & 0 deletions tests/Transports.AspNetCore.Tests/Middleware/GetTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,29 @@ public async Task BasicTest()
await response.ShouldBeAsync(@"{""data"":{""count"":0}}");
}

[Theory]
[InlineData("application/graphql+json", "application/graphql+json; charset=utf-8")]
[InlineData("application/json", "application/json; charset=utf-8")]
[InlineData("application/json; charset=utf-8", "application/json; charset=utf-8")]
[InlineData("application/json; charset=UTF-8", "application/json; charset=utf-8")]
[InlineData("APPLICATION/JSON", "application/json; charset=utf-8")]
[InlineData("APPLICATION/JSON; CHARSET=\"UTF-8\" ", "application/json; charset=utf-8")]
[InlineData("*/*; CHARSET=\"UTF-8\" ", "application/graphql-response+json; charset=utf-8")]
[InlineData("application/*; charset=utf-8", "application/graphql-response+json; charset=utf-8")]
[InlineData("application/*+json; charset=utf-8", "application/graphql-response+json; charset=utf-8")]
[InlineData("application/pdf", "application/graphql-response+json; charset=utf-8")]
[InlineData("application/json; charset=utf-7", "application/json; charset=utf-8")]
public async Task AcceptHeaderHonored(string mediaType, string expected)
{
var client = _server.CreateClient();
using var request = new HttpRequestMessage(HttpMethod.Get, "/graphql?query={count}");
request.Headers.Add("Accept", mediaType);
using var response = await client.SendAsync(request);
var contentType = response.Content.Headers.ContentType?.ToString();
contentType.ShouldBe(expected);
(await response.Content.ReadAsStringAsync()).ShouldBe(@"{""data"":{""count"":0}}");
}

[Fact]
public async Task NoUseWebSockets()
{
Expand Down
6 changes: 4 additions & 2 deletions tests/Transports.AspNetCore.Tests/ShouldlyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ public static Task ShouldBeAsync(this HttpResponseMessage message, string expect
public static async Task ShouldBeAsync(this HttpResponseMessage message, HttpStatusCode httpStatusCode, string expectedResponse)
{
message.StatusCode.ShouldBe(httpStatusCode);
message.Content.Headers.ContentType?.MediaType.ShouldBe("application/graphql+json");
message.Content.Headers.ContentType?.CharSet.ShouldBe("utf-8");
var contentType = message.Content.Headers.ContentType;
contentType.ShouldNotBeNull();
contentType.MediaType.ShouldBe("application/graphql-response+json");
contentType.CharSet.ShouldBe("utf-8");
var actualResponse = await message.Content.ReadAsStringAsync();
actualResponse.ShouldBe(expectedResponse);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Transports.AspNetCore.Tests/TestServerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static async Task<string> ExecutePost(this TestServer server, string url,
{
var client = server.CreateClient();
var data = System.Text.Json.JsonSerializer.Serialize(new { query = query, variables = variables });
var content = new StringContent(data, Encoding.UTF8, "application/graphql+json");
var content = new StringContent(data, Encoding.UTF8, "application/json");
using var response = await client.PostAsync(url, content);
response.EnsureSuccessStatusCode();
var str = await response.Content.ReadAsStringAsync();
Expand Down