Merge pull request #541 from retendo/develop

RFC-compliant Content-Length handling for 1xx, 204 and 205 responses and CONNECT requests

src/Giraffe/Helpers.fs

@@ -34,3 +34,44 @@ module Helpers =
             use reader = new StreamReader(filePath)
             return! reader.ReadToEndAsync()
         }
+
+    /// <summary>
+    /// Utility function for matching 1xx HTTP status codes.
+    /// </summary>
+    /// <param name="statusCode">The HTTP status code.</param>
+    /// <returns>Returns true if the status code is between 100 and 199.</returns>
+    let is1xxStatusCode (statusCode : int) =
+        100 <= statusCode && statusCode <= 199
+
+    /// <summary>
+    /// Utility function for matching 2xx HTTP status codes.
+    /// </summary>
+    /// <param name="statusCode">The HTTP status code.</param>
+    /// <returns>Returns true if the status code is between 200 and 299.</returns>
+    let is2xxStatusCode (statusCode : int) =
+        200 <= statusCode && statusCode <= 299
+
+    /// <summary>
+    /// Utility function for matching 3xx HTTP status codes.
+    /// </summary>
+    /// <param name="statusCode">The HTTP status code.</param>
+    /// <returns>Returns true if the status code is between 300 and 399.</returns>
+    let is3xxStatusCode (statusCode : int) =
+        300 <= statusCode && statusCode <= 399
+
+    /// <summary>
+    /// Utility function for matching 4xx HTTP status codes.
+    /// </summary>
+    /// <param name="statusCode">The HTTP status code.</param>
+    /// <returns>Returns true if the status code is between 400 and 499.</returns>
+    let is4xxStatusCode (statusCode : int) =
+        400 <= statusCode && statusCode <= 499
+
+    /// <summary>
+    /// Utility function for matching 5xx HTTP status codes.
+    /// </summary>
+    /// <param name="statusCode">The HTTP status code.</param>
+    /// <returns>Returns true if the status code is between 500 and 599.</returns>
+    let is5xxStatusCode (statusCode : int) =
+        500 <= statusCode && statusCode <= 599
+

src/Giraffe/HttpContextExtensions.fs

@@ -334,15 +334,29 @@ type HttpContextExtensions() =
         }
 
     /// <summary>
-    /// Writes a byte array to the body of the HTTP response and sets the HTTP Content-Length header accordingly.
+    /// Writes a byte array to the body of the HTTP response and sets the HTTP Content-Length header accordingly.<br />
+    /// <br />
+    /// There are exceptions to be taken care of according to the RFC.<br />
+    /// 1. Don't send Content-Length headers on 1xx and 204 responses and on 2xx responses to CONNECT requests (https://httpwg.org/specs/rfc7230.html#rfc.section.3.3.2)<br />
+    /// 2. Don't send non-zero Content-Length headers for 205 responses (https://httpwg.org/specs/rfc7231.html#rfc.section.6.3.6)<br />
+    /// <br />
+    /// Since .NET 7 these rules are enforced by Kestrel (https://github.com/dotnet/aspnetcore/pull/43103)
     /// </summary>
     /// <param name="ctx">The current http context object.</param>
     /// <param name="bytes">The byte array to be send back to the client.</param>
     /// <returns>Task of Some HttpContext after writing to the body of the response.</returns>
     [<Extension>]
     static member WriteBytesAsync (ctx : HttpContext, bytes : byte[]) =
         task {
-            ctx.SetHttpHeader(HeaderNames.ContentLength, bytes.Length)
+            let canIncludeContentLengthHeader =
+                match ctx.Response.StatusCode, ctx.Request.Method with
+                | statusCode, _ when statusCode |> is1xxStatusCode || statusCode = 204 -> false
+                | statusCode, method when method = "CONNECT" && statusCode |> is2xxStatusCode -> false
+                | _ -> true
+            let is205StatusCode = ctx.Response.StatusCode = 205
+            if canIncludeContentLengthHeader then
+                let contentLength = if is205StatusCode then 0 else bytes.Length
+                ctx.SetHttpHeader(HeaderNames.ContentLength, contentLength)
             if ctx.Request.Method <> HttpMethods.Head then
                 do! ctx.Response.Body.WriteAsync(bytes, 0, bytes.Length)
             return Some ctx

tests/Giraffe.Tests/HttpContextExtensionsTests.fs

@@ -2,6 +2,7 @@ module Giraffe.Tests.HttpContextExtensionsTests
 
 open System
 open System.IO
+open System.Text
 open System.Threading.Tasks
 open Microsoft.AspNetCore.Http
 open Microsoft.Extensions.Primitives
@@ -175,6 +176,102 @@ let ``WriteTextAsync with HTTP HEAD should not return text in body`` () =
         | Some ctx -> Assert.Equal(expected, getBody ctx)
     }
 
+[<Fact>]
+let ``WriteBytesAsync should not return Content-Length in header on 100`` () =
+    let ctx = Substitute.For<HttpContext>()
+
+    let testHandler =
+        fun (_ : HttpFunc) (ctx : HttpContext) ->
+            ctx.WriteBytesAsync (Encoding.UTF8.GetBytes "")
+
+    let app = route "/" >=> testHandler
+
+    ctx.Request.Method.ReturnsForAnyArgs "GET" |> ignore
+    ctx.Request.Path.ReturnsForAnyArgs (PathString("/")) |> ignore
+    ctx.Response.StatusCode.ReturnsForAnyArgs 100 |> ignore
+    ctx.Response.Body <- new MemoryStream()
+
+    task {
+        let! result = app (Some >> Task.FromResult) ctx
+
+        match result with
+        | None -> assertFail "Result was expected to be non-empty"
+        | Some ctx ->
+            Assert.Empty(ctx.Response.Headers["Content-Length"])
+    }
+
+[<Fact>]
+let ``WriteBytesAsync should not return Content-Length in header on 204`` () =
+    let ctx = Substitute.For<HttpContext>()
+
+    let testHandler =
+        fun (_ : HttpFunc) (ctx : HttpContext) ->
+            ctx.WriteBytesAsync (Encoding.UTF8.GetBytes "")
+
+    let app = route "/" >=> testHandler
+
+    ctx.Request.Method.ReturnsForAnyArgs "GET" |> ignore
+    ctx.Request.Path.ReturnsForAnyArgs (PathString("/")) |> ignore
+    ctx.Response.StatusCode.ReturnsForAnyArgs 204 |> ignore
+    ctx.Response.Body <- new MemoryStream()
+
+    task {
+        let! result = app (Some >> Task.FromResult) ctx
+
+        match result with
+        | None -> assertFail "Result was expected to be non-empty"
+        | Some ctx ->
+            Assert.Empty(ctx.Response.Headers["Content-Length"])
+    }
+
+[<Fact>]
+let ``WriteBytesAsync with HTTP CONNECT should not return Content-Length in header on status code 200`` () =
+    let ctx = Substitute.For<HttpContext>()
+
+    let testHandler =
+        fun (_ : HttpFunc) (ctx : HttpContext) ->
+            ctx.WriteBytesAsync (Encoding.UTF8.GetBytes "")
+
+    let app = route "/" >=> testHandler
+
+    ctx.Request.Method.ReturnsForAnyArgs "CONNECT" |> ignore
+    ctx.Request.Path.ReturnsForAnyArgs (PathString("/")) |> ignore
+    ctx.Response.StatusCode.ReturnsForAnyArgs 200 |> ignore
+    ctx.Response.Body <- new MemoryStream()
+
+    task {
+        let! result = app (Some >> Task.FromResult) ctx
+
+        match result with
+        | None -> assertFail "Result was expected to be non-empty"
+        | Some ctx ->
+            Assert.Empty(ctx.Response.Headers["Content-Length"])
+    }
+
+[<Fact>]
+let ``WriteBytesAsync should return Content-Length 0 in header on 205`` () =
+    let ctx = Substitute.For<HttpContext>()
+
+    let testHandler =
+        fun (_ : HttpFunc) (ctx : HttpContext) ->
+            ctx.WriteBytesAsync (Encoding.UTF8.GetBytes "Hello World")
+
+    let app = route "/" >=> testHandler
+
+    ctx.Request.Method.ReturnsForAnyArgs "GET" |> ignore
+    ctx.Request.Path.ReturnsForAnyArgs (PathString("/")) |> ignore
+    ctx.Response.StatusCode.ReturnsForAnyArgs 205 |> ignore
+    ctx.Response.Body <- new MemoryStream()
+
+    task {
+        let! result = app (Some >> Task.FromResult) ctx
+
+        match result with
+        | None -> assertFail "Result was expected to be non-empty"
+        | Some ctx ->
+            Assert.True(ctx.Response.Headers["Content-Length"].ToString() = "0")
+    }
+
 [<Fact>]
 let ``WriteHtmlViewAsync should add html to the context`` () =
     let ctx = Substitute.For<HttpContext>()