Fixes

Ensure that we copy response headers only once. It appears that response headers related to content can vanish after we've read the content stream, so to avoid this, we read just once on initial response. Fixes an issue where the Headers property of HttpMessageInfo objects can be null, which causes null reference exceptions when calling various methods. Changed to always initialize this property with a valid reference, and to perform null checks when accessing internally.
TechnikEmpire · Dec 17, 2018 · a542e3e · a542e3e
1 parent ae0025c
commit a542e3e
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 23 deletions.
diff --git a/CitadelCore/CitadelCore.csproj b/CitadelCore/CitadelCore.csproj
@@ -12,23 +12,15 @@
     <PackageIconUrl />
     <RepositoryUrl>https://github.com/TechnikEmpire/CitadelCore</RepositoryUrl>
     <PackageTags>proxy filtering content-filtering transparent-proxy</PackageTags>
-    <PackageReleaseNotes>Upgrades core deps.
-Enables HTTP/2 on the proxy front-end, but backend fulfillment is still HTTP/1.x.
-Strips all compression methods from the front end.
-Configures Kestrel for optimal throughput.
-Ensures that headers are replaced if they exist. This ensures that client changes always apply.
-Removes arbitrary limits, such as max length on request strings.
-Fixes an issue where ASP.NET Core incorrectly decodes some URL's, which causes some things to randomly fail as bad requests. Google maps is an example of what suffered in previous versions.
-Now builds out the full URL based on the raw values sent by the browser. This applies to websockets as well.
-Adds the ability to inspect individual websocket messages.
-Fixes an issue where the whole body inspection callback was invoked on websockets even if not requested.
-Everything is now extremely fast, and stable. No more public changes or additions will be made.</PackageReleaseNotes>
+    <PackageReleaseNotes>Ensure that we copy response headers only once. It appears that response headers related to content can vanish after we've read the content stream, so to avoid this, we read just once on initial response.
+
+Fixes an issue where the Headers property of HttpMessageInfo objects can be null, which causes null reference exceptions when calling various methods. Changed to always initialize this property with a valid reference, and to perform null checks when accessing internally.</PackageReleaseNotes>
     <Title>CitadelCore</Title>
     <Summary>Transparent filtering HTTP/S and Websocket/WebsocketSecure proxy.</Summary>
     <Description>Transparent filtering HTTP/S and Websocket/WebsocketSecure proxy.</Description>
-    <Version>4.2.3</Version>
-    <AssemblyVersion>4.2.3.0</AssemblyVersion>
-    <FileVersion>4.2.3.0</FileVersion>    
+    <Version>4.2.5</Version>
+    <AssemblyVersion>4.2.5.0</AssemblyVersion>
+    <FileVersion>4.2.5.0</FileVersion>    
   </PropertyGroup>
 
   <ItemGroup Label="dotnet pack instructions">

diff --git a/CitadelCore/Net/Handlers/FilterHttpResponseHandler.cs b/CitadelCore/Net/Handlers/FilterHttpResponseHandler.cs
@@ -431,7 +431,7 @@ public override async Task Handle(HttpContext context)
                             MessageId = requestMessageNfo.MessageId,
                             BodyContentType = response?.Content?.Headers?.ContentType?.ToString() ?? string.Empty,
                             IsEncrypted = context.Request.IsHttps,
-                            Headers = response.ExportAllHeaders(),
+                            Headers = responseHeaders,
                             MessageProtocol = MessageProtocol.Http,
                             HttpVersion = upstreamReqVersionMatch ?? new Version(1, 0),
                             StatusCode = response.StatusCode,
@@ -483,7 +483,7 @@ public override async Task Handle(HttpContext context)
                                                 MessageId = requestMessageNfo.MessageId,
                                                 BodyContentType = response?.Content?.Headers?.ContentType?.ToString() ?? string.Empty,
                                                 IsEncrypted = context.Request.IsHttps,
-                                                Headers = response.ExportAllHeaders(),
+                                                Headers = responseHeaders,
                                                 MessageProtocol = MessageProtocol.Http,
                                                 HttpVersion = upstreamReqVersionMatch ?? new Version(1, 0),
                                                 StatusCode = response.StatusCode,
@@ -553,7 +553,7 @@ public override async Task Handle(HttpContext context)
                                         MessageId = requestMessageNfo.MessageId,
                                         BodyContentType = response?.Content?.Headers?.ContentType?.ToString() ?? string.Empty,
                                         IsEncrypted = context.Request.IsHttps,
-                                        Headers = response.ExportAllHeaders(),
+                                        Headers = responseHeaders,
                                         MessageProtocol = MessageProtocol.Http,
                                         StatusCode = response.StatusCode,
                                         HttpVersion = upstreamReqVersionMatch ?? new Version(1, 0),
@@ -590,7 +590,7 @@ public override async Task Handle(HttpContext context)
                                         MessageId = requestMessageNfo.MessageId,
                                         BodyContentType = response?.Content?.Headers?.ContentType?.ToString() ?? string.Empty,
                                         IsEncrypted = context.Request.IsHttps,
-                                        Headers = response.ExportAllHeaders(),
+                                        Headers = responseHeaders,
                                         MessageProtocol = MessageProtocol.Http,
                                         StatusCode = response.StatusCode,
                                         HttpVersion = upstreamReqVersionMatch ?? new Version(1, 0),
@@ -652,7 +652,7 @@ public override async Task Handle(HttpContext context)
                     using (var responseStream = await response?.Content.ReadAsStreamAsync())
                     {
                         context.Response.StatusCode = (int)response.StatusCode;
-                        context.Response.PopulateHeaders(response.ExportAllHeaders(), s_emptyExemptedHeaders);
+                        context.Response.PopulateHeaders(responseHeaders, s_emptyExemptedHeaders);
 
                         if (!responseHasZeroContentLength && (upstreamIsHttp1 || responseIsFixedLength))
                         {

diff --git a/CitadelCore/Net/Http/HttpMessageInfo.cs b/CitadelCore/Net/Http/HttpMessageInfo.cs
@@ -132,7 +132,7 @@ public NameValueCollection Headers
         {
             get;
             internal set;
-        } = null;
+        } = new NameValueCollection();
 
         /// <summary>
         /// Gets or sets the body content type.
@@ -355,7 +355,7 @@ public void MakeTemporaryRedirect(string location)
         {
             StatusCode = HttpStatusCode.Redirect;
             MessageType = MessageType.Response;
-            Headers.Clear();
+            Headers?.Clear();
             BodyContentType = string.Empty;
 
             if (_body != null && _body.Length > 0)
@@ -365,8 +365,11 @@ public void MakeTemporaryRedirect(string location)
 
             Body = new Memory<byte>();
 
-            Headers["Expires"] = TimeUtil.UnixEpochString;
-            Headers["Location"] = location;
+            if (Headers != null)
+            {
+                Headers["Expires"] = TimeUtil.UnixEpochString;
+                Headers["Location"] = location;
+            }
         }
 
         /// <summary>