Skip to content

ReferenceReadStream is not compatible with CryptoStream #18503

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

Closed
drauch opened this issue Jan 22, 2020 · 2 comments
Closed

ReferenceReadStream is not compatible with CryptoStream #18503

drauch opened this issue Jan 22, 2020 · 2 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue

Comments

@drauch
Copy link

drauch commented Jan 22, 2020

Describe the bug

Very similar to aspnet/KestrelHttpServer#2341 (PR aspnet/KestrelHttpServer#2342) the ReferenceReadStream does throw a NotSupportedException in its Flush method when it should not.

To Reproduce

using(var cs = new CryptoStream(formFile.OpenReadStream(), ...))
{
    throw new Exception("stream has not been read completely and therefore CryptoStream will call Flush");
}

This should not trigger a NotSupportedException.

Further technical details

  • ASP.NET Core version 2.1.x on .NET Framework 4.7.2
@analogrelay
Copy link
Contributor

Seems reasonable to just no-op in Flush instead of throwing.

The simplest workaround you can use for now (and in 2.1, since the change would be in 5.0) would be to build your own wrapper that forwards all calls to the inner stream except Flush and then wrap that around ReferenceReadStream.

@analogrelay analogrelay added this to the Next sprint planning milestone Jan 22, 2020
@drauch
Copy link
Author

drauch commented Jan 23, 2020

A pity it's not backported, but I guess the workaround is good enough. Thank you for the quick response.

@Tratcher Tratcher added good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue labels Jan 23, 2020
@Tratcher Tratcher added Done This issue has been fixed bug This issue describes a behavior which is not expected - a bug. labels Jan 25, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Feb 24, 2020
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

No branches or pull requests

5 participants