Skip to content

NIOSSLClientHandler: require serverHostname #82

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 12, 2019
Merged

NIOSSLClientHandler: require serverHostname #82

merged 2 commits into from
Mar 12, 2019

Conversation

@weissi
Copy link
Member

@weissi weissi commented Mar 12, 2019

Motivation:

It was too easy to forget passing serverHostname to NIOSSLClientHandler
which made certificate validation fail.

Modification:

Require users to pass a server hostname or nil explictly.

Result:

@weissi
NIOSSLClientHandler: require serverHostname
d5bb6a6 
Motivation:

It was too easy to forget passing serverHostname to NIOSSLClientHandler
which made certificate validation fail.

Modification:

Require users to pass a server hostname or `nil` explictly.

Result:

- harder to use NIOSSLClientHandler wrongly
- fixes apple#80
@weissi weissi requested a review from Lukasa March 12, 2019 12:03
@weissi weissi added this to the 2.0.0 milestone Mar 12, 2019
@weissi weissi added the ⚠️ semver/major Breaks existing public API. label Mar 12, 2019
ktoso
ktoso approved these changes Mar 12, 2019
View reviewed changes
Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Bystander LGTM, good to enforce passing that in. At least now it's explicit and easier to spot :-)

@weissi weissi merged commit 31bb183 into apple:master Mar 12, 2019
@weissi weissi deleted the jw-req-sh branch March 12, 2019 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ktoso ktoso ktoso approved these changes

@Lukasa Lukasa Lukasa approved these changes

Assignees

No one assigned

Labels

⚠️ semver/major Breaks existing public API.

Projects

None yet

Milestone

2.0.0

Development

Successfully merging this pull request may close these issues.

NIOSSLClientHandler should not have a default have for serverHostname

3 participants

@weissi @ktoso @Lukasa