Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions src/Giraffe/Helpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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

18 changes: 16 additions & 2 deletions src/Giraffe/HttpContextExtensions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
97 changes: 97 additions & 0 deletions tests/Giraffe.Tests/HttpContextExtensionsTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>()
Expand Down