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

Improve using Signature to connect socket #147

Merged
merged 4 commits into from
Sep 4, 2018
Merged

Conversation

mcfedr
Copy link
Contributor

@mcfedr mcfedr commented Aug 31, 2018

Allows to create a signature from an Address object and still have a hostname.

Description

The first commit just adds creation of Socket with an Address using the enums for socketType and protocol

The second one rearranges func connect(using so that if an address is in the signature this is used for the connection. It also only converts that address back to a hostname if it wasn't included in the Signature.

Previously this would have used the hostname and ignored the address - note this cannot change any existing code because before it wasn't possible to create a Signature with both address and hostname.

I also think its a bit clearly whats going on in the function as before the ordering of checks was complicated with nested ifs and guards.

There are a couple of bonus commits :)

  • 114f6b9 updates the status flag in a number of cases it would otherwise not have been, but pretty sure this makes sense - although it there are probably other places that are not handled, seems this flag probably isn't very reliable.

Motivation and Context

The main motivation was that I need to resolve a hostname early on, and use that address to make a connection later and still have TLS work.

How Has This Been Tested?

I have tested and used these changes on both iOS and MacOS

Checklist:

  • I have submitted a CLA form
  • [N/A] If applicable, I have updated the documentation accordingly.
  • [N/A] If applicable, I have added tests to cover my changes.

mcfedr added 4 commits August 30, 2018 12:18
…et before hostname

Also allow creating a Signature with Address and hostname

In this case the hostname is only used for SSL verification as the address is already resolved
@CLAassistant
Copy link

CLAassistant commented Aug 31, 2018

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@billabt billabt left a comment

Choose a reason for hiding this comment

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

Changes look good. Approved.

@billabt billabt merged commit 4d7860f into Kitura:master Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants