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

fix: Workaround to pass UInt16 value for port in ClientRequest.Options #285

Merged
merged 2 commits into from
Jan 7, 2019

Conversation

djones6
Copy link
Contributor

@djones6 djones6 commented Jan 2, 2019

Description

Applies the fix proposed in Kitura/Kitura-NIO#140 to Kitura-net, in order to allow port values greater than Int16.max to be used with ClientRequest.

Motivation and Context

ClientRequest.Options defines the port option to have a value of type Int16. This should be UInt16 as it needs to represent values in the range 1 - 65,535.

This code works until you try to connect to a server whose port is in a higher port range (32,768 - 65,535), such as would happen if you were to start the server on an ephemeral port.

Unfortunately, it would be a breaking change for any existing users of ClientRequest (eg. Kitura's tests: https://github.com/IBM-Swift/Kitura/blob/2.6.0/Tests/KituraTests/KituraTest.swift#L161) if we were to correct the type. Instead, this fix uses a trick to pass the bit pattern of a UInt16 value via the Int16 parameter. This requires that the user applies their half of the trick in cases where they want to pass a value greater than Int16.max.

This was already reported under: #275

More recently, Kitura/Kitura#1382 enables the Kitura tests to use an ephemeral port, so that the tests can run in parallel. However, the default ephemeral port range on Mac is 49,152 - 65,535, which guarantees that assignment of the port value to ClientRequest.Options.port will fail.

Kitura-CouchDB also mirrors this bug, since it sits on top of ClientRequest, and we should apply similar changes: https://github.com/IBM-Swift/Kitura-CouchDB/blob/2.1.1/Sources/CouchDB/ConnectionProperties.swift#L29

How Has This Been Tested?

Checklist:

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

@djones6 djones6 changed the title BREAKING: Correct type of port in ClientRequest.Options to UInt16 fix: Workaround to pass UInt16 value for port in ClientRequest.Options Jan 4, 2019
@djones6 djones6 requested a review from ianpartridge January 4, 2019 11:25
Copy link
Collaborator

@ianpartridge ianpartridge left a comment

Choose a reason for hiding this comment

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

Neat!

@djones6 djones6 merged commit 23895b6 into master Jan 7, 2019
@djones6 djones6 deleted the issue.uint_port branch January 7, 2019 10:10
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.

2 participants