From 5bf0b040fd3999d875efa55241e13ab91779233e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20Ro=CC=88sner?= Date: Wed, 5 Jul 2023 14:05:11 +0200 Subject: [PATCH] RFC-compliant Content-Length handling for 1xx, 204 and 205 responses and CONNECT requests --- src/Giraffe/Helpers.fs | 41 ++++++++ src/Giraffe/HttpContextExtensions.fs | 18 +++- .../HttpContextExtensionsTests.fs | 97 +++++++++++++++++++ 3 files changed, 154 insertions(+), 2 deletions(-) diff --git a/src/Giraffe/Helpers.fs b/src/Giraffe/Helpers.fs index bb7ba7e1..80ae55fc 100644 --- a/src/Giraffe/Helpers.fs +++ b/src/Giraffe/Helpers.fs @@ -34,3 +34,44 @@ module Helpers = use reader = new StreamReader(filePath) return! reader.ReadToEndAsync() } + + /// + /// Utility function for matching 1xx HTTP status codes. + /// + /// The HTTP status code. + /// Returns true if the status code is between 100 and 199. + let is1xxStatusCode (statusCode : int) = + 100 <= statusCode && statusCode <= 199 + + /// + /// Utility function for matching 2xx HTTP status codes. + /// + /// The HTTP status code. + /// Returns true if the status code is between 200 and 299. + let is2xxStatusCode (statusCode : int) = + 200 <= statusCode && statusCode <= 299 + + /// + /// Utility function for matching 3xx HTTP status codes. + /// + /// The HTTP status code. + /// Returns true if the status code is between 300 and 399. + let is3xxStatusCode (statusCode : int) = + 300 <= statusCode && statusCode <= 399 + + /// + /// Utility function for matching 4xx HTTP status codes. + /// + /// The HTTP status code. + /// Returns true if the status code is between 400 and 499. + let is4xxStatusCode (statusCode : int) = + 400 <= statusCode && statusCode <= 499 + + /// + /// Utility function for matching 5xx HTTP status codes. + /// + /// The HTTP status code. + /// Returns true if the status code is between 500 and 599. + let is5xxStatusCode (statusCode : int) = + 500 <= statusCode && statusCode <= 599 + diff --git a/src/Giraffe/HttpContextExtensions.fs b/src/Giraffe/HttpContextExtensions.fs index 7e069fc9..5b97ae29 100644 --- a/src/Giraffe/HttpContextExtensions.fs +++ b/src/Giraffe/HttpContextExtensions.fs @@ -334,7 +334,13 @@ type HttpContextExtensions() = } /// - /// 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.
+ ///
+ /// There are exceptions to be taken care of according to the RFC.
+ /// 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)
+ /// 2. Don't send non-zero Content-Length headers for 205 responses (https://httpwg.org/specs/rfc7231.html#rfc.section.6.3.6)
+ ///
+ /// Since .NET 7 these rules are enforced by Kestrel (https://github.com/dotnet/aspnetcore/pull/43103) /// /// The current http context object. /// The byte array to be send back to the client. @@ -342,7 +348,15 @@ type HttpContextExtensions() = [] 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 diff --git a/tests/Giraffe.Tests/HttpContextExtensionsTests.fs b/tests/Giraffe.Tests/HttpContextExtensionsTests.fs index 4b1c688a..5655af27 100644 --- a/tests/Giraffe.Tests/HttpContextExtensionsTests.fs +++ b/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) } +[] +let ``WriteBytesAsync should not return Content-Length in header on 100`` () = + let ctx = Substitute.For() + + 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"]) + } + +[] +let ``WriteBytesAsync should not return Content-Length in header on 204`` () = + let ctx = Substitute.For() + + 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"]) + } + +[] +let ``WriteBytesAsync with HTTP CONNECT should not return Content-Length in header on status code 200`` () = + let ctx = Substitute.For() + + 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"]) + } + +[] +let ``WriteBytesAsync should return Content-Length 0 in header on 205`` () = + let ctx = Substitute.For() + + 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") + } + [] let ``WriteHtmlViewAsync should add html to the context`` () = let ctx = Substitute.For()