Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

Commit

Permalink
Don't always buffer responses #6
Browse files Browse the repository at this point in the history
  • Loading branch information
@azzlack
azzlack committed Jan 28, 2016
1 parent 0325fad commit a68c7e3
Showing 1 changed file with 28 additions and 3 deletions.
31 changes: 28 additions & 3 deletions src/Server/ServerCompressionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,11 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
// Compress content if it should processed
if (process && response.Content != null)
{
// Buffer content for further processing.
// NOTE: This is needed to properly calculate Content-Length
await response.Content.LoadIntoBufferAsync();
// Buffer content for further processing if content length is not known
if (!this.ContentLengthKnown(response))
{
await response.Content.LoadIntoBufferAsync();
}

await this.CompressResponse(request, response);
}
Expand Down Expand Up @@ -210,5 +212,28 @@ private async Task DecompressRequest(HttpRequestMessage request)
}
}
}

/// <summary>Checks if the response content length is known.</summary>
/// <param name="response">The response.</param>
/// <returns>true if it is known, false if it it is not.</returns>
private bool ContentLengthKnown(HttpResponseMessage response)
{
if (response?.Content == null)
{
return false;
}

if (response.Content is StringContent)
{
return true;
}

if (response.Content is ByteArrayContent)
{
return true;
}

return false;
}
}
}

4 comments on commit a68c7e3

@ahmedalejo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess System.Net.Http.HttpContent.TryComputeLength should be more appropriate, but since it´s protected a wrapper ``HttpContent` could be used expose this method.

Or simply ignore content length since GzipStream doesn´t need to know the length of the stream to be compressed(correct me if i´m wrong)

@CheloXL
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahmedalejo that computation is in place for the client to know the number of total bytes that the request's response is going to have, in advance. That let's browsers (for example) display a progress bar while downloading data. It has nothing to do with the actual compression.

@ahmedalejo
Copy link

@ahmedalejo ahmedalejo commented on a68c7e3 Aug 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CheloXL i´m aware of that. The case is that in CompressedContent.SerializeToStreamAsync we always do

await this.originalContent.ReadAsByteArrayAsync()

instead of

await this.originalContent.ReadAsStreamAsync()

which makes await response.Content.LoadIntoBufferAsync(); superfluous and using more memory.

  • Another point why are we buffering and not streaming?
  • ContentLengthKnown(HttpResponseMessage response) isn´t considering the response´s Content-Length header

@ahmedalejo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also doing await response.Content.LoadIntoBufferAsync() in messageHandler doesn´t allow you implement a custom ICompressor that wants to do streaming (i.e no In-Memory buffer, or at least not a big one)

Please sign in to comment.