Don't catch InvalidDataExceptions when reading JSON

Author: halter73

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

@@ -586,7 +586,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
                             Log.RequestBodyIOException(httpContext, ex);
                             return;
                         }
-                        catch (Exception ex) when (ex is InvalidDataException || ex is JsonException)
+                        catch (JsonException ex)
                         {
                             Log.InvalidJsonRequestBody(httpContext, parameterTypeName, parameterName, ex, factoryContext.ThrowOnBadRequest);
                             httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
@@ -624,7 +624,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
                             Log.RequestBodyIOException(httpContext, ex);
                             return;
                         }
-                        catch (Exception ex) when (ex is InvalidDataException || ex is JsonException)
+                        catch (JsonException ex)
                         {
 
                             Log.InvalidJsonRequestBody(httpContext, parameterTypeName, parameterName, ex, factoryContext.ThrowOnBadRequest);
@@ -1271,16 +1271,6 @@ public static void RequiredParameterNotProvided(HttpContext httpContext, string
             [LoggerMessage(4, LogLevel.Debug, RequiredParameterNotProvidedLogMessage, EventName = "RequiredParameterNotProvided")]
             private static partial void RequiredParameterNotProvided(ILogger logger, string parameterType, string parameterName, string source);
 
-            public static void UnexpectedContentType(HttpContext httpContext, string? contentType, bool shouldThrow)
-            {
-                if (shouldThrow)
-                {
-                    var message = string.Format(CultureInfo.InvariantCulture, UnexpectedContentTypeExceptionMessage, contentType);
-                    throw new BadHttpRequestException(message, StatusCodes.Status415UnsupportedMediaType);
-                }
-
-                UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");
-            }
             public static void ImplicitBodyNotProvided(HttpContext httpContext, string parameterName, bool shouldThrow)
             {
                 if (shouldThrow)
@@ -1295,8 +1285,16 @@ public static void ImplicitBodyNotProvided(HttpContext httpContext, string param
             [LoggerMessage(5, LogLevel.Debug, ImplicitBodyNotProvidedLogMessage, EventName = "ImplicitBodyNotProvided")]
             private static partial void ImplicitBodyNotProvided(ILogger logger, string parameterName);
 
-            public static void UnexpectedContentType(HttpContext httpContext, string? contentType)
-                => UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");
+            public static void UnexpectedContentType(HttpContext httpContext, string? contentType, bool shouldThrow)
+            {
+                if (shouldThrow)
+                {
+                    var message = string.Format(CultureInfo.InvariantCulture, UnexpectedContentTypeExceptionMessage, contentType);
+                    throw new BadHttpRequestException(message, StatusCodes.Status415UnsupportedMediaType);
+                }
+
+                UnexpectedContentType(GetLogger(httpContext), contentType ?? "(none)");
+            }
 
             [LoggerMessage(6, LogLevel.Debug, UnexpectedContentTypeLogMessage, EventName = "UnexpectedContentType")]
             private static partial void UnexpectedContentType(ILogger logger, string contentType);

src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs

@@ -904,7 +904,7 @@ void TestAction([FromRoute] int tryParsable, [FromRoute] int tryParsable2)
         }
 
         [Fact]
-        public async Task RequestDelegateLogsTryParsableFailuresAsDebugAndThrowsIfThrowOnBadRequest()
+        public async Task RequestDelegateThrowsForTryParsableFailuresIfThrowOnBadRequest()
         {
             var invoked = false;
 
@@ -969,7 +969,7 @@ public async Task RequestDelegateLogsBindAsyncFailuresAndSets400Response()
         }
 
         [Fact]
-        public async Task RequestDelegateLogsBindAsyncFailuresAndThrowsIfThrowOnBadRequest()
+        public async Task RequestDelegateThrowsForBindAsyncFailuresIfThrowOnBadRequest()
         {
             // Not supplying any headers will cause the HttpContext BindAsync overload to return null.
             var httpContext = CreateHttpContext();
@@ -1031,7 +1031,7 @@ public async Task RequestDelegateLogsSingleArgBindAsyncFailuresAndSets400Respons
         }
 
         [Fact]
-        public async Task RequestDelegateLogsSingleArgBindAsyncFailuresAndThrowsIfThrowOnBadRequest()
+        public async Task RequestDelegateThrowsForSingleArgBindAsyncFailuresIfThrowOnBadRequest()
         {
             // Not supplying any headers will cause the HttpContext BindAsync overload to return null.
             var httpContext = CreateHttpContext();
@@ -1421,7 +1421,7 @@ void TestAction([FromBody(AllowEmpty = true)] BodyStruct bodyStruct)
         [Theory]
         [InlineData(true)]
         [InlineData(false)]
-        public async Task RequestDelegateLogsFromBodyIOExceptionsAsDebugDoesNotAbortAndNeverThrows(bool throwOnBadRequests)
+        public async Task RequestDelegateLogsIOExceptionsAsDebugDoesNotAbortAndNeverThrows(bool throwOnBadRequests)
         {
             var invoked = false;
 
@@ -1454,7 +1454,7 @@ void TestAction([FromBody] Todo todo)
         }
 
         [Fact]
-        public async Task RequestDelegateLogsFromBodyInvalidDataExceptionsAsDebugAndSets400Response()
+        public async Task RequestDelegateLogsJsonExceptionsAsDebugAndSets400Response()
         {
             var invoked = false;
 
@@ -1463,12 +1463,12 @@ void TestAction([FromBody] Todo todo)
                 invoked = true;
             }
 
-            var invalidDataException = new InvalidDataException();
+            var jsonException = new JsonException();
 
             var httpContext = CreateHttpContext();
             httpContext.Request.Headers["Content-Type"] = "application/json";
             httpContext.Request.Headers["Content-Length"] = "1";
-            httpContext.Request.Body = new ExceptionThrowingRequestBodyStream(invalidDataException);
+            httpContext.Request.Body = new ExceptionThrowingRequestBodyStream(jsonException);
             httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
 
             var factoryResult = RequestDelegateFactory.Create(TestAction);
@@ -1485,11 +1485,11 @@ void TestAction([FromBody] Todo todo)
             Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
             Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
             Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
-            Assert.Same(invalidDataException, logMessage.Exception);
+            Assert.Same(jsonException, logMessage.Exception);
         }
 
         [Fact]
-        public async Task RequestDelegateLogsFromBodyJsonExceptionAsDebugAndSets400Response()
+        public async Task RequestDelegateThrowsForJsonExceptionsIfThrowOnBadRequest()
         {
             var invoked = false;
 
@@ -1498,31 +1498,36 @@ void TestAction([FromBody] Todo todo)
                 invoked = true;
             }
 
+            var jsonException = new JsonException();
+
             var httpContext = CreateHttpContext();
             httpContext.Request.Headers["Content-Type"] = "application/json";
             httpContext.Request.Headers["Content-Length"] = "1";
-            httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{"));
+            httpContext.Request.Body = new ExceptionThrowingRequestBodyStream(jsonException);
             httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
 
-            var factoryResult = RequestDelegateFactory.Create(TestAction);
+            var factoryResult = RequestDelegateFactory.Create(TestAction, new() { ThrowOnBadRequest = true });
             var requestDelegate = factoryResult.RequestDelegate;
 
-            await requestDelegate(httpContext);
+            var badHttpRequestException = await Assert.ThrowsAsync<BadHttpRequestException>(() => requestDelegate(httpContext));
 
             Assert.False(invoked);
+
+            // The httpContext should be untouched.
             Assert.False(httpContext.RequestAborted.IsCancellationRequested);
-            Assert.Equal(400, httpContext.Response.StatusCode);
+            Assert.Equal(200, httpContext.Response.StatusCode);
             Assert.False(httpContext.Response.HasStarted);
 
-            var logMessage = Assert.Single(TestSink.Writes);
-            Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
-            Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
-            Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
-            Assert.IsType<JsonException>(logMessage.Exception);
+            // We don't log bad requests when we throw.
+            Assert.Empty(TestSink.Writes);
+
+            Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", badHttpRequestException.Message);
+            Assert.Equal(400, badHttpRequestException.StatusCode);
+            Assert.Same(jsonException, badHttpRequestException.InnerException);
         }
 
         [Fact]
-        public async Task RequestDelegateLogsFromBodyInvalidDataExceptionsAsDebugAndThrowsIfThrowOnBadRequest()
+        public async Task RequestDelegateLogsMalformedJsonAsDebugAndSets400Response()
         {
             var invoked = false;
 
@@ -1531,36 +1536,31 @@ void TestAction([FromBody] Todo todo)
                 invoked = true;
             }
 
-            var invalidDataException = new InvalidDataException();
-
             var httpContext = CreateHttpContext();
             httpContext.Request.Headers["Content-Type"] = "application/json";
             httpContext.Request.Headers["Content-Length"] = "1";
-            httpContext.Request.Body = new ExceptionThrowingRequestBodyStream(invalidDataException);
+            httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{"));
             httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
 
-            var factoryResult = RequestDelegateFactory.Create(TestAction, new() { ThrowOnBadRequest = true });
+            var factoryResult = RequestDelegateFactory.Create(TestAction);
             var requestDelegate = factoryResult.RequestDelegate;
 
-            var badHttpRequestException = await Assert.ThrowsAsync<BadHttpRequestException>(() => requestDelegate(httpContext));
+            await requestDelegate(httpContext);
 
             Assert.False(invoked);
-
-            // The httpContext should be untouched.
             Assert.False(httpContext.RequestAborted.IsCancellationRequested);
-            Assert.Equal(200, httpContext.Response.StatusCode);
+            Assert.Equal(400, httpContext.Response.StatusCode);
             Assert.False(httpContext.Response.HasStarted);
 
-            // We don't log bad requests when we throw.
-            Assert.Empty(TestSink.Writes);
-
-            Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", badHttpRequestException.Message);
-            Assert.Equal(400, badHttpRequestException.StatusCode);
-            Assert.Same(invalidDataException, badHttpRequestException.InnerException);
+            var logMessage = Assert.Single(TestSink.Writes);
+            Assert.Equal(new EventId(2, "InvalidJsonRequestBody"), logMessage.EventId);
+            Assert.Equal(LogLevel.Debug, logMessage.LogLevel);
+            Assert.Equal(@"Failed to read parameter ""Todo todo"" from the request body as JSON.", logMessage.Message);
+            Assert.IsType<JsonException>(logMessage.Exception);
         }
 
         [Fact]
-        public async Task RequestDelegateLogsFromBodyJsonExceptionsAsDebugAndThrowsIfThrowOnBadRequest()
+        public async Task RequestDelegateThrowsForMalformedJsonIfThrowOnBadRequest()
         {
             var invoked = false;