Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

SslStream WriteAsync#5505

Closed
benaadams wants to merge 2 commits into
dotnet:masterfrom
benaadams:sslstream-async
Closed

SslStream WriteAsync#5505
benaadams wants to merge 2 commits into
dotnet:masterfrom
benaadams:sslstream-async

Conversation

@benaadams
Copy link
Copy Markdown
Member

This is just the WriteAsync portion of https://github.com/dotnet/corefx/issues/5077

The ReadAsync looks a lot more complicated so I'm not even going to attempt (i.e. the Read wasn't immediately apparent to me)

@davidsh
Copy link
Copy Markdown
Contributor

davidsh commented Jan 18, 2016

cc: @CIPop @stephentoub

@benaadams
Copy link
Copy Markdown
Member Author

Actually ReadAsync doesn't look too bad, other than the effects of async all the way down...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rather move these validations out of async scope.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done. Also have moved it up for sync so they remain matching implementations

Also have done it for sync so they remain matching implementations
@benaadams
Copy link
Copy Markdown
Member Author

Going through Read in more detail; which really is more complicated, and since its the security stream I don't think its advisable for me to try to convert it. So will just leave my contribution as the WriteAsync portion and bow out...

@CIPop
Copy link
Copy Markdown

CIPop commented Jan 19, 2016

Thanks @benaadams. Please add tests to validate the new behaviors.
Also, these tests should pass in both .Net Desktop and CoreFX to ensure app-compat.

@CIPop
Copy link
Copy Markdown

CIPop commented Jan 19, 2016

The ReadAsync looks a lot more complicated so I'm not even going to attempt

@benaadams Given the code complexity of the original .Net Desktop code, I'd rather retry to port the code from _SslStream.cs instead of creating new TPL APIs (i.e. merging this PR). After the Begin/End APIs are implemented, we can add TPL overrides to Read/WriteAsync based on wrappers similar to the ones in NetworkStream and NegotiateStream.

@benaadams
Copy link
Copy Markdown
Member Author

@CIPop I'm good with that

@benaadams benaadams closed this Jan 19, 2016
@karelz karelz modified the milestones: 1.2.0, 1.0.0-rtm Dec 3, 2016
@benaadams benaadams deleted the sslstream-async branch January 11, 2019 21:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants