Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement HttpContentReadStream.Position getter #31000

Closed
zivkan opened this issue Sep 27, 2019 · 1 comment
Closed

implement HttpContentReadStream.Position getter #31000

zivkan opened this issue Sep 27, 2019 · 1 comment
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@zivkan
Copy link
Member

zivkan commented Sep 27, 2019

There are some scenarios where knowing the number of bytes downloaded from an HttpClient response stream would be useful. For example, showing progress during a large download, or reporting on how many bytes were transferred after the response stream is processed, when HttpResposeMessage.Content.Headers.ContentLength is not available and a buffered stream of the entire response was not used. I'm fine with this not working with the WinHttpHandler, but working in both the HTTP 1.x SocketsHttpHandler and the HTTP2 handler would be useful.

I think complexities from HTTP keepalive, pipelineing and HTTP2 streams are out of scope. I think this code snippet explains why better than I can describe with words:

var response = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);
var stream = await response.Content.ReadAsStreamAsync();
Assert.AreEqual(0, stream.Position);

I care about the position of the individual response stream. The total number of bytes received since the TCP connection was opened is not important to me (and if needed should be obtained though something other than the HTTP response's content stream).

HttpBaseStream currently throws a NotSupportedException. I do have a workaround, which is to use my own stream wrapper, something similar to:

internal class PositionStream : Stream
{
    private long _position;
    private Stream _baseStream;

    public PositionStream(Stream baseStream)
    {
        _position = 0;
        _baseStream = baseStream;
    }

    public override long Position { get => _position; set => throw new NotSupportedException(); }

    public override int Read(byte[] buffer, int offset, int count)
    {
        var read = _baseStream.Read(buffer, offset, count);
        _position += read;
        return read;
    }

    // the rest of the class
}

This is what I'd like HttpConentReadStream to implement. While this is a simple workaround and therefore it's tempting to keep it out of the BCL, the problem is that when AutomaticDecompression is used, it measures a different number of bytes that may be relevant, depending on what the app author wants to measure.

SocketsHttpHandler already has code to use the correct decompression algorithm based on content encoding, but it's in a private class, so I can't call it from my own code, meaning if a new compression type becomes available, I'd have to update my code. Perhaps if the code that uses the right decompression based on AutomaticDecompression settings and content-encoding, if that is made public, then it would be an alternative to my proposal as it would be possible to use my own byte-counting stream wrapper while also being forwards-compatible with new compressions types being added to HTTP responses in the future.

Here's a simple example of what I'd like to be able to do:

var sw = Stopwatch.StartNew();
var response = await client.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead);
var stream = await response.Content.ReadAsStreamAsync();

Stream baseStream;
if (stream is GZipStream gzipStream)
{
    baseStream = gzipStream.BaseStream;
}
else if (stream is DeflateStream deflateStream)
{
    baseStream = deflateStream.BaseStream;
}
else if (stream is BrotliStream brotliStream)
{
    baseStream = brotliStream.BaseStream;
}
else
{
    baseStream = stream;
}

var value = Deserialize<T>(stream);

sw.Stop();
networkTelemetry.OnComplete(uri, sw.Elapsed, bytesTransferred: baseStream.Position);
@karelz
Copy link
Member

karelz commented Jan 16, 2020

Triage: We currently don't track it. Adding one more field for a corner-case scenario (which has a workaround) does not seem to be worth it (perf impact on all scenarios).

@karelz karelz closed this as completed Jan 16, 2020
@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

3 participants