From 492b013d5edf3276789fe26623cfcc02f4546fd9 Mon Sep 17 00:00:00 2001 From: Mikolaj Date: Wed, 26 Apr 2023 17:13:58 +0200 Subject: [PATCH 1/6] change default implementation of response validation method to ensure correct response content type --- src/GraphQL.Client/GraphQLHttpClientOptions.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/GraphQL.Client/GraphQLHttpClientOptions.cs b/src/GraphQL.Client/GraphQLHttpClientOptions.cs index 13a9c576..2c1b8875 100644 --- a/src/GraphQL.Client/GraphQLHttpClientOptions.cs +++ b/src/GraphQL.Client/GraphQLHttpClientOptions.cs @@ -61,9 +61,18 @@ public class GraphQLHttpClientOptions /// Note that compatible to the draft graphql-over-http spec GraphQL Server MAY return 4xx status codes (401/403, etc.) /// with well-formed GraphQL response containing errors collection. /// - public Func IsValidResponseToDeserialize { get; set; } = r => + public Func IsValidResponseToDeserialize { get; set; } = DefaultIsValidResponseToDeserialize; + + internal static IReadOnlyCollection AcceptedResponseContentTypes { get; } = new[] { "application/graphql+json", "application/json", "application/graphql-response+json" }; + + private static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r) + { // Why not application/json? See https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#processing-the-response - r.IsSuccessStatusCode || r.StatusCode == HttpStatusCode.BadRequest || r.Content.Headers.ContentType?.MediaType == "application/graphql+json"; + if (r.Content.Headers.ContentType?.MediaType != null && !AcceptedResponseContentTypes.Contains(r.Content.Headers.ContentType.MediaType)) + return false; + + return r.IsSuccessStatusCode || r.StatusCode == HttpStatusCode.BadRequest; + } /// /// This callback is called after successfully establishing a websocket connection but before any regular request is made. From c300c0821eae9f8717481c2f2bdc2557e2a63436 Mon Sep 17 00:00:00 2001 From: Mikolaj Date: Thu, 27 Apr 2023 11:08:14 +0200 Subject: [PATCH 2/6] add tests for the default validation method --- .../DefaultValidationTest.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs diff --git a/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs b/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs new file mode 100644 index 00000000..00ceb428 --- /dev/null +++ b/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs @@ -0,0 +1,30 @@ +using System.Net; +using System.Net.Http.Headers; +using FluentAssertions; +using GraphQL.Client.Http; + +using Xunit; + +namespace GraphQL.Client.Serializer.Tests; + +public class DefaultValidationTest +{ + + [Theory] + [InlineData(HttpStatusCode.OK, "application/json", true)] + [InlineData(HttpStatusCode.OK, "application/graphql-response+json", true)] + [InlineData(HttpStatusCode.BadRequest, "application/json", true)] + [InlineData(HttpStatusCode.BadRequest, "text/html", false)] + [InlineData(HttpStatusCode.OK, "text/html", false)] + [InlineData(HttpStatusCode.Forbidden, "text/html", false)] + [InlineData(HttpStatusCode.Forbidden, "application/json", false)] + public void IsValidResponse_OkJson_True(HttpStatusCode statusCode, string mediaType, bool expectedResult) + { + var response = new HttpResponseMessage(statusCode); + response.Content.Headers.ContentType = new MediaTypeHeaderValue(mediaType); + + bool result = new GraphQLHttpClientOptions().IsValidResponseToDeserialize(response); + + result.Should().Be(expectedResult); + } +} From 9b9447be97dce918e4ec6a13148e3097c18a8fdf Mon Sep 17 00:00:00 2001 From: Mikolaj Date: Fri, 28 Apr 2023 11:00:12 +0200 Subject: [PATCH 3/6] remove unnecessary comments Co-authored-by: Ivan Maximov --- src/GraphQL.Client/GraphQLHttpClientOptions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/GraphQL.Client/GraphQLHttpClientOptions.cs b/src/GraphQL.Client/GraphQLHttpClientOptions.cs index 2c1b8875..b92784b6 100644 --- a/src/GraphQL.Client/GraphQLHttpClientOptions.cs +++ b/src/GraphQL.Client/GraphQLHttpClientOptions.cs @@ -67,7 +67,6 @@ public class GraphQLHttpClientOptions private static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r) { - // Why not application/json? See https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#processing-the-response if (r.Content.Headers.ContentType?.MediaType != null && !AcceptedResponseContentTypes.Contains(r.Content.Headers.ContentType.MediaType)) return false; From 93b25cbb0745421e22b0ef13091778e02266338e Mon Sep 17 00:00:00 2001 From: Mikolaj Date: Fri, 28 Apr 2023 11:03:42 +0200 Subject: [PATCH 4/6] accepted response types private, formatting --- src/GraphQL.Client/GraphQLHttpClientOptions.cs | 2 +- .../DefaultValidationTest.cs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/GraphQL.Client/GraphQLHttpClientOptions.cs b/src/GraphQL.Client/GraphQLHttpClientOptions.cs index b92784b6..eba873ca 100644 --- a/src/GraphQL.Client/GraphQLHttpClientOptions.cs +++ b/src/GraphQL.Client/GraphQLHttpClientOptions.cs @@ -63,7 +63,7 @@ public class GraphQLHttpClientOptions /// public Func IsValidResponseToDeserialize { get; set; } = DefaultIsValidResponseToDeserialize; - internal static IReadOnlyCollection AcceptedResponseContentTypes { get; } = new[] { "application/graphql+json", "application/json", "application/graphql-response+json" }; + private static IReadOnlyCollection AcceptedResponseContentTypes { get; } = new[] { "application/graphql+json", "application/json", "application/graphql-response+json" }; private static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r) { diff --git a/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs b/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs index 00ceb428..861b59b6 100644 --- a/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs +++ b/tests/GraphQL.Client.Serializer.Tests/DefaultValidationTest.cs @@ -2,14 +2,12 @@ using System.Net.Http.Headers; using FluentAssertions; using GraphQL.Client.Http; - using Xunit; namespace GraphQL.Client.Serializer.Tests; public class DefaultValidationTest { - [Theory] [InlineData(HttpStatusCode.OK, "application/json", true)] [InlineData(HttpStatusCode.OK, "application/graphql-response+json", true)] @@ -23,8 +21,8 @@ public void IsValidResponse_OkJson_True(HttpStatusCode statusCode, string mediaT var response = new HttpResponseMessage(statusCode); response.Content.Headers.ContentType = new MediaTypeHeaderValue(mediaType); - bool result = new GraphQLHttpClientOptions().IsValidResponseToDeserialize(response); + bool isValid = new GraphQLHttpClientOptions().IsValidResponseToDeserialize(response); - result.Should().Be(expectedResult); + isValid.Should().Be(expectedResult); } } From bd3b61ae74f4cd9b6b6a24d2098c4235fd08eb19 Mon Sep 17 00:00:00 2001 From: Mikolaj Date: Fri, 28 Apr 2023 11:05:58 +0200 Subject: [PATCH 5/6] make DefaultIsValidResponseToDeserialize publicly accessible Co-authored-by: Ivan Maximov --- src/GraphQL.Client/GraphQLHttpClientOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GraphQL.Client/GraphQLHttpClientOptions.cs b/src/GraphQL.Client/GraphQLHttpClientOptions.cs index eba873ca..b749b160 100644 --- a/src/GraphQL.Client/GraphQLHttpClientOptions.cs +++ b/src/GraphQL.Client/GraphQLHttpClientOptions.cs @@ -65,7 +65,7 @@ public class GraphQLHttpClientOptions private static IReadOnlyCollection AcceptedResponseContentTypes { get; } = new[] { "application/graphql+json", "application/json", "application/graphql-response+json" }; - private static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r) + public static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r) { if (r.Content.Headers.ContentType?.MediaType != null && !AcceptedResponseContentTypes.Contains(r.Content.Headers.ContentType.MediaType)) return false; From 024cf11fbe0d6031c93aea0f73bcf81a6274ac18 Mon Sep 17 00:00:00 2001 From: Mikolaj Date: Fri, 28 Apr 2023 16:43:08 +0200 Subject: [PATCH 6/6] AcceptedResponseContentTypes as static field --- src/GraphQL.Client/GraphQLHttpClientOptions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GraphQL.Client/GraphQLHttpClientOptions.cs b/src/GraphQL.Client/GraphQLHttpClientOptions.cs index eba873ca..1a9af223 100644 --- a/src/GraphQL.Client/GraphQLHttpClientOptions.cs +++ b/src/GraphQL.Client/GraphQLHttpClientOptions.cs @@ -63,11 +63,11 @@ public class GraphQLHttpClientOptions /// public Func IsValidResponseToDeserialize { get; set; } = DefaultIsValidResponseToDeserialize; - private static IReadOnlyCollection AcceptedResponseContentTypes { get; } = new[] { "application/graphql+json", "application/json", "application/graphql-response+json" }; + private static readonly IReadOnlyCollection _acceptedResponseContentTypes = new[] { "application/graphql+json", "application/json", "application/graphql-response+json" }; private static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r) { - if (r.Content.Headers.ContentType?.MediaType != null && !AcceptedResponseContentTypes.Contains(r.Content.Headers.ContentType.MediaType)) + if (r.Content.Headers.ContentType?.MediaType != null && !_acceptedResponseContentTypes.Contains(r.Content.Headers.ContentType.MediaType)) return false; return r.IsSuccessStatusCode || r.StatusCode == HttpStatusCode.BadRequest;