From 69a110d1857d0c985535b42937026ac206192d81 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 28 Apr 2015 16:35:40 +0100 Subject: [PATCH 1/5] RequestStream behavior changed to not buffer by default. Explicit buffering is done where required. Behavior to switch to a file buffer remains the same --- src/Nancy.Testing.Tests/BrowserFixture.cs | 4 +- src/Nancy.Testing/Browser.cs | 1 + src/Nancy.Testing/BrowserContextExtensions.cs | 2 + .../Unit/IO/RequestStreamFixture.cs | 119 +++--------- src/Nancy.Tests/Unit/RequestFixture.cs | 108 +++++------ src/Nancy/IO/RequestStream.cs | 180 +++++------------- .../JsonBodyDeserializer.cs | 7 +- src/Nancy/Request.cs | 2 + 8 files changed, 129 insertions(+), 294 deletions(-) diff --git a/src/Nancy.Testing.Tests/BrowserFixture.cs b/src/Nancy.Testing.Tests/BrowserFixture.cs index faabae6c0b..ad57c3f3da 100644 --- a/src/Nancy.Testing.Tests/BrowserFixture.cs +++ b/src/Nancy.Testing.Tests/BrowserFixture.cs @@ -131,7 +131,7 @@ public async Task Should_be_able_to_send_stream_in_body() var writer = new StreamWriter(stream); writer.Write(thisIsMyRequestBody); writer.Flush(); - + stream.Position = 0; // When var result = await browser.Post("/", with => { @@ -299,10 +299,12 @@ public async Task Should_be_able_to_continue_with_another_request() var firstRequestWriter = new StreamWriter(firstRequestStream); firstRequestWriter.Write(FirstRequestBody); firstRequestWriter.Flush(); + firstRequestStream.Position = 0; var secondRequestStream = new MemoryStream(); var secondRequestWriter = new StreamWriter(secondRequestStream); secondRequestWriter.Write(SecondRequestBody); secondRequestWriter.Flush(); + secondRequestStream.Position = 0; // When await browser.Post("/", with => diff --git a/src/Nancy.Testing/Browser.cs b/src/Nancy.Testing/Browser.cs index d74771c469..5acff9909b 100644 --- a/src/Nancy.Testing/Browser.cs +++ b/src/Nancy.Testing/Browser.cs @@ -325,6 +325,7 @@ private static Request CreateRequest(string method, Url url, IBrowserContextValu var requestStream = RequestStream.FromStream(contextValues.Body, 0, true); + requestStream.BufferStream(); var certBytes = (contextValues.ClientCertificate == null) ? new byte[] { } diff --git a/src/Nancy.Testing/BrowserContextExtensions.cs b/src/Nancy.Testing/BrowserContextExtensions.cs index ceb669d261..37a3ffb8e1 100644 --- a/src/Nancy.Testing/BrowserContextExtensions.cs +++ b/src/Nancy.Testing/BrowserContextExtensions.cs @@ -64,6 +64,7 @@ public static void JsonBody(this BrowserContext browserContext, TModel m contextValues.Body = new MemoryStream(); serializer.Serialize("application/json", model, contextValues.Body); + contextValues.Body.Position = 0; browserContext.Header("Content-Type", "application/json"); } @@ -86,6 +87,7 @@ public static void XMLBody(this BrowserContext browserContext, TModel mo contextValues.Body = new MemoryStream(); serializer.Serialize("application/xml", model, contextValues.Body); + contextValues.Body.Position = 0; browserContext.Header("Content-Type", "application/xml"); } diff --git a/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs b/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs index c2148b4157..1609121374 100644 --- a/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs +++ b/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs @@ -25,19 +25,6 @@ public void Should_not_dispose_wrapped_stream_when_not_switched() stream.HasBeenDisposed.ShouldBeFalse(); } - [Fact] - public void Should_move_non_seekable_stream_into_seekable_stream_when_stream_switching_is_disabled() - { - // Given - var stream = new ConfigurableMemoryStream(Seekable: false); - - // When - var result = RequestStream.FromStream(stream, 0, 1, true); - - // Then - result.CanSeek.ShouldBeTrue(); - } - [Fact] public void Should_move_stream_out_of_memory_if_longer_than_threshold_and_stream_switching_is_enabled() { @@ -46,6 +33,7 @@ public void Should_move_stream_out_of_memory_if_longer_than_threshold_and_stream // When var result = RequestStream.FromStream(inputStream, 0, 4, false); + result.BufferStream(); // Then result.IsInMemory.ShouldBeFalse(); @@ -59,6 +47,7 @@ public void Should_not_move_stream_out_of_memory_if_longer_than_threshold_and_st // When var result = RequestStream.FromStream(inputStream, 0, 4, true); + result.BufferStream(); // Then result.IsInMemory.ShouldBeTrue(); @@ -134,7 +123,7 @@ public void Should_return_true_when_queried_about_supporting_reading() } [Fact] - public void Should_return_true_when_queried_about_supporting_writing() + public void Should_return_false_when_queried_about_supporting_writing() { // Given var stream = new ConfigurableMemoryStream(); @@ -144,11 +133,11 @@ public void Should_return_true_when_queried_about_supporting_writing() var result = request.CanWrite; // Then - result.ShouldBeTrue(); + result.ShouldBeFalse(); } [Fact] - public void Should_return_true_when_queried_about_supporting_seeking() + public void Should_return_false_when_queried_about_supporting_seeking() { // Given var stream = new ConfigurableMemoryStream(); @@ -158,21 +147,21 @@ public void Should_return_true_when_queried_about_supporting_seeking() var result = request.CanSeek; // Then - result.ShouldBeTrue(); + result.ShouldBeFalse(); } [Fact] - public void Should_return_false_when_queried_about_supporting_timeout() + public void Should_return_underlaying_stream_when_queried_about_supporting_timeout() { // Given - var stream = new ConfigurableMemoryStream(); + var stream = new ConfigurableMemoryStream(Timeoutable: true); var request = RequestStream.FromStream(stream, 0, 1, false); // When var result = request.CanTimeout; // Then - result.ShouldBeFalse(); + result.ShouldBeTrue(); } [Fact] @@ -247,17 +236,17 @@ public void Should_throw_invalidoperationexception_when_position_is_set_to_great } [Fact] - public void Should_flush_underlaying_stream() + public void Should_throw_on_flush() { // Given var stream = new ConfigurableMemoryStream(); var request = RequestStream.FromStream(stream, 0, 1, false); // When - request.Flush(); + var exception = Record.Exception(() => request.Flush()); // Then - stream.HasBeenFlushed.ShouldBeTrue(); + exception.ShouldBeOfType(); } [Fact] @@ -275,45 +264,17 @@ public void Should_throw_notsupportedexception_when_setting_length() } [Fact] - public void Should_set_position_of_underlaying_stream_to_zero_when_created() - { - // Given - var stream = new ConfigurableMemoryStream(Position: 10L); - - // When - var request = RequestStream.FromStream(stream, 0, 1, false); - - // Then - stream.Position.ShouldEqual(0L); - } - - [Fact] - public void Should_seek_in_the_underlaying_stream_when_seek_is_called() + public void Should_throw_when_seek_is_called() { // Given var stream = new ConfigurableMemoryStream(); var request = RequestStream.FromStream(stream, 0, 1, false); // When - request.Seek(10L, SeekOrigin.Current); - - // Then - stream.HasBeenSeeked.ShouldBeTrue(); - } - - [Fact] - public void Should_return_the_new_position_of_the_underlaying_stream_when_seek_is_called() - { - // Given - var stream = CreateFakeStream(); - A.CallTo(() => stream.Seek(A.Ignored, A.Ignored)).Returns(100L); - var request = RequestStream.FromStream(stream, 0, 1, false); - - // When - var result = request.Seek(10L, SeekOrigin.Current); + var exception = Record.Exception(() => request.Seek(10L, SeekOrigin.Current)); // Then - result.ShouldEqual(100L); + exception.ShouldBeOfType(); } [Fact] @@ -377,7 +338,7 @@ public void Should_return_result_from_reading_underlaying_stream() } [Fact] - public void Should_write_to_underlaying_stream_when_write_is_called() + public void Should_throw_when_write_is_called() { // Given var stream = CreateFakeStream(); @@ -385,10 +346,10 @@ public void Should_write_to_underlaying_stream_when_write_is_called() var request = RequestStream.FromStream(stream, 0, 1, false); // When - request.Write(buffer, 0, buffer.Length); + var exception = Record.Exception(() => request.Write(buffer, 0, buffer.Length)); // Then - A.CallTo(() => stream.Write(buffer, 0, buffer.Length)).MustHaveHappened(); + exception.ShouldBeOfType(); } [Fact] @@ -396,46 +357,14 @@ public void Should_no_longer_be_in_memory_if_expected_length_is_greater_or_equal { // Given var stream = CreateFakeStream(); - - // When - var request = RequestStream.FromStream(stream, 1, 0, false); - - // Then - request.IsInMemory.ShouldBeFalse(); - } - - [Fact] - public void Should_no_longer_be_in_memory_when_more_bytes_have_been_written_to_stream_then_size_of_the_threshold_and_stream_swapping_is_enabled() - { - // Given - var stream = CreateFakeStream(); - var buffer = new byte[100]; - var request = RequestStream.FromStream(stream, 0, 10, false); - A.CallTo(() => stream.Length).Returns(100); // When - request.Write(buffer, 0, buffer.Length); + var request = RequestStream.FromStream(stream, 1, 0, false); // Then request.IsInMemory.ShouldBeFalse(); } - [Fact] - public void Should_still_be_in_memory_when_more_bytes_have_been_written_to_stream_than_size_of_threshold_and_stream_swapping_is_disabled() - { - // Given - var stream = CreateFakeStream(); - var buffer = new byte[100]; - var request = RequestStream.FromStream(stream, 0, 10, true); - A.CallTo(() => stream.Length).Returns(100); - - // When - request.Write(buffer, 0, buffer.Length); - - // Then - request.IsInMemory.ShouldBeTrue(); - } - [Fact] public void Should_call_beginread_on_underlaying_stream_when_beginread_is_called() { @@ -540,7 +469,7 @@ public void Should_return_result_from_underlaying_endread_when_endread_is_called } [Fact] - public void Should_call_endwrite_on_underlaying_stream_when_endwrite_is_called() + public void Should_throw_when_endwrite_is_called() { // Given var stream = CreateFakeStream(); @@ -548,10 +477,10 @@ public void Should_call_endwrite_on_underlaying_stream_when_endwrite_is_called() var request = RequestStream.FromStream(stream, 0, 10, true); // When - request.EndWrite(asyncResult); + var exception = Record.Exception(() => request.EndWrite(asyncResult)); // Then - A.CallTo(() => stream.EndWrite(asyncResult)).MustHaveHappened(); + exception.ShouldBeOfType(); } private static Stream CreateFakeStream() @@ -565,7 +494,7 @@ private static Stream CreateFakeStream() A.CallTo(() => stream.CanSeek).Returns(true); A.CallTo(() => stream.CanTimeout).Returns(true); A.CallTo(() => stream.CanWrite).Returns(true); - + return stream; } @@ -620,7 +549,7 @@ public override long Length public override long Position { get { return this.position; } - set { this.position = value ; } + set { this.position = value; } } protected override void Dispose(bool disposing) diff --git a/src/Nancy.Tests/Unit/RequestFixture.cs b/src/Nancy.Tests/Unit/RequestFixture.cs index 0c632a4422..0326cb6a75 100644 --- a/src/Nancy.Tests/Unit/RequestFixture.cs +++ b/src/Nancy.Tests/Unit/RequestFixture.cs @@ -50,11 +50,7 @@ public void Should_override_request_method_on_post() { // Given const string bodyContent = "_method=GET"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> { { "content-type", new[] { "application/x-www-form-urlencoded" } } }; @@ -75,11 +71,7 @@ public void Should_only_override_method_on_post(string method) { // Given const string bodyContent = "_method=TEST"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> { { "content-type", new[] { "application/x-www-form-urlencoded" } } }; @@ -160,7 +152,7 @@ public void Should_set_header_parameter_value_to_header_property_when_initialize }; // When - var request = new Request("GET", new Url { Path = "/", Scheme = "http" }, CreateRequestStream(), headers); + var request = new Request("GET", new Url { Path = "/", Scheme = "http" }, CreateRequestStream(new MemoryStream()), headers); // Then request.Headers.ContentType.ShouldNotBeEmpty(); @@ -170,7 +162,7 @@ public void Should_set_header_parameter_value_to_header_property_when_initialize public void Should_set_body_parameter_value_to_body_property_when_initialized() { // Given - var body = CreateRequestStream(); + var body = CreateRequestStream(new MemoryStream()); // When var request = new Request("GET", new Url { Path = "/", Scheme = "http" }, body, new Dictionary>()); @@ -184,11 +176,7 @@ public void Should_set_extract_form_data_from_body_when_content_type_is_x_www_fo { // Given const string bodyContent = "name=John+Doe&gender=male&family=5&city=kent&city=miami&other=abc%0D%0Adef&nickname=J%26D"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> @@ -208,11 +196,7 @@ public void Should_set_extract_form_data_from_body_when_content_type_is_x_www_fo { // Given const string bodyContent = "name=John+Doe&gender=male&family=5&city=kent&city=miami&other=abc%0D%0Adef&nickname=J%26D"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> @@ -258,11 +242,7 @@ public void Should_respect_case_insensitivity_when_extracting_form_data_from_bod // Given StaticConfiguration.CaseSensitive = false; const string bodyContent = "key=value&key=value&KEY=VALUE"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> @@ -284,11 +264,7 @@ public void Should_respect_case_sensitivity_when_extracting_form_data_from_body_ // Given StaticConfiguration.CaseSensitive = true; const string bodyContent = "key=value&key=value&KEY=VALUE"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> @@ -495,7 +471,7 @@ public void Should_be_able_to_invoke_form_repeatedly() // When var request = new Request("POST", new Url { Path = "/", Scheme = "http" }, CreateRequestStream(memory), headers); - + // Then ((string)request.Form.name).ShouldEqual("John Doe"); } @@ -562,7 +538,7 @@ public void Should_split_cookie_in_two_parts_with_secure_attribute() const string cookieName = "path"; const string cookieData = "/"; var headers = new Dictionary>(); - var cookies = new List { string.Format("{0}={1}; Secure", cookieName, cookieData)} ; + var cookies = new List { string.Format("{0}={1}; Secure", cookieName, cookieData) }; headers.Add("cookie", cookies); var newUrl = new Url { @@ -571,7 +547,7 @@ public void Should_split_cookie_in_two_parts_with_secure_attribute() var request = new Request("GET", newUrl, null, headers); // Then - request.Cookies[cookieName].ShouldEqual(cookieData); + request.Cookies[cookieName].ShouldEqual(cookieData); } [Fact] @@ -634,22 +610,22 @@ public void Should_split_cookie_in_two_parts_with_httponly_attribute() [Fact] public void Should_add_attribute_in_cookie_as_empty_value() { - // Given, when - const string cookieName = "path"; - const string cookieData = "/"; - const string cookieAttribute = "SomeAttribute"; - var headers = new Dictionary>(); - var cookies = new List { string.Format("{0}={1}; {2}", cookieName, cookieData, cookieAttribute) }; - headers.Add("cookie", cookies); - var newUrl = new Url - { - Path = "/" - }; - var request = new Request("GET", newUrl, null, headers); + // Given, when + const string cookieName = "path"; + const string cookieData = "/"; + const string cookieAttribute = "SomeAttribute"; + var headers = new Dictionary>(); + var cookies = new List { string.Format("{0}={1}; {2}", cookieName, cookieData, cookieAttribute) }; + headers.Add("cookie", cookies); + var newUrl = new Url + { + Path = "/" + }; + var request = new Request("GET", newUrl, null, headers); - // Then - request.Cookies[cookieName].ShouldEqual(cookieData); - request.Cookies[cookieAttribute].ShouldEqual(string.Empty); + // Then + request.Cookies[cookieName].ShouldEqual(cookieData); + request.Cookies[cookieAttribute].ShouldEqual(string.Empty); } [Fact] @@ -657,11 +633,7 @@ public void Should_move_request_body_position_to_zero_after_parsing_url_encoded_ { // Given const string bodyContent = "name=John+Doe&gender=male&family=5&city=kent&city=miami&other=abc%0D%0Adef&nickname=J%26D"; - var memory = CreateRequestStream(); - var writer = new StreamWriter(memory); - writer.Write(bodyContent); - writer.Flush(); - memory.Position = 0; + var memory = CreateRequestStream(bodyContent); var headers = new Dictionary> @@ -696,7 +668,7 @@ public void Should_move_request_body_position_to_zero_after_parsing_multipart_en var request = new Request("POST", new Url { Path = "/", Scheme = "http" }, CreateRequestStream(memory), headers); // Then - memory.Position.ShouldEqual(0L); + request.Body.Position.ShouldEqual(0L); } [Fact] @@ -737,11 +709,12 @@ public void Should_limit_the_amount_of_form_fields_parsed() sb.AppendFormat("Field{0}=Value{0}", i); } - var memory = CreateRequestStream(); + var memory = new MemoryStream(); var writer = new StreamWriter(memory); writer.Write(sb.ToString()); writer.Flush(); memory.Position = 0; + var requestStream = CreateRequestStream(memory); var headers = new Dictionary> @@ -750,7 +723,7 @@ public void Should_limit_the_amount_of_form_fields_parsed() }; // When - var request = new Request("POST", new Url { Path = "/", Scheme = "http" }, memory, headers); + var request = new Request("POST", new Url { Path = "/", Scheme = "http" }, requestStream, headers); // Then ((IEnumerable)request.Form.GetDynamicMemberNames()).Count().ShouldEqual(StaticConfiguration.RequestQueryFormMultipartLimit); @@ -770,10 +743,10 @@ public void Should_limit_the_amount_of_querystring_fields_parsed() sb.AppendFormat("Field{0}=Value{0}", i); } - var memory = CreateRequestStream(); + var requestStream = CreateRequestStream(new MemoryStream()); // When - var request = new Request("GET", new Url { Path = "/", Scheme = "http", Query = sb.ToString() }, memory, new Dictionary>()); + var request = new Request("GET", new Url { Path = "/", Scheme = "http", Query = sb.ToString() }, requestStream, new Dictionary>()); // Then ((IEnumerable)request.Query.GetDynamicMemberNames()).Count().ShouldEqual(StaticConfiguration.RequestQueryFormMultipartLimit); @@ -791,21 +764,21 @@ public void Should_change_empty_path_to_root() public void Should_replace_value_of_query_key_without_value_with_true() { // Given - var memory = CreateRequestStream(); + var memory = CreateRequestStream(new MemoryStream()); // When var request = new Request("GET", new Url { Path = "/", Scheme = "http", Query = "key1" }, memory); // Then ((bool)request.Query.key1).ShouldBeTrue(); - ((string)request.Query.key1).ShouldEqual("key1"); + ((string)request.Query.key1).ShouldEqual("key1"); } [Fact] public void Should_not_replace_equal_key_value_query_with_bool() { // Given - var memory = CreateRequestStream(); + var memory = CreateRequestStream(new MemoryStream()); // When var request = new Request("GET", new Url { Path = "/", Scheme = "http", Query = "key1=key1" }, memory); @@ -814,9 +787,14 @@ public void Should_not_replace_equal_key_value_query_with_bool() ShouldAssertExtensions.ShouldBeOfType(request.Query["key1"].Value); } - private static RequestStream CreateRequestStream() + private static RequestStream CreateRequestStream(string bodyContent) { - return CreateRequestStream(new MemoryStream()); + var memory = new MemoryStream(); + var writer = new StreamWriter(memory); + writer.Write(bodyContent); + writer.Flush(); + memory.Position = 0; + return RequestStream.FromStream(memory); } private static RequestStream CreateRequestStream(Stream stream) diff --git a/src/Nancy/IO/RequestStream.cs b/src/Nancy/IO/RequestStream.cs index d1339071c3..60030b7f5d 100644 --- a/src/Nancy/IO/RequestStream.cs +++ b/src/Nancy/IO/RequestStream.cs @@ -2,7 +2,6 @@ { using System; using System.IO; - using System.Threading.Tasks; using Nancy.Extensions; @@ -15,7 +14,7 @@ public class RequestStream : Stream private bool disableStreamSwitching; private readonly long thresholdLength; - private bool isSafeToDisposeStream; + private bool isBuffered; private Stream stream; /// @@ -46,7 +45,7 @@ public RequestStream(Stream stream, long expectedLength, bool disableStreamSwitc /// The expected length of the contents in the stream. /// if set to the stream will never explicitly be moved to disk. public RequestStream(long expectedLength, bool disableStreamSwitching) - : this(null, expectedLength, DEFAULT_SWITCHOVER_THRESHOLD, disableStreamSwitching) + : this(new MemoryStream(), expectedLength, DEFAULT_SWITCHOVER_THRESHOLD, disableStreamSwitching) { } @@ -59,39 +58,57 @@ public RequestStream(long expectedLength, bool disableStreamSwitching) /// if set to the stream will never explicitly be moved to disk. public RequestStream(Stream stream, long expectedLength, long thresholdLength, bool disableStreamSwitching) { + if (stream == null) + { + throw new ArgumentNullException("stream"); + } this.thresholdLength = thresholdLength; this.disableStreamSwitching = disableStreamSwitching; - this.stream = stream ?? this.CreateDefaultMemoryStream(expectedLength); + this.stream = stream; ThrowExceptionIfCtorParametersWereInvalid(this.stream, expectedLength, this.thresholdLength); + } - if (!this.MoveStreamOutOfMemoryIfExpectedLengthExceedSwitchLength(expectedLength)) + public void BufferStream() + { + if (isBuffered) { - this.MoveStreamOutOfMemoryIfContentsLengthExceedThresholdAndSwitchingIsEnabled(); + //already buffered + return; } + this.isBuffered = true; + var sourceStream = this.stream; + var buffer = new byte[StreamExtensions.BufferSize]; + int read = 0; + this.stream = new MemoryStream(StreamExtensions.BufferSize); - if (!this.stream.CanSeek) + bool bufferToDisk = false; + while ((read = sourceStream.Read(buffer, 0, buffer.Length)) > 0) { - var task = - MoveToWritableStream(); - - task.Wait(); - - if (task.IsFaulted) + this.stream.Write(buffer, 0, read); + if (!disableStreamSwitching && this.stream.Length > thresholdLength) { - throw new InvalidOperationException("Unable to copy stream", task.Exception); + bufferToDisk = true; + break; } } - this.stream.Position = 0; - } - - private Task MoveToWritableStream() - { - var sourceStream = this.stream; - this.stream = new MemoryStream(StreamExtensions.BufferSize); - - return sourceStream.CopyToAsync(this); + if (bufferToDisk) + { + var fileStream = CreateTemporaryFileStream(); + //copy the memory stream to disk + while ((read = this.stream.Read(buffer, 0, buffer.Length)) > 0) + { + fileStream.Write(buffer, 0, read); + } + //now finish copying the request stream to disk + while ((read = sourceStream.Read(buffer, 0, buffer.Length)) > 0) + { + fileStream.Write(buffer, 0, read); + } + fileStream.Position = 0; + this.stream = fileStream; + } } /// @@ -106,10 +123,10 @@ public override bool CanRead /// /// Gets a value indicating whether the current stream supports seeking. /// - /// Always returns . + /// Always returns . public override bool CanSeek { - get { return this.stream.CanSeek; } + get { return false; } } /// @@ -118,16 +135,16 @@ public override bool CanSeek /// Always returns . public override bool CanTimeout { - get { return false; } + get { return this.stream.CanTimeout; } } /// /// Gets a value indicating whether the current stream supports writing. /// - /// Always returns . + /// Always returns . public override bool CanWrite { - get { return true; } + get { return false; } } /// @@ -146,7 +163,7 @@ public override long Length /// The stream is moved to disk when either the length of the contents or expected content length exceeds the threshold specified in the constructor. public bool IsInMemory { - get { return !(this.stream.GetType() == typeof(FileStream)); } + get { return isBuffered && !(this.stream.GetType() == typeof(FileStream)); } } /// @@ -198,7 +215,7 @@ public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, As protected override void Dispose(bool disposing) { - if (this.isSafeToDisposeStream) + if (this.isBuffered) { ((IDisposable)this.stream).Dispose(); @@ -230,9 +247,7 @@ public override int EndRead(IAsyncResult asyncResult) /// A reference to the outstanding asynchronous I/O request. public override void EndWrite(IAsyncResult asyncResult) { - this.stream.EndWrite(asyncResult); - - this.ShiftStreamToFileStreamIfNecessary(); + throw new NotSupportedException(); } /// @@ -240,7 +255,7 @@ public override void EndWrite(IAsyncResult asyncResult) /// public override void Flush() { - this.stream.Flush(); + throw new NotSupportedException(); } public static RequestStream FromStream(Stream stream) @@ -297,7 +312,7 @@ public override int ReadByte() /// A value of type indicating the reference point used to obtain the new position. public override long Seek(long offset, SeekOrigin origin) { - return this.stream.Seek(offset, origin); + throw new NotSupportedException(); } /// @@ -319,27 +334,7 @@ public override void SetLength(long value) /// The number of bytes to be written to the current stream. public override void Write(byte[] buffer, int offset, int count) { - this.stream.Write(buffer, offset, count); - - this.ShiftStreamToFileStreamIfNecessary(); - } - - private void ShiftStreamToFileStreamIfNecessary() - { - if (this.disableStreamSwitching) - { - return; - } - - if (this.stream.Length >= this.thresholdLength) - { - // Close the stream here as closing it every time we call - // MoveStreamContentsToFileStream causes an (ObjectDisposedException) - // in NancyWcfGenericService - webRequest.UriTemplateMatch - var old = this.stream; - this.MoveStreamContentsToFileStream(); - old.Close(); - } + throw new NotSupportedException(); } private static FileStream CreateTemporaryFileStream() @@ -361,20 +356,6 @@ private static FileStream CreateTemporaryFileStream() StaticConfiguration.AllowFileStreamUploadAsync); } - private Stream CreateDefaultMemoryStream(long expectedLength) - { - this.isSafeToDisposeStream = true; - - if (this.disableStreamSwitching || expectedLength < this.thresholdLength) - { - return new MemoryStream((int)expectedLength); - } - - this.disableStreamSwitching = true; - - return CreateTemporaryFileStream(); - } - private static void DeleteTemporaryFile(string fileName) { if (string.IsNullOrEmpty(fileName) || !File.Exists(fileName)) @@ -391,65 +372,6 @@ private static void DeleteTemporaryFile(string fileName) } } - private void MoveStreamOutOfMemoryIfContentsLengthExceedThresholdAndSwitchingIsEnabled() - { - if (!this.stream.CanSeek) - { - return; - } - - try - { - if ((this.stream.Length > this.thresholdLength) && !this.disableStreamSwitching) - { - this.MoveStreamContentsToFileStream(); - } - } - catch (NotSupportedException) - { - } - } - - private bool MoveStreamOutOfMemoryIfExpectedLengthExceedSwitchLength(long expectedLength) - { - if ((expectedLength >= this.thresholdLength) && !this.disableStreamSwitching) - { - this.MoveStreamContentsToFileStream(); - return true; - } - return false; - } - - private void MoveStreamContentsToFileStream() - { - var targetStream = CreateTemporaryFileStream(); - this.isSafeToDisposeStream = true; - - if (this.stream.CanSeek && this.stream.Length == 0) - { - this.stream.Close(); - this.stream = targetStream; - return; - } - - // Seek to 0 if we can, although if we can't seek, and we've already written (if the size is unknown) then - // we are screwed anyway, and some streams that don't support seek also don't let you read the position so - // there's no real way to check :-/ - if (this.stream.CanSeek) - { - this.stream.Position = 0; - } - this.stream.CopyTo(targetStream, 8196); - if (this.stream.CanSeek) - { - this.stream.Flush(); - } - - this.stream = targetStream; - - this.disableStreamSwitching = true; - } - private static void ThrowExceptionIfCtorParametersWereInvalid(Stream stream, long expectedLength, long thresholdLength) { if (!stream.CanRead) diff --git a/src/Nancy/ModelBinding/DefaultBodyDeserializers/JsonBodyDeserializer.cs b/src/Nancy/ModelBinding/DefaultBodyDeserializers/JsonBodyDeserializer.cs index cd85670346..9e3c7743ba 100644 --- a/src/Nancy/ModelBinding/DefaultBodyDeserializers/JsonBodyDeserializer.cs +++ b/src/Nancy/ModelBinding/DefaultBodyDeserializers/JsonBodyDeserializer.cs @@ -58,10 +58,9 @@ public object Deserialize(MediaRange mediaRange, Stream bodyStream, BindingConte bodyStream.Position = 0; string bodyText; - using (var bodyReader = new StreamReader(bodyStream)) - { - bodyText = bodyReader.ReadToEnd(); - } + var bodyReader = new StreamReader(bodyStream); + bodyText = bodyReader.ReadToEnd(); + bodyStream.Position = 0; var genericDeserializeMethod = this.deserializeMethod.MakeGenericMethod(context.DestinationType); diff --git a/src/Nancy/Request.cs b/src/Nancy/Request.cs index a978f67da4..85dfeb448d 100644 --- a/src/Nancy/Request.cs +++ b/src/Nancy/Request.cs @@ -243,6 +243,7 @@ private void ParseFormData() var mimeType = contentType.Split(';').First(); if (mimeType.Equals("application/x-www-form-urlencoded", StringComparison.OrdinalIgnoreCase)) { + this.Body.BufferStream(); var reader = new StreamReader(this.Body); this.form = reader.ReadToEnd().AsQueryDictionary(); this.Body.Position = 0; @@ -253,6 +254,7 @@ private void ParseFormData() return; } + this.Body.BufferStream(); var boundary = Regex.Match(contentType, @"boundary=""?(?[^\n\;\"" ]*)").Groups["token"].Value; var multipart = new HttpMultipart(this.Body, boundary); From 70b7861b559764c1d353fdc725505a1017b38002 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Thu, 30 Apr 2015 15:19:13 +0100 Subject: [PATCH 2/5] Be lazy about form and multipart access. --- src/Nancy/Request.cs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/Nancy/Request.cs b/src/Nancy/Request.cs index 85dfeb448d..daa9d70676 100644 --- a/src/Nancy/Request.cs +++ b/src/Nancy/Request.cs @@ -21,6 +21,7 @@ public class Request : IDisposable { private readonly List files = new List(); private dynamic form = new DynamicDictionary(); + private bool parsedFormData; private IDictionary cookies; @@ -98,8 +99,6 @@ public Request(string method, { this.Url.Path = "/"; } - - this.ParseFormData(); this.RewriteMethod(); } @@ -207,7 +206,11 @@ private IDictionary GetCookieData() /// An instance, containing an instance for each uploaded file. public IEnumerable Files { - get { return this.files; } + get + { + ParseFormData(); + return this.files; + } } /// @@ -217,7 +220,11 @@ public IEnumerable Files /// Currently Nancy will only parse form data sent using the application/x-www-url-encoded mime-type. public dynamic Form { - get { return this.form; } + get + { + ParseFormData(); + return this.form; + } } /// @@ -234,6 +241,10 @@ public void Dispose() private void ParseFormData() { + if (parsedFormData) + { + return; + } if (string.IsNullOrEmpty(this.Headers.ContentType)) { return; @@ -247,6 +258,7 @@ private void ParseFormData() var reader = new StreamReader(this.Body); this.form = reader.ReadToEnd().AsQueryDictionary(); this.Body.Position = 0; + parsedFormData = true; } if (!mimeType.Equals("multipart/form-data", StringComparison.OrdinalIgnoreCase)) @@ -282,6 +294,7 @@ private void ParseFormData() } this.Body.Position = 0; + parsedFormData = true; } private void RewriteMethod() From 1993a135076e26baa7bcb9222be6ebeb266baffd Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Thu, 30 Apr 2015 16:35:36 +0100 Subject: [PATCH 3/5] Add flag to stop always checking the form body for the override flag --- src/Nancy/Request.cs | 7 +++++-- src/Nancy/StaticConfiguration.cs | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Nancy/Request.cs b/src/Nancy/Request.cs index daa9d70676..f25f98c929 100644 --- a/src/Nancy/Request.cs +++ b/src/Nancy/Request.cs @@ -307,10 +307,13 @@ private void RewriteMethod() var overrides = new List> { - Tuple.Create("_method form input element", (string)this.Form["_method"]), - Tuple.Create("X-HTTP-Method-Override form input element", (string)this.Form["X-HTTP-Method-Override"]), Tuple.Create("X-HTTP-Method-Override header", this.Headers["X-HTTP-Method-Override"].FirstOrDefault()) }; + if (!StaticConfiguration.DisableXHttpMethodOverrideBodyDiscovery) + { + overrides.Add(Tuple.Create("_method form input element", (string)this.Form["_method"])); + overrides.Add(Tuple.Create("X-HTTP-Method-Override form input element", (string)this.Form["X-HTTP-Method-Override"])); + } var providedOverride = overrides.Where(x => !string.IsNullOrEmpty(x.Item2)) diff --git a/src/Nancy/StaticConfiguration.cs b/src/Nancy/StaticConfiguration.cs index dbcbd656b8..f722cfadc7 100644 --- a/src/Nancy/StaticConfiguration.cs +++ b/src/Nancy/StaticConfiguration.cs @@ -55,6 +55,14 @@ public static bool DisableErrorTraces [Description("Enables explicit HEAD routing and disables the usage of GET routes for HEAD requests.")] public static bool EnableHeadRouting { get; set; } + /// + /// Gets or sets a value indicating whether or not to disable the discovery of the X-HTTP-Method-Override flag in a request body. + /// This can be in the headers or posted form body of a request. This flag will disable the discovery of this + /// flag in the body of a request to avoid buffering the request body. + /// + [Description("Gets or sets a value indicating whether or not to disable the discovery of the X-HTTP-Method-Override flag in a request body.")] + public static bool DisableXHttpMethodOverrideBodyDiscovery { get; set; } + /// /// Gets a value indicating whether we are running in debug mode or not. /// Checks the entry assembly to see whether it has been built in debug mode. From 53b5f3fb5591cb6c0c547b9a5d32f4346fdf943c Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 26 May 2015 12:18:47 +0100 Subject: [PATCH 4/5] Allow seekable methods to be used and let the current underlying stream manage that. --- src/Nancy/IO/RequestStream.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Nancy/IO/RequestStream.cs b/src/Nancy/IO/RequestStream.cs index 60030b7f5d..607c0be8e7 100644 --- a/src/Nancy/IO/RequestStream.cs +++ b/src/Nancy/IO/RequestStream.cs @@ -123,10 +123,10 @@ public override bool CanRead /// /// Gets a value indicating whether the current stream supports seeking. /// - /// Always returns . + /// Returns depending on whether the stream is buffered or not. public override bool CanSeek { - get { return false; } + get { return this.stream.CanSeek; } } /// @@ -312,7 +312,7 @@ public override int ReadByte() /// A value of type indicating the reference point used to obtain the new position. public override long Seek(long offset, SeekOrigin origin) { - throw new NotSupportedException(); + return this.stream.Seek(offset, origin); } /// @@ -323,7 +323,7 @@ public override long Seek(long offset, SeekOrigin origin) /// This functionality is not supported by the type and will always throw . public override void SetLength(long value) { - throw new NotSupportedException(); + this.stream.SetLength(value); } /// From 700b66ffb0de10c3ec0c62c13cc0882cfdac2f10 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Wed, 27 May 2015 15:37:54 +0100 Subject: [PATCH 5/5] Only use underlying stream when buffered. Otherwise, it's not seekable. --- src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs | 15 +++++++++++++++ src/Nancy/IO/RequestStream.cs | 17 ++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs b/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs index 1609121374..ea8662fdba 100644 --- a/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs +++ b/src/Nancy.Tests/Unit/IO/RequestStreamFixture.cs @@ -150,6 +150,21 @@ public void Should_return_false_when_queried_about_supporting_seeking() result.ShouldBeFalse(); } + [Fact] + public void Should_return_true_when_queried_about_supporting_seeking_if_buffered() + { + // Given + var stream = new ConfigurableMemoryStream(); + var request = RequestStream.FromStream(stream, 0, 1, false); + request.BufferStream(); + + // When + var result = request.CanSeek; + + // Then + result.ShouldBeTrue(); + } + [Fact] public void Should_return_underlaying_stream_when_queried_about_supporting_timeout() { diff --git a/src/Nancy/IO/RequestStream.cs b/src/Nancy/IO/RequestStream.cs index 607c0be8e7..9822a22983 100644 --- a/src/Nancy/IO/RequestStream.cs +++ b/src/Nancy/IO/RequestStream.cs @@ -126,7 +126,14 @@ public override bool CanRead /// Returns depending on whether the stream is buffered or not. public override bool CanSeek { - get { return this.stream.CanSeek; } + get + { + if (!isBuffered) + { + return false; + } + return this.stream.CanSeek; + } } /// @@ -312,6 +319,10 @@ public override int ReadByte() /// A value of type indicating the reference point used to obtain the new position. public override long Seek(long offset, SeekOrigin origin) { + if (!isBuffered) + { + throw new NotSupportedException(); + } return this.stream.Seek(offset, origin); } @@ -323,6 +334,10 @@ public override long Seek(long offset, SeekOrigin origin) /// This functionality is not supported by the type and will always throw . public override void SetLength(long value) { + if (!isBuffered) + { + throw new NotSupportedException(); + } this.stream.SetLength(value); }