Add accept header to grpc-web-text requests (#1522)

JamesNK · web-flow · commit 7d081d7c2682 · 2021-12-16T11:27:23.000+13:00
diff --git a/src/Grpc.AspNetCore.Web/Internal/GrpcWebFeature.cs b/src/Grpc.AspNetCore.Web/Internal/GrpcWebFeature.cs
@@ -34,7 +34,7 @@ internal class GrpcWebFeature :
         private readonly IHttpResponseTrailersFeature? _initialTrailersFeature;
         private bool _isComplete;
 
-        public GrpcWebFeature(ServerGrpcWebMode grpcWebMode, HttpContext httpContext)
+        public GrpcWebFeature(ServerGrpcWebContext grcpWebContext, HttpContext httpContext)
         {
             // Capture existing features. We'll use these internally, and restore them onto the context
             // once the middleware has finished executing.
@@ -52,14 +52,20 @@ public GrpcWebFeature(ServerGrpcWebMode grpcWebMode, HttpContext httpContext)
             var innerWriter = _initialResponseFeature.Writer ?? httpContext.Response.BodyWriter;
 
             Trailers = new HeaderDictionary();
-            if (grpcWebMode == ServerGrpcWebMode.GrpcWebText)
+            if (grcpWebContext.Request == ServerGrpcWebMode.GrpcWebText)
             {
                 Reader = new Base64PipeReader(innerReader);
-                Writer = new Base64PipeWriter(innerWriter);
             }
             else
             {
                 Reader = innerReader;
+            }
+            if (grcpWebContext.Response == ServerGrpcWebMode.GrpcWebText)
+            {
+                Writer = new Base64PipeWriter(innerWriter);
+            }
+            else
+            {
                 Writer = innerWriter;
             }
 
diff --git a/src/Grpc.AspNetCore.Web/Internal/GrpcWebMiddleware.cs b/src/Grpc.AspNetCore.Web/Internal/GrpcWebMiddleware.cs
@@ -39,15 +39,15 @@ public GrpcWebMiddleware(IOptions<GrpcWebOptions> options, ILogger<GrpcWebMiddle
 
         public Task Invoke(HttpContext httpContext)
         {
-            var mode = GetGrpcWebMode(httpContext);
-            if (mode != ServerGrpcWebMode.None)
+            var grcpWebContext = GetGrpcWebContext(httpContext);
+            if (grcpWebContext.Request != ServerGrpcWebMode.None)
             {
                 Log.DetectedGrpcWebRequest(_logger, httpContext.Request.ContentType!);
 
                 var metadata = httpContext.GetEndpoint()?.Metadata.GetMetadata<IGrpcWebEnabledMetadata>();
                 if (metadata?.GrpcWebEnabled ?? _options.DefaultEnabled)
                 {
-                    return HandleGrpcWebRequest(httpContext, mode);
+                    return HandleGrpcWebRequest(httpContext, grcpWebContext);
                 }
 
                 Log.GrpcWebRequestNotProcessed(_logger);
@@ -56,9 +56,9 @@ public Task Invoke(HttpContext httpContext)
             return _next(httpContext);
         }
 
-        private async Task HandleGrpcWebRequest(HttpContext httpContext, ServerGrpcWebMode mode)
+        private async Task HandleGrpcWebRequest(HttpContext httpContext, ServerGrpcWebContext grcpWebContext)
         {
-            var feature = new GrpcWebFeature(mode, httpContext);
+            var feature = new GrpcWebFeature(grcpWebContext, httpContext);
 
             var initialProtocol = httpContext.Request.Protocol;
 
@@ -75,7 +75,7 @@ private async Task HandleGrpcWebRequest(HttpContext httpContext, ServerGrpcWebMo
 
                 if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcContentType, httpContext.Response.ContentType!))
                 {
-                    var contentType = mode == ServerGrpcWebMode.GrpcWeb
+                    var contentType = grcpWebContext.Response == ServerGrpcWebMode.GrpcWeb
                         ? GrpcWebProtocolConstants.GrpcWebContentType
                         : GrpcWebProtocolConstants.GrpcWebTextContentType;
                     var responseContentType = ResolveContentType(contentType, httpContext.Response.ContentType);
@@ -111,21 +111,46 @@ private static string ResolveContentType(string newContentType, string originalC
             return newContentType;
         }
 
-        internal static ServerGrpcWebMode GetGrpcWebMode(HttpContext httpContext)
+        internal static ServerGrpcWebContext GetGrpcWebContext(HttpContext httpContext)
         {
-            if (HttpMethods.IsPost(httpContext.Request.Method))
+            var serverContext = new ServerGrpcWebContext();
+
+            if (TryGetWebMode(httpContext.Request.ContentType, out var requestMode))
             {
-                if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebContentType, httpContext.Request.ContentType))
+                serverContext.Request = requestMode;
+
+                if (TryGetWebMode(httpContext.Request.Headers["Accept"], out var responseMode))
                 {
-                    return ServerGrpcWebMode.GrpcWeb;
+                    serverContext.Response = responseMode;
                 }
-                else if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebTextContentType, httpContext.Request.ContentType))
+                else
                 {
-                    return ServerGrpcWebMode.GrpcWebText;
+                    // If there isn't a request 'accept' header then default to mode to 'application/grpc`.
+                    serverContext.Response = ServerGrpcWebMode.GrpcWeb;
                 }
             }
-            
-            return ServerGrpcWebMode.None;
+
+            return serverContext;
+        }
+
+        private static bool TryGetWebMode(string? contentType, out ServerGrpcWebMode mode)
+        {
+            if (!string.IsNullOrEmpty(contentType))
+            {
+                if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebContentType, contentType))
+                {
+                    mode = ServerGrpcWebMode.GrpcWeb;
+                    return true;
+                }
+                else if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebTextContentType, contentType))
+                {
+                    mode = ServerGrpcWebMode.GrpcWebText;
+                    return true;
+                }
+            }
+
+            mode = ServerGrpcWebMode.None;
+            return false;
         }
 
         private static class Log
diff --git a/src/Grpc.AspNetCore.Web/Internal/ServerGrpcWebMode.cs b/src/Grpc.AspNetCore.Web/Internal/ServerGrpcWebMode.cs
@@ -18,6 +18,8 @@
 
 namespace Grpc.AspNetCore.Web.Internal
 {
+    internal record struct ServerGrpcWebContext(ServerGrpcWebMode Request, ServerGrpcWebMode Response);
+
     internal enum ServerGrpcWebMode
     {
         None,
diff --git a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs
@@ -114,6 +114,14 @@ private async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request
         {
             request.Content = new GrpcWebRequestContent(request.Content!, GrpcWebMode);
 
+            // https://github.com/grpc/grpc/blob/f8a5022a2629e0929eb30e0583af66f0c220791b/doc/PROTOCOL-WEB.md
+            // The client library should indicate to the server via the "Accept" header that the response stream
+            // needs to be text encoded e.g. when XHR is used or due to security policies with XHR.
+            if (GrpcWebMode == GrpcWebMode.GrpcWebText)
+            {
+                request.Headers.TryAddWithoutValidation("Accept", GrpcWebProtocolConstants.GrpcWebTextContentType);
+            }
+
             if (OperatingSystem.IsBrowser)
             {
                 FixBrowserUserAgent(request);
diff --git a/test/Grpc.AspNetCore.Server.Tests/Web/GrpcWebFeatureTests.cs b/test/Grpc.AspNetCore.Server.Tests/Web/GrpcWebFeatureTests.cs
@@ -33,7 +33,7 @@ public void Ctor_DefaultHttpContext_FeaturesSet()
             var httpContext = new DefaultHttpContext();
 
             // Act
-            var feature = new GrpcWebFeature(ServerGrpcWebMode.GrpcWeb, httpContext);
+            var feature = CreateFeature(httpContext);
 
             // Assert
             Assert.AreEqual(feature, httpContext.Features.Get<IHttpResponseBodyFeature>());
@@ -48,7 +48,7 @@ public void DetachFromContext_InitialHttpContext_FeaturesReset()
             // Arrange
             var httpContext = new DefaultHttpContext();
             var responseBodyFeature = httpContext.Features.Get<IHttpResponseBodyFeature>();
-            var feature = new GrpcWebFeature(ServerGrpcWebMode.GrpcWeb, httpContext);
+            var feature = CreateFeature(httpContext);
 
             // Act
             feature.DetachFromContext(httpContext);
@@ -59,5 +59,12 @@ public void DetachFromContext_InitialHttpContext_FeaturesReset()
             Assert.AreEqual(null, httpContext.Features.Get<IHttpResponseTrailersFeature>());
             Assert.AreEqual(null, httpContext.Features.Get<IHttpResetFeature>());
         }
+
+        private static GrpcWebFeature CreateFeature(DefaultHttpContext httpContext)
+        {
+            return new GrpcWebFeature(
+                new ServerGrpcWebContext(ServerGrpcWebMode.GrpcWeb, ServerGrpcWebMode.GrpcWeb),
+                httpContext);
+        }
     }
 }
diff --git a/test/Grpc.AspNetCore.Server.Tests/Web/GrpcWebMiddlewareTests.cs b/test/Grpc.AspNetCore.Server.Tests/Web/GrpcWebMiddlewareTests.cs
@@ -66,10 +66,38 @@ public void GetGrpcWebMode_ContentTypes_Matched(string contentType, string expec
             httpContext.Request.ContentType = contentType;
 
             // Act
-            var grpcWebMode = GrpcWebMiddleware.GetGrpcWebMode(httpContext);
+            var grpcWebContext = GrpcWebMiddleware.GetGrpcWebContext(httpContext);
 
             // Assert
-            Assert.AreEqual(Enum.Parse<ServerGrpcWebMode>(expectedGrpcWebMode), grpcWebMode);
+            Assert.AreEqual(Enum.Parse<ServerGrpcWebMode>(expectedGrpcWebMode), grpcWebContext.Request);
+        }
+
+        [TestCase(GrpcWebProtocolConstants.GrpcWebContentType, null,
+            nameof(ServerGrpcWebMode.GrpcWeb), nameof(ServerGrpcWebMode.GrpcWeb))]
+        [TestCase(GrpcWebProtocolConstants.GrpcWebContentType, GrpcWebProtocolConstants.GrpcWebTextContentType,
+            nameof(ServerGrpcWebMode.GrpcWeb), nameof(ServerGrpcWebMode.GrpcWebText))]
+        [TestCase(GrpcWebProtocolConstants.GrpcWebTextContentType, GrpcWebProtocolConstants.GrpcWebTextContentType,
+            nameof(ServerGrpcWebMode.GrpcWebText), nameof(ServerGrpcWebMode.GrpcWebText))]
+        [TestCase("application/json", null,
+            nameof(ServerGrpcWebMode.None), nameof(ServerGrpcWebMode.None))]
+        [TestCase("", null,
+            nameof(ServerGrpcWebMode.None), nameof(ServerGrpcWebMode.None))]
+        public void GetGrpcWebMode_Accept_Matched(
+            string? contentType, string? accept,
+            string expectedRequestGrpcWebMode, string expectedResponseGrpcWebMode)
+        {
+            // Arrange
+            var httpContext = new DefaultHttpContext();
+            httpContext.Request.Method = HttpMethods.Post;
+            httpContext.Request.ContentType = contentType!;
+            httpContext.Request.Headers["Accept"] = accept;
+
+            // Act
+            var grpcWebContext = GrpcWebMiddleware.GetGrpcWebContext(httpContext);
+
+            // Assert
+            Assert.AreEqual(Enum.Parse<ServerGrpcWebMode>(expectedRequestGrpcWebMode), grpcWebContext.Request);
+            Assert.AreEqual(Enum.Parse<ServerGrpcWebMode>(expectedResponseGrpcWebMode), grpcWebContext.Response);
         }
 
         [Test]
diff --git a/test/Grpc.Net.Client.Web.Tests/GrpcWebHandlerTests.cs b/test/Grpc.Net.Client.Web.Tests/GrpcWebHandlerTests.cs
@@ -82,6 +82,33 @@ public async Task HttpVersion_Set_HttpRequestMessageVersionChanged()
             Assert.AreEqual(GrpcWebProtocolConstants.Http2Version, response.Version);
         }
 
+        [Test]
+        public async Task GrpcWebMode_GrpcWebText_AcceptHeaderAdded()
+        {
+            // Arrange
+            var request = new HttpRequestMessage
+            {
+                Version = GrpcWebProtocolConstants.Http2Version,
+                Content = new ByteArrayContent(Array.Empty<byte>())
+                {
+                    Headers = { ContentType = new MediaTypeHeaderValue("application/grpc") }
+                }
+            };
+            var testHttpHandler = new TestHttpHandler();
+            var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWebText)
+            {
+                InnerHandler = testHttpHandler
+            };
+            var messageInvoker = new HttpMessageInvoker(grpcWebHandler);
+
+            // Act
+            await messageInvoker.SendAsync(request, CancellationToken.None);
+
+            // Assert
+            Assert.IsTrue(testHttpHandler.Request!.Headers.TryGetValues("Accept", out var values));
+            Assert.AreEqual(GrpcWebProtocolConstants.GrpcWebTextContentType, values!.Single());
+        }
+
         [Test]
         public async Task SendAsync_GrpcCall_ResponseStreamingPropertySet()
         {

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ internal class GrpcWebFeature :`
`34`	`34`	`private readonly IHttpResponseTrailersFeature? _initialTrailersFeature;`
`35`	`35`	`private bool _isComplete;`
`36`	`36`
`37`		`- public GrpcWebFeature(ServerGrpcWebMode grpcWebMode, HttpContext httpContext)`
	`37`	`+ public GrpcWebFeature(ServerGrpcWebContext grcpWebContext, HttpContext httpContext)`
`38`	`38`	`{`
`39`	`39`	`// Capture existing features. We'll use these internally, and restore them onto the context`
`40`	`40`	`// once the middleware has finished executing.`
`@@ -52,14 +52,20 @@ public GrpcWebFeature(ServerGrpcWebMode grpcWebMode, HttpContext httpContext)`
`52`	`52`	`var innerWriter = _initialResponseFeature.Writer ?? httpContext.Response.BodyWriter;`
`53`	`53`
`54`	`54`	`Trailers = new HeaderDictionary();`
`55`		`- if (grpcWebMode == ServerGrpcWebMode.GrpcWebText)`
	`55`	`+ if (grcpWebContext.Request == ServerGrpcWebMode.GrpcWebText)`
`56`	`56`	`{`
`57`	`57`	`Reader = new Base64PipeReader(innerReader);`
`58`		`- Writer = new Base64PipeWriter(innerWriter);`
`59`	`58`	`}`
`60`	`59`	`else`
`61`	`60`	`{`
`62`	`61`	`Reader = innerReader;`
	`62`	`+ }`
	`63`	`+ if (grcpWebContext.Response == ServerGrpcWebMode.GrpcWebText)`
	`64`	`+ {`
	`65`	`+ Writer = new Base64PipeWriter(innerWriter);`
	`66`	`+ }`
	`67`	`+ else`
	`68`	`+ {`
`63`	`69`	`Writer = innerWriter;`
`64`	`70`	`}`
`65`	`71`
Original file line number	Diff line number	Diff line change
`@@ -39,15 +39,15 @@ public GrpcWebMiddleware(IOptions<GrpcWebOptions> options, ILogger<GrpcWebMiddle`
`39`	`39`
`40`	`40`	`public Task Invoke(HttpContext httpContext)`
`41`	`41`	`{`
`42`		`- var mode = GetGrpcWebMode(httpContext);`
`43`		`- if (mode != ServerGrpcWebMode.None)`
	`42`	`+ var grcpWebContext = GetGrpcWebContext(httpContext);`
	`43`	`+ if (grcpWebContext.Request != ServerGrpcWebMode.None)`
`44`	`44`	`{`
`45`	`45`	`Log.DetectedGrpcWebRequest(_logger, httpContext.Request.ContentType!);`
`46`	`46`
`47`	`47`	`var metadata = httpContext.GetEndpoint()?.Metadata.GetMetadata<IGrpcWebEnabledMetadata>();`
`48`	`48`	`if (metadata?.GrpcWebEnabled ?? _options.DefaultEnabled)`
`49`	`49`	`{`
`50`		`- return HandleGrpcWebRequest(httpContext, mode);`
	`50`	`+ return HandleGrpcWebRequest(httpContext, grcpWebContext);`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`Log.GrpcWebRequestNotProcessed(_logger);`
`@@ -56,9 +56,9 @@ public Task Invoke(HttpContext httpContext)`
`56`	`56`	`return _next(httpContext);`
`57`	`57`	`}`
`58`	`58`
`59`		`- private async Task HandleGrpcWebRequest(HttpContext httpContext, ServerGrpcWebMode mode)`
	`59`	`+ private async Task HandleGrpcWebRequest(HttpContext httpContext, ServerGrpcWebContext grcpWebContext)`
`60`	`60`	`{`
`61`		`- var feature = new GrpcWebFeature(mode, httpContext);`
	`61`	`+ var feature = new GrpcWebFeature(grcpWebContext, httpContext);`
`62`	`62`
`63`	`63`	`var initialProtocol = httpContext.Request.Protocol;`
`64`	`64`
`@@ -75,7 +75,7 @@ private async Task HandleGrpcWebRequest(HttpContext httpContext, ServerGrpcWebMo`
`75`	`75`
`76`	`76`	`if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcContentType, httpContext.Response.ContentType!))`
`77`	`77`	`{`
`78`		`- var contentType = mode == ServerGrpcWebMode.GrpcWeb`
	`78`	`+ var contentType = grcpWebContext.Response == ServerGrpcWebMode.GrpcWeb`
`79`	`79`	`? GrpcWebProtocolConstants.GrpcWebContentType`
`80`	`80`	`: GrpcWebProtocolConstants.GrpcWebTextContentType;`
`81`	`81`	`var responseContentType = ResolveContentType(contentType, httpContext.Response.ContentType);`
`@@ -111,21 +111,46 @@ private static string ResolveContentType(string newContentType, string originalC`
`111`	`111`	`return newContentType;`
`112`	`112`	`}`
`113`	`113`
`114`		`- internal static ServerGrpcWebMode GetGrpcWebMode(HttpContext httpContext)`
	`114`	`+ internal static ServerGrpcWebContext GetGrpcWebContext(HttpContext httpContext)`
`115`	`115`	`{`
`116`		`- if (HttpMethods.IsPost(httpContext.Request.Method))`
	`116`	`+ var serverContext = new ServerGrpcWebContext();`
	`117`	`+`
	`118`	`+ if (TryGetWebMode(httpContext.Request.ContentType, out var requestMode))`
`117`	`119`	`{`
`118`		`- if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebContentType, httpContext.Request.ContentType))`
	`120`	`+ serverContext.Request = requestMode;`
	`121`	`+`
	`122`	`+ if (TryGetWebMode(httpContext.Request.Headers["Accept"], out var responseMode))`
`119`	`123`	`{`
`120`		`- return ServerGrpcWebMode.GrpcWeb;`
	`124`	`+ serverContext.Response = responseMode;`
`121`	`125`	`}`
`122`		`- else if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebTextContentType, httpContext.Request.ContentType))`
	`126`	`+ else`
`123`	`127`	`{`
`124`		`- return ServerGrpcWebMode.GrpcWebText;`
	`128`	+ // If there isn't a request 'accept' header then default to mode to 'application/grpc`.
	`129`	`+ serverContext.Response = ServerGrpcWebMode.GrpcWeb;`
`125`	`130`	`}`
`126`	`131`	`}`
`127`		`-`
`128`		`- return ServerGrpcWebMode.None;`
	`132`	`+`
	`133`	`+ return serverContext;`
	`134`	`+ }`
	`135`	`+`
	`136`	`+ private static bool TryGetWebMode(string? contentType, out ServerGrpcWebMode mode)`
	`137`	`+ {`
	`138`	`+ if (!string.IsNullOrEmpty(contentType))`
	`139`	`+ {`
	`140`	`+ if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebContentType, contentType))`
	`141`	`+ {`
	`142`	`+ mode = ServerGrpcWebMode.GrpcWeb;`
	`143`	`+ return true;`
	`144`	`+ }`
	`145`	`+ else if (CommonGrpcProtocolHelpers.IsContentType(GrpcWebProtocolConstants.GrpcWebTextContentType, contentType))`
	`146`	`+ {`
	`147`	`+ mode = ServerGrpcWebMode.GrpcWebText;`
	`148`	`+ return true;`
	`149`	`+ }`
	`150`	`+ }`
	`151`	`+`
	`152`	`+ mode = ServerGrpcWebMode.None;`
	`153`	`+ return false;`
`129`	`154`	`}`
`130`	`155`
`131`	`156`	`private static class Log`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,8 @@`
`18`	`18`
`19`	`19`	`namespace Grpc.AspNetCore.Web.Internal`
`20`	`20`	`{`
	`21`	`+ internal record struct ServerGrpcWebContext(ServerGrpcWebMode Request, ServerGrpcWebMode Response);`
	`22`	`+`
`21`	`23`	`internal enum ServerGrpcWebMode`
`22`	`24`	`{`
`23`	`25`	`None,`
Original file line number	Diff line number	Diff line change
`@@ -114,6 +114,14 @@ private async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request`
`114`	`114`	`{`
`115`	`115`	`request.Content = new GrpcWebRequestContent(request.Content!, GrpcWebMode);`
`116`	`116`
	`117`	`+ // https://github.com/grpc/grpc/blob/f8a5022a2629e0929eb30e0583af66f0c220791b/doc/PROTOCOL-WEB.md`
	`118`	`+ // The client library should indicate to the server via the "Accept" header that the response stream`
	`119`	`+ // needs to be text encoded e.g. when XHR is used or due to security policies with XHR.`
	`120`	`+ if (GrpcWebMode == GrpcWebMode.GrpcWebText)`
	`121`	`+ {`
	`122`	`+ request.Headers.TryAddWithoutValidation("Accept", GrpcWebProtocolConstants.GrpcWebTextContentType);`
	`123`	`+ }`
	`124`	`+`
`117`	`125`	`if (OperatingSystem.IsBrowser)`
`118`	`126`	`{`
`119`	`127`	`FixBrowserUserAgent(request);`