From 85170dada3d329edf02cb64fafd19575e8ea0c7b Mon Sep 17 00:00:00 2001 From: neil Date: Thu, 22 Mar 2018 12:33:43 +0000 Subject: [PATCH] Test & fix issue closing http listener response - Need to close the response - Wrote some tests around serving IFile via HttpListener - Fixed a TODO with setting content length header in ApplicationOctetStreamCodec --- ...plicationOctetStreamCodec_Specification.cs | 49 +++---- .../media_type_reader_context.cs | 1 - .../ApplicationOctetStreamCodec.Sync.cs | 28 ++-- .../ApplicationOctetStreamCodec.cs | 125 +++++++++--------- .../Hosting/HttpListener/HttpListenerHost.cs | 2 + src/Tests/Hosting.HttpListener/disposing.cs | 2 +- .../Hosting.HttpListener/serving_files.cs | 58 ++++++++ src/Tests/MemoryHostExtensions.cs | 2 +- .../LocationHeader/relative_uri.cs | 55 +++++++- .../Resource/get_with_dependency.cs | 1 - ...on_with_logging_from_http_listener_host.cs | 56 +++----- src/Tests/TestHttpListener.cs | 51 +++++++ 12 files changed, 290 insertions(+), 140 deletions(-) create mode 100644 src/Tests/Hosting.HttpListener/serving_files.cs create mode 100644 src/Tests/TestHttpListener.cs diff --git a/src/OpenRasta.Tests.Unit/Codecs/ApplicationOctetStreamCodec_Specification.cs b/src/OpenRasta.Tests.Unit/Codecs/ApplicationOctetStreamCodec_Specification.cs index 24105d00..dfc9cd0e 100644 --- a/src/OpenRasta.Tests.Unit/Codecs/ApplicationOctetStreamCodec_Specification.cs +++ b/src/OpenRasta.Tests.Unit/Codecs/ApplicationOctetStreamCodec_Specification.cs @@ -10,12 +10,8 @@ #endregion -using System; -using System.Collections.Generic; -using System.Linq; using System.Text; using OpenRasta.Codecs; -using OpenRasta.TypeSystem.ReflectionBased; using OpenRasta.Web; using NUnit.Framework; using OpenRasta.IO; @@ -29,60 +25,60 @@ namespace ApplicationOctetStreamCodec_Specification public class when_converting_a_byte_stream_to_an_ifile : applicationoctetstream_context { [Test] - public void an_ifile_object_is_generated() + public async Task an_ifile_object_is_generated() { given_context(); given_request_entity_stream(); - when_decoding(); + await when_decoding(); ThenTheResult .ShouldNotBeNull(); } [Test] - public void the_length_is_set_to_the_proper_value() + public async Task the_length_is_set_to_the_proper_value() { given_context(); given_request_entity_stream(1000); - when_decoding(); + await when_decoding(); ThenTheResult.Length.ShouldBe(1000); } [Test] - public void an_ireceivedfile_object_is_generated() + public async Task an_ireceivedfile_object_is_generated() { given_context(); given_request_entity_stream(); - when_decoding(); + await when_decoding(); ThenTheResult .ShouldNotBeNull(); } [Test] - public void the_file_name_is_null_if_no_content_disposition_header_is_present() + public async Task the_file_name_is_null_if_no_content_disposition_header_is_present() { given_context(); given_request_entity_stream(); given_content_disposition_header("attachment"); - when_decoding(); + await when_decoding(); ThenTheResult.FileName.ShouldBeNull(); } [Test] - public void the_original_name_is_set_when_present_in_the_content_disposition_header() + public async Task the_original_name_is_set_when_present_in_the_content_disposition_header() { given_context(); given_request_entity_stream(); given_content_disposition_header("attachment;filename=\"test.txt\""); - when_decoding(); + await when_decoding(); ThenTheResult.FileName.ShouldBe("test.txt"); } @@ -166,6 +162,16 @@ public async Task a_file_with_a_more_specific_content_type_overrides_the_respons Response.Headers.ContentType.ShouldBe(MediaType.Xml); } + [Test] + public async Task a_file_with_a_length_sets_the_response_length() + { + given_context(); + given_entity(new InMemoryFile { Length = 1029}); + + await when_coding(); + Response.Headers.ContentLength.ShouldBe(1029); + } + [Test] public async Task a_downloadable_file_with_name_generates_a_content_disposition() { @@ -194,7 +200,7 @@ public async Task a_downloadable_file_without_name_generates_a_content_dispositi async Task when_coding() { - await (CreateCodec(Context)).WriteTo(_entity, Context.Response.Entity, null); + await CreateCodec(Context).WriteTo(_entity, Context.Response.Entity, null); } void given_entity(InMemoryFile file) @@ -213,26 +219,23 @@ protected override ApplicationOctetStreamCodec CreateCodec(ICommunicationContext public class when_converting_a_byte_stream_to_an_instance_of_a_stream : applicationoctetstream_context { [Test] - public void the_stream_length_is_set_to_the_size_of_the_sent_byte_stream() + public async Task the_stream_length_is_set_to_the_size_of_the_sent_byte_stream() { given_context(); given_request_entity_stream(); given_content_disposition_header("attachment;filename=\"test.txt\""); - WhenParsing(); + await WhenParsing(); ThenTheResult.Length.ShouldBe(1024); } - public void WhenParsing() + public async Task WhenParsing() { - when_decoding(); + await when_decoding(); } - public Stream ThenTheResult - { - get { return then_decoding_result(); } - } + public Stream ThenTheResult => then_decoding_result(); } public class applicationoctetstream_context : media_type_reader_context diff --git a/src/OpenRasta.Tests.Unit/Infrastructure/media_type_reader_context.cs b/src/OpenRasta.Tests.Unit/Infrastructure/media_type_reader_context.cs index ee78c959..60fc452a 100644 --- a/src/OpenRasta.Tests.Unit/Infrastructure/media_type_reader_context.cs +++ b/src/OpenRasta.Tests.Unit/Infrastructure/media_type_reader_context.cs @@ -69,7 +69,6 @@ protected async Task when_decoding() protected async Task when_decoding(string paramName) { var codecInstance = CreateCodec(Context); - var codec = codecInstance as IMediaTypeReader; switch (codecInstance) { case IMediaTypeReaderAsync async: diff --git a/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.Sync.cs b/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.Sync.cs index 2d73a2de..b563d483 100644 --- a/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.Sync.cs +++ b/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.Sync.cs @@ -28,33 +28,41 @@ void IMediaTypeWriter.WriteTo(object entity, IHttpEntity response, string[] code throw new InvalidOperationException(); } - private static bool TryProcessAs(object target, Action action) where T : class + static bool TryProcessAs(object target, Action action) where T : class { - var typedTarget = target as T; - if (typedTarget == null) return false; - action(typedTarget); - return true; + if (target is T typedTarget) + { + action(typedTarget); + return true; + } + + return false; } - private static void WriteFileWithFilename(IFile file, string disposition, IHttpEntity response) + static void WriteFileWithFilename(IFile file, string disposition, IHttpEntity response) { var contentDispositionHeader = response.Headers.ContentDisposition ?? new ContentDispositionHeader(disposition); if (!string.IsNullOrEmpty(file.FileName)) contentDispositionHeader.FileName = file.FileName; - if (!string.IsNullOrEmpty(contentDispositionHeader.FileName) || - contentDispositionHeader.Disposition != "inline") + + if (!string.IsNullOrEmpty(contentDispositionHeader.FileName) + || contentDispositionHeader.Disposition != "inline") response.Headers.ContentDisposition = contentDispositionHeader; + if (file.ContentType != null && file.ContentType != MediaType.ApplicationOctetStream || (file.ContentType == MediaType.ApplicationOctetStream && response.ContentType == null)) response.ContentType = file.ContentType; - // TODO: use contentLength from IFile + + if (file.Length > 0 && response.ContentLength == null) + response.ContentLength = file.Length; using (var stream = file.OpenStream()) + stream.CopyTo(response.Stream); } - private IEnumerable GetWriters(object entity, IHttpEntity response) + IEnumerable GetWriters(object entity, IHttpEntity response) { yield return TryProcessAs(entity, file => WriteFileWithFilename(file, "attachment", response)); diff --git a/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.cs b/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.cs index fef17dd9..b147f665 100644 --- a/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.cs +++ b/src/OpenRasta/Codecs/application/octet-stream/ApplicationOctetStreamCodec.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Reflection; using System.Threading.Tasks; using OpenRasta.IO; @@ -10,73 +9,77 @@ namespace OpenRasta.Codecs { - [MediaType("application/octet-stream;q=0.5")] - [MediaType("*/*;q=0.1")] - [SupportedType(typeof(IFile))] - [SupportedType(typeof(Stream))] - [SupportedType(typeof(byte[]))] - public partial class ApplicationOctetStreamCodec : - Codec, - IMediaTypeReaderAsync, - IMediaTypeReader, - IMediaTypeWriterAsync, - IMediaTypeWriter + [MediaType("application/octet-stream;q=0.5")] + [MediaType("*/*;q=0.1")] + [SupportedType(typeof(IFile))] + [SupportedType(typeof(Stream))] + [SupportedType(typeof(byte[]))] + public partial class ApplicationOctetStreamCodec : + Codec, + IMediaTypeReaderAsync, + IMediaTypeReader, + IMediaTypeWriterAsync, + IMediaTypeWriter + { + public async Task ReadFrom(IHttpEntity request, IType destinationType, string destinationName) { - public async Task ReadFrom(IHttpEntity request, IType destinationType, string destinationName) - { - if (destinationType.IsAssignableTo()) - return new HttpEntityFile(request); - if (destinationType.IsAssignableTo()) - return request.Stream; - if (destinationType.IsAssignableTo()) - return await request.Stream.ReadToEndAsync(); - return Missing.Value; - } + if (destinationType.IsAssignableTo()) + return new HttpEntityFile(request); + if (destinationType.IsAssignableTo()) + return request.Stream; + if (destinationType.IsAssignableTo()) + return await request.Stream.ReadToEndAsync(); + return Missing.Value; + } + + public async Task WriteTo(object entity, IHttpEntity response, IEnumerable codecParameters) + { + foreach (var writer in GetWritersAsync(entity, response)) + if (await writer) + return; + + throw new InvalidOperationException(); + } + - public async Task WriteTo(object entity, IHttpEntity response, IEnumerable codecParameters) - { - foreach (var writer in GetWritersAsync(entity, response)) - if (await writer) - return; + static async Task TryProcessAsAsync(object target, Func action) where T : class + { + if (!(target is T typedTarget)) return false; + await action(typedTarget); + return true; + } + + static async Task WriteFileWithFilenameAsync(IFile file, string disposition, IHttpEntity response) + { + var contentDispositionHeader = + response.Headers.ContentDisposition ?? new ContentDispositionHeader(disposition); - throw new InvalidOperationException(); - } + if (!string.IsNullOrEmpty(file.FileName)) + contentDispositionHeader.FileName = file.FileName; + if (!string.IsNullOrEmpty(contentDispositionHeader.FileName) || + contentDispositionHeader.Disposition != "inline") + response.Headers.ContentDisposition = contentDispositionHeader; - static async Task TryProcessAsAsync(object target, Func action) where T : class - { - var typedTarget = target as T; - if (typedTarget == null) return false; - await action(typedTarget); - return true; - } + if (file.ContentType != null && file.ContentType != MediaType.ApplicationOctetStream + || (file.ContentType == MediaType.ApplicationOctetStream && response.ContentType == null)) + response.ContentType = file.ContentType; - static async Task WriteFileWithFilenameAsync(IFile file, string disposition, IHttpEntity response) - { - var contentDispositionHeader = - response.Headers.ContentDisposition ?? new ContentDispositionHeader(disposition); + if (file.Length > 0 && response.ContentLength == null) + response.ContentLength = file.Length; - if (!string.IsNullOrEmpty(file.FileName)) - contentDispositionHeader.FileName = file.FileName; - if (!string.IsNullOrEmpty(contentDispositionHeader.FileName) || - contentDispositionHeader.Disposition != "inline") - response.Headers.ContentDisposition = contentDispositionHeader; - if (file.ContentType != null && file.ContentType != MediaType.ApplicationOctetStream - || (file.ContentType == MediaType.ApplicationOctetStream && response.ContentType == null)) - response.ContentType = file.ContentType; - // TODO: use contentLength from IFile - using (var stream = file.OpenStream()) - await stream.CopyToAsync(response.Stream); - } + using (var stream = file.OpenStream()) + await stream.CopyToAsync(response.Stream); + } - IEnumerable> GetWritersAsync(object entity, IHttpEntity response) - { - yield return TryProcessAsAsync(entity, - file => WriteFileWithFilenameAsync(file, "attachment", response)); - yield return TryProcessAsAsync(entity, file => WriteFileWithFilenameAsync(file, "inline", response)); - // TODO: Stream to be disposed and length to be written if needed - yield return TryProcessAsAsync(entity, stream => stream.CopyToAsync(response.Stream)); - yield return TryProcessAsAsync(entity, bytes => response.Stream.WriteAsync(bytes, 0, bytes.Length)); - } + IEnumerable> GetWritersAsync(object entity, IHttpEntity response) + { + yield return TryProcessAsAsync(entity, + file => WriteFileWithFilenameAsync(file, "attachment", response)); + yield return TryProcessAsAsync(entity, file => WriteFileWithFilenameAsync(file, "inline", response)); + // TODO: Stream to be disposed and length to be written if needed + yield return TryProcessAsAsync(entity, stream => stream.CopyToAsync(response.Stream)); + yield return TryProcessAsAsync(entity, bytes => response.Stream.WriteAsync(bytes, 0, bytes.Length)); } + } } diff --git a/src/OpenRasta/Hosting/HttpListener/HttpListenerHost.cs b/src/OpenRasta/Hosting/HttpListener/HttpListenerHost.cs index 6e1be861..60f4b9a7 100644 --- a/src/OpenRasta/Hosting/HttpListener/HttpListenerHost.cs +++ b/src/OpenRasta/Hosting/HttpListener/HttpListenerHost.cs @@ -128,6 +128,8 @@ async Task ProcessContext(HttpListenerContext nativeContext) { _zeroPendingRequests.Set(); } + + nativeContext.Response.Close(); } } diff --git a/src/Tests/Hosting.HttpListener/disposing.cs b/src/Tests/Hosting.HttpListener/disposing.cs index 12cf9872..01400fb2 100644 --- a/src/Tests/Hosting.HttpListener/disposing.cs +++ b/src/Tests/Hosting.HttpListener/disposing.cs @@ -2,7 +2,7 @@ using Shouldly; using Xunit; -namespace Tests.Scenarios.HandlerSelection.Hosting.HttpListener +namespace Tests.Hosting.HttpListener { public class disposing { diff --git a/src/Tests/Hosting.HttpListener/serving_files.cs b/src/Tests/Hosting.HttpListener/serving_files.cs new file mode 100644 index 00000000..8ceed4a6 --- /dev/null +++ b/src/Tests/Hosting.HttpListener/serving_files.cs @@ -0,0 +1,58 @@ +using System; +using System.IO; +using System.Net; +using System.Text; +using OpenRasta.Configuration; +using OpenRasta.IO; +using Shouldly; +using Xunit; + +namespace Tests.Hosting.HttpListener +{ + public class serving_files : IDisposable + { + readonly TestHttpListener _httpListener; + + public serving_files() + { + _httpListener = new TestHttpListener(new Configuration()); + } + + [Fact] + public void can_get_without_length() => _httpListener.WebGet("file-no-length").ShouldBe("hello world"); + + [Fact] + public void can_get_with_length() => _httpListener.WebGet("file-with-length").ShouldBe("hello world"); + + class Configuration : IConfigurationSource + { + public void Configure() + { + ResourceSpace.Has.ResourcesNamed("File1").AtUri("/file-no-length").HandledBy(); + ResourceSpace.Has.ResourcesNamed("File2").AtUri("/file-with-length").HandledBy(); + } + } + + class NoLengthHandler + { + readonly byte[] Bytes = Encoding.UTF8.GetBytes("hello world"); + + public IFile Get() => new InMemoryFile(new MemoryStream(Bytes)); + } + + class WithLengthHandler + { + readonly byte[] Bytes = Encoding.UTF8.GetBytes("hello world"); + + public IFile Get() => new InMemoryFile(new MemoryStream(Bytes)) + { + Length = Bytes.Length + }; + } + + public void Dispose() + { + _httpListener.Host.Close(); + } + } +} \ No newline at end of file diff --git a/src/Tests/MemoryHostExtensions.cs b/src/Tests/MemoryHostExtensions.cs index 5bc0970f..a4fe1785 100644 --- a/src/Tests/MemoryHostExtensions.cs +++ b/src/Tests/MemoryHostExtensions.cs @@ -5,7 +5,7 @@ using OpenRasta.IO; using OpenRasta.Web; -namespace Tests.Scenarios.HandlerSelection +namespace Tests { public static class MemoryHostExtensions { diff --git a/src/Tests/Scenarios.HandlerReturns/LocationHeader/relative_uri.cs b/src/Tests/Scenarios.HandlerReturns/LocationHeader/relative_uri.cs index 2c01eec1..ca659dec 100644 --- a/src/Tests/Scenarios.HandlerReturns/LocationHeader/relative_uri.cs +++ b/src/Tests/Scenarios.HandlerReturns/LocationHeader/relative_uri.cs @@ -1,15 +1,68 @@ using System.Threading.Tasks; +using OpenRasta.Codecs; +using OpenRasta.Configuration; +using OpenRasta.Diagnostics; +using OpenRasta.DI; +using OpenRasta.Hosting.InMemory; +using OpenRasta.Pipeline; +using OpenRasta.Web; using Shouldly; +using Tests.Scenarios.HandlerSelection; +using Tests.Scenarios.HandlerThrows; using Xunit; namespace Tests.Scenarios.HandlerReturns.LocationHeader { + public class mytest + { + readonly InMemoryHost _inMemoryHost; + + public mytest() + { + _inMemoryHost = new InMemoryHost(() => + { + ResourceSpace.Uses.PipelineContributor(); + + ResourceSpace.Has.ResourcesOfType() + .AtUri("/") + .HandledBy().TranscodedBy(); + }); + } + + [Fact] + public void test() + { + var result = _inMemoryHost.Get("/").Result; + result.ReadString().ShouldBe("hello world"); + } + + class MyContributor : IPipelineContributor + { + public void Initialize(IPipeline pipelineRunner) + { + pipelineRunner.Notify(context => + { + context.OperationResult = new OperationResult.OK("hello world"); + return PipelineContinuation.RenderNow; + }).Before().And.After(); + } + } + + class MyHandler + { + public string Get() + { + return "hello world"; + } + } + } + public class relative_uri : location_header { [Fact] public async Task location_is_absolute() { - var r = await Response; + var r = await Response; var rAsync = await ResponseAsync; r.StatusCode.ShouldBe(200); rAsync.StatusCode.ShouldBe(200); diff --git a/src/Tests/Scenarios.HandlerReturns/Resource/get_with_dependency.cs b/src/Tests/Scenarios.HandlerReturns/Resource/get_with_dependency.cs index 2f6a4bb4..98d3c194 100644 --- a/src/Tests/Scenarios.HandlerReturns/Resource/get_with_dependency.cs +++ b/src/Tests/Scenarios.HandlerReturns/Resource/get_with_dependency.cs @@ -3,7 +3,6 @@ using OpenRasta.Hosting.InMemory; using OpenRasta.Web; using Shouldly; -using Tests.Scenarios.HandlerSelection; using Xunit; namespace Tests.Scenarios.HandlerReturns.Resource diff --git a/src/Tests/Scenarios.HandlerThrows/exception_with_logging_from_http_listener_host.cs b/src/Tests/Scenarios.HandlerThrows/exception_with_logging_from_http_listener_host.cs index a1376d58..1915dab3 100644 --- a/src/Tests/Scenarios.HandlerThrows/exception_with_logging_from_http_listener_host.cs +++ b/src/Tests/Scenarios.HandlerThrows/exception_with_logging_from_http_listener_host.cs @@ -6,7 +6,6 @@ using OpenRasta.Configuration; using OpenRasta.Diagnostics; using OpenRasta.DI; -using OpenRasta.Hosting.HttpListener; using Shouldly; using Xunit; @@ -15,44 +14,22 @@ namespace Tests.Scenarios.HandlerThrows public class exception_with_logging_from_http_listener_host : IDisposable { readonly FakeLogger _fakeLogger; - readonly HttpListenerHost _httpListenerHost; + readonly TestHttpListener _httpListener; readonly HttpWebResponse _response; - static readonly Random _random = new Random(); public exception_with_logging_from_http_listener_host() { _fakeLogger = new FakeLogger(); - var appPathVDir = $"Temporary_Listen_Addresses/{Guid.NewGuid()}/"; + _httpListener = new TestHttpListener(new Configuration(_fakeLogger)); - var started = false; - int port = -1; - do + try { - try - { - port = _random.Next(2048, 4096); - - _httpListenerHost = new HttpListenerHost(new Configuration(_fakeLogger)); - _httpListenerHost.Initialize(new[] {$"http://+:{port}/{appPathVDir}"}, appPathVDir, null); - _httpListenerHost.StartListening(); - started = true; - } - catch - { - } - } while (!started); - - using (var webClient = new WebClient()) + _httpListener.WebGet("/"); + } + catch (WebException e) { - try - { - webClient.DownloadString($"http://localhost:{port}/{appPathVDir}"); - } - catch (WebException e) - { - _response = (HttpWebResponse) e.Response; - } + _response = (HttpWebResponse) e.Response; } } @@ -80,23 +57,20 @@ public Configuration(FakeLogger fakeLogger) public void Configure() { - using (OpenRastaConfiguration.Manual) - { - ResourceSpace.Uses.Resolver.AddDependencyInstance( - typeof(ILogger), - _fakeLogger, - DependencyLifetime.Singleton); + ResourceSpace.Uses.Resolver.AddDependencyInstance( + typeof(ILogger), + _fakeLogger, + DependencyLifetime.Singleton); - ResourceSpace.Has.ResourcesNamed("root") - .AtUri("/") - .HandledBy().TranscodedBy(); - } + ResourceSpace.Has.ResourcesNamed("root") + .AtUri("/") + .HandledBy().TranscodedBy(); } } public void Dispose() { - _httpListenerHost.Close(); + _httpListener.Host.Close(); } } } \ No newline at end of file diff --git a/src/Tests/TestHttpListener.cs b/src/Tests/TestHttpListener.cs new file mode 100644 index 00000000..1d16f54f --- /dev/null +++ b/src/Tests/TestHttpListener.cs @@ -0,0 +1,51 @@ +using System; +using System.Net; +using OpenRasta.Configuration; +using OpenRasta.Hosting.HttpListener; + +namespace Tests +{ + public class TestHttpListener + { + static readonly Random Random = new Random(); + + public TestHttpListener(IConfigurationSource configuration) + { + // TODO: On windows, anyone can listen on port 80 at Temporary_Listen_Addresses + // TODO: On Mono, not the case. So we probably need different logic for windows/mono + // TODO: otherwise you need to be Admin to run these tests on Windows + AppPathVDir = $"Temporary_Listen_Addresses/{Guid.NewGuid()}/"; + + var isStarting = true; + do + { + try + { + Port = Random.Next(2048, 4096); + + Host = new HttpListenerHost(configuration); + Host.Initialize(new[] { $"http://+:{Port}/{AppPathVDir}" }, AppPathVDir, null); + Host.StartListening(); + + isStarting = false; + } + catch + { + // Ignore + } + } while (isStarting); + } + + public string WebGet(string path) + { + using (var webClient = new WebClient()) + { + return webClient.DownloadString($"http://localhost:{Port}/{AppPathVDir}{path}"); + } + } + + public HttpListenerHost Host { get; } + public string AppPathVDir { get; } + public int Port { get; } + } +} \ No newline at end of file