Merge | Remove OS-specific TdsParser files #3469
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was lingering from earlier work to change the SSPI abstractions
The managed SNI doesn't use this, so will always return SNI_SUCCESS_IO_PENDING.
The handling of this method is unusual. Only Windows actually waits for the SSL handshake.
Also mop up the unused SNIErrorDetails structure
This exposes a behavioural difference in TdsParser between Unix and Windows: downlevel TLS warnings only appear on Windows.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This picks up where #3434 left off. The earlier PR handled the simple case of merging OS-specific functionality (which was actually just different between the native and managed SNIs.) This one expands the principle, moving
PostReadAsyncForMarsandWaitForSSLHandShakeToCompletefromTdsParsertoTdsParserStateObject's child classes. This removes TdsParser.Unix.cs and TdsParser.Windows.cs.This has also involved modifying the call site for the method within TdsParser to reference TdsParserStateObject. This is particularly noticeable in EnableSsl.
Finally, while it's not a new gap, this process has highlighted that the existing functionality of warning when down-level SSL is in use only functions on Windows. I've maintained this gap and made it a little more explicit.
Issues
Contributes to #1261 and #2953.
Testing
Testing with SSL- and MARS-enabled connection strings works, I'd appreciate a CI run.