diff --git a/src/Giraffe/HttpContextExtensions.fs b/src/Giraffe/HttpContextExtensions.fs index c4277ada..33e96764 100644 --- a/src/Giraffe/HttpContextExtensions.fs +++ b/src/Giraffe/HttpContextExtensions.fs @@ -203,7 +203,22 @@ type HttpContextExtensions() = [] static member ReadBodyFromRequestAsync(ctx: HttpContext) = task { - use reader = new StreamReader(ctx.Request.Body, Encoding.UTF8) + use reader = new StreamReader(ctx.Request.Body, Encoding.UTF8, leaveOpen = true) + return! reader.ReadToEndAsync() + } + + /// + /// Reads the entire body of the asynchronously and returns it as a value. This function let's you decide if you want to leave the ctx.Request.Body element open or not. + /// + /// The current http context object. + /// `true` to leave the stream open after the StreamReader object is disposed; otherwise, `false`. + /// Returns the contents of the request body as a . + [] + static member ReadBodyFromRequestAsync(ctx: HttpContext, leaveOpen: bool) = + task { + use reader = + new StreamReader(ctx.Request.Body, Encoding.UTF8, leaveOpen = leaveOpen) + return! reader.ReadToEndAsync() } diff --git a/tests/Giraffe.Tests/Helpers.fs b/tests/Giraffe.Tests/Helpers.fs index 674c9bb6..dfb10ef2 100644 --- a/tests/Giraffe.Tests/Helpers.fs +++ b/tests/Giraffe.Tests/Helpers.fs @@ -196,9 +196,14 @@ let hasLastModified (lastModified: DateTimeOffset) (response: HttpResponseMessag Assert.Equal(lastModified, response.Content.Headers.LastModified.Value) response +let getReqBody (ctx: HttpContext) = + ctx.Request.Body.Position <- 0L + use reader = new StreamReader(ctx.Request.Body, Encoding.UTF8, leaveOpen = true) + reader.ReadToEnd() + let getBody (ctx: HttpContext) = ctx.Response.Body.Position <- 0L - use reader = new StreamReader(ctx.Response.Body, Encoding.UTF8) + use reader = new StreamReader(ctx.Response.Body, Encoding.UTF8, leaveOpen = true) reader.ReadToEnd() let readText (response: HttpResponseMessage) = response.Content.ReadAsStringAsync() diff --git a/tests/Giraffe.Tests/HttpContextExtensionsTests.fs b/tests/Giraffe.Tests/HttpContextExtensionsTests.fs index 8eef39c1..d384f57c 100644 --- a/tests/Giraffe.Tests/HttpContextExtensionsTests.fs +++ b/tests/Giraffe.Tests/HttpContextExtensionsTests.fs @@ -289,3 +289,112 @@ let ``WriteHtmlViewAsync should add html to the context`` () = | None -> assertFailf "Result was expected to be %s" expected | Some ctx -> Assert.Equal(expected, getBody ctx) } + +[] +let ``ReadBodyFromRequestAsync is not incorrectly disposing the request body`` () = + let ctx = Substitute.For() + + let bodyStr = "Hello world!" + let mutable reqBodyDisposed = false + + let testHandler: HttpHandler = + fun (_: HttpFunc) (ctx: HttpContext) -> + task { + let! bodyFstRead = ctx.ReadBodyFromRequestAsync() + Assert.Equal(bodyStr, bodyFstRead) + Assert.False reqBodyDisposed + + // By the second time, the request body was read, so the position will be at the end of + // the stream. Due to it, we need to get back to the beginning of the stream: + ctx.Request.Body.Position <- 0L + let! bodySndRead = ctx.ReadBodyFromRequestAsync() + Assert.Equal(bodyStr, bodySndRead) + Assert.False reqBodyDisposed + + // If we don't change the position, it will simply return an empty string. + let! bodyThirdRead = ctx.ReadBodyFromRequestAsync() + Assert.Equal("", bodyThirdRead) + Assert.False reqBodyDisposed + + return! Encoding.UTF8.GetBytes bodyFstRead |> ctx.WriteBytesAsync + } + + let app = route "/" >=> testHandler + + ctx.Request.Method.ReturnsForAnyArgs "GET" |> ignore + ctx.Request.Path.ReturnsForAnyArgs(PathString("/")) |> ignore + + ctx.Request.Body <- + { new MemoryStream(buffer = Encoding.UTF8.GetBytes(bodyStr)) with + member this.Dispose(disposing: bool) : unit = + reqBodyDisposed <- true + base.Dispose(disposing: bool) + } + + ctx.Response.Body <- new MemoryStream() + + task { + let! result = app (Some >> Task.FromResult) ctx + + match result with + | None -> assertFail "Result was expected to be Option.Some" + | Some ctx -> + let ctxReqBody = getReqBody ctx + Assert.Equal(bodyStr, ctxReqBody) + Assert.False reqBodyDisposed // not disposed yet + + do ctx.Request.Body.Dispose() // "manually" disposed + Assert.True reqBodyDisposed // now it's disposed + } + +[] +let ``Old ReadBodyFromRequestAsync implementation was incorrectly disposing the request body`` () = + let ctx = Substitute.For() + + let bodyStr = "Hello world!" + let mutable reqBodyDisposed = false + let leaveOpen = false // the old default + + let testHandler: HttpHandler = + fun (_: HttpFunc) (ctx: HttpContext) -> + task { + Assert.False reqBodyDisposed + + let! bodyFstRead = ctx.ReadBodyFromRequestAsync(leaveOpen) + Assert.Equal(bodyStr, bodyFstRead) + Assert.True reqBodyDisposed + + let! capturedException = + Assert.ThrowsAsync(fun () -> ctx.ReadBodyFromRequestAsync(leaveOpen)) + + Assert.Equal("Stream was not readable.", capturedException.Message) + + return! Encoding.UTF8.GetBytes bodyFstRead |> ctx.WriteBytesAsync + } + + let app = route "/" >=> testHandler + + ctx.Request.Method.ReturnsForAnyArgs "GET" |> ignore + ctx.Request.Path.ReturnsForAnyArgs(PathString("/")) |> ignore + + ctx.Request.Body <- + { new MemoryStream(buffer = Encoding.UTF8.GetBytes(bodyStr)) with + member this.Dispose(disposing: bool) : unit = + reqBodyDisposed <- true + base.Dispose(disposing: bool) + } + + ctx.Response.Body <- new MemoryStream() + + task { + let! result = app (Some >> Task.FromResult) ctx + + match result with + | None -> assertFail "Result was expected to be Option.Some" + | Some ctx -> + let capturedException = + Assert.Throws(fun () -> getReqBody ctx |> ignore) + + Assert.Equal("Cannot access a closed Stream.", capturedException.Message) + Assert.True reqBodyDisposed + }