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

[wasm][http] Make content stream unseekable #54303

Closed
wants to merge 2 commits into from

Conversation

lewing
Copy link
Member

@lewing lewing commented Jun 16, 2021

fix for #54159

@ghost
Copy link

ghost commented Jun 16, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

fix for #54159

Author: lewing
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@lewing lewing requested review from kjpou1 and pavelsavara and removed request for kjpou1 June 16, 2021 23:59
@lewing lewing added the arch-wasm WebAssembly architecture label Jun 17, 2021
@ghost
Copy link

ghost commented Jun 17, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

fix for #54159

Author: lewing
Assignees: -
Labels:

arch-wasm, area-System.Net.Http

Milestone: -

@pavelsavara
Copy link
Member

Let's merge #54289 first, so that we could see the tests pass

@stephentoub
Copy link
Member

I think arguably the test is being too strict here, and shouldn't require that the response stream be unseekable. That happens to be the case for SocketsHttpHandler here, but there's no fundamental reason it needs to be true. @geoffkizer?

@geoffkizer
Copy link
Contributor

That happens to be the case for SocketsHttpHandler here, but there's no fundamental reason it needs to be true.

I agree; if the resulting response Stream is seekable by default, then there's no reason to disallow this.

@pavelsavara
Copy link
Member

Closing in favor of #54603

@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Net.Http
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants