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

Fixes client mode version parsing #153

Merged
merged 1 commit into from Jul 21, 2023

Conversation

artemredkin
Copy link
Contributor

Motivation:
Server may send additional lines of data before version, but since we use version as part of key exchange, we need to filter out those lines, otherwise it will fail key exchange.

Modifications:

  • Strip out lines of data before version in client role
  • Update tests

@artemredkin artemredkin requested a review from Lukasa July 21, 2023 15:39
Motivation:
Server may send additional lines of data before version, but since we
use version as part of key exchange, we need to filter out those lines,
otherwise it will fail key exchange.

Modifications:
 - Strip out lines of data before version in client role
 - Update tests
@Lukasa Lukasa added the patch-version-bump-only For PRs that when merged will only cause a bump of the patch version, ie. 1.0.x -> 1.0.(x+1) label Jul 21, 2023
@Lukasa Lukasa enabled auto-merge (squash) July 21, 2023 16:19
@Lukasa Lukasa merged commit ded5e5c into apple:main Jul 21, 2023
6 of 8 checks passed
@artemredkin artemredkin deleted the fix_version_parsing branch July 21, 2023 16:20
@finagolfin
Copy link
Contributor

@Lukasa, I see that the minimum NIO version needs to be bumped after this pull, as takingOwnershipOfDescriptors() added here was not added till NIO 2.56.0. This causes build failures for me locally because this package is checking out 2.45.0 instead.

@Lukasa
Copy link
Collaborator

Lukasa commented Sep 6, 2023

Good catch, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-version-bump-only For PRs that when merged will only cause a bump of the patch version, ie. 1.0.x -> 1.0.(x+1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants