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
openrasta · Mar 24, 2018 · 85170da · 85170da
1 parent 398d3fa
commit 85170da
Show file tree

Hide file tree

Showing 12 changed files with 290 additions and 140 deletions.
diff --git 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<Stream>();
+            await when_decoding<Stream>();
         }
 
-        public Stream ThenTheResult
-        {
-            get { return then_decoding_result<Stream>(); }
-        }
+        public Stream ThenTheResult => then_decoding_result<Stream>();
     }
 
     public class applicationoctetstream_context : media_type_reader_context<ApplicationOctetStreamCodec>

diff --git 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<T>()
         protected async Task when_decoding<T>(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
@@ -28,33 +28,41 @@ void IMediaTypeWriter.WriteTo(object entity, IHttpEntity response, string[] code
         throw new InvalidOperationException();
     }
 
-    private static bool TryProcessAs<T>(object target, Action<T> action) where T : class
+    static bool TryProcessAs<T>(object target, Action<T> 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<bool> GetWriters(object entity, IHttpEntity response)
+    IEnumerable<bool> GetWriters(object entity, IHttpEntity response)
     {
       yield return TryProcessAs<IDownloadableFile>(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
@@ -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<object> ReadFrom(IHttpEntity request, IType destinationType, string destinationName)
     {
-      public async Task<object> ReadFrom(IHttpEntity request, IType destinationType, string destinationName)
-        {
-            if (destinationType.IsAssignableTo<IFile>())
-                return new HttpEntityFile(request);
-            if (destinationType.IsAssignableTo<Stream>())
-                return request.Stream;
-            if (destinationType.IsAssignableTo<byte[]>())
-                return await request.Stream.ReadToEndAsync();
-            return Missing.Value;
-        }
+      if (destinationType.IsAssignableTo<IFile>())
+        return new HttpEntityFile(request);
+      if (destinationType.IsAssignableTo<Stream>())
+        return request.Stream;
+      if (destinationType.IsAssignableTo<byte[]>())
+        return await request.Stream.ReadToEndAsync();
+      return Missing.Value;
+    }
+
+    public async Task WriteTo(object entity, IHttpEntity response, IEnumerable<string> codecParameters)
+    {
+      foreach (var writer in GetWritersAsync(entity, response))
+        if (await writer)
+          return;
+
+      throw new InvalidOperationException();
+    }
+
 
-      public async Task WriteTo(object entity, IHttpEntity response, IEnumerable<string> codecParameters)
-        {
-            foreach (var writer in GetWritersAsync(entity, response))
-                if (await writer)
-                    return;
+    static async Task<bool> TryProcessAsAsync<T>(object target, Func<T, Task> 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<bool> TryProcessAsAsync<T>(object target, Func<T, Task> 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<Task<bool>> GetWritersAsync(object entity, IHttpEntity response)
-        {
-            yield return TryProcessAsAsync<IDownloadableFile>(entity,
-                file => WriteFileWithFilenameAsync(file, "attachment", response));
-            yield return TryProcessAsAsync<IFile>(entity, file => WriteFileWithFilenameAsync(file, "inline", response));
-            // TODO: Stream to be disposed and length to be written if needed
-            yield return TryProcessAsAsync<Stream>(entity, stream => stream.CopyToAsync(response.Stream));
-            yield return TryProcessAsAsync<byte[]>(entity, bytes => response.Stream.WriteAsync(bytes, 0, bytes.Length));
-        }
+    IEnumerable<Task<bool>> GetWritersAsync(object entity, IHttpEntity response)
+    {
+      yield return TryProcessAsAsync<IDownloadableFile>(entity,
+          file => WriteFileWithFilenameAsync(file, "attachment", response));
+      yield return TryProcessAsAsync<IFile>(entity, file => WriteFileWithFilenameAsync(file, "inline", response));
+      // TODO: Stream to be disposed and length to be written if needed
+      yield return TryProcessAsAsync<Stream>(entity, stream => stream.CopyToAsync(response.Stream));
+      yield return TryProcessAsAsync<byte[]>(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
@@ -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
@@ -2,7 +2,7 @@
 using Shouldly;
 using Xunit;
 
-namespace Tests.Scenarios.HandlerSelection.Hosting.HttpListener
+namespace Tests.Hosting.HttpListener
 {
   public class disposing
   {