Report the proper socket error #897
Merged
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
When a connection fails, the socket error was being incorrectly reported as an address family not supported error, instead of connection refused. It looks like this happened between c4e698c (Unity 2018.1b5), where the proper error was reported, and ec98a4e (Unity 2018.1b6), there the incorrect error is reported. Why did this happen? Between those two changesets (there are about 180 intervening changesets), the order of addresses returned from `Dns.GetHostAddresses` changed. The newer code returns the IPv6 address first, and the IPv4 address second. While `GetCheckedIPs` mentioned in a comment that it skips addresses that don't match the address family, it did not. The code using the addresses returned by `GetCheckedIPs` will use the first address in the array. If this address happens to match the address family of the socket, things will work "properly" (the correct socket error will occur). If, however the first entry has the wrong address family, then the socket error will be an address family not supported error, since the socket was created with different address family. This change modifies `GetCheckedIPs` to filter the addresses, removing those which do not match the address family. This corrects Unity case 1012875.
joncham
approved these changes
Mar 30, 2018
mderoy
approved these changes
Apr 5, 2018
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.
When a connection fails, the socket error was being incorrectly reported
as an address family not supported error, instead of connection refused.
It looks like this happened between
c4e698c (Unity 2018.1b5), where the
proper error was reported, and ec98a4e
(Unity 2018.1b6), there the incorrect error is reported.
Why did this happen? Between those two changesets (there are about 180
intervening changesets), the order of addresses returned from
Dns.GetHostAddresseschanged. The newer code returns the IPv6 addressfirst, and the IPv4 address second. While
GetCheckedIPsmentioned in acomment that it skips addresses that don't match the address family, it
did not. The code using the addresses returned by
GetCheckedIPswilluse the first address in the array. If this address happens to match the
address family of the socket, things will work "properly" (the correct
socket error will occur).
If, however the first entry has the wrong address family, then the
socket error will be an address family not supported error, since the
socket was created with different address family.
This change modifies
GetCheckedIPsto filter the addresses, removingthose which do not match the address family.
This corrects Unity case 1012875.
Release notes:
Scripting: Provide the proper socket error when an IPv4 connection is refused.
Should we back port this change? I think it should go to 2018.1. Also, should we upstream this? I think that we should.