You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix: Accept single words as valid hostnames (#3591)
## Purpose of this PR
The regex we used to validate hostnames did not accept single words as
valid hostnames. But single words _can_ be valid hostnames. The most
common is of course "localhost" but one can edit one's hosts file to
define any word as something the local resolver will resolve to an IP
address.
This PR addresses this by removing any validation of the provided
hostname. We could modify the validity check to accept single words too,
but the regex is pretty indigestible already and it's simpler to just
let the resolver fail if the user provided garbage. UTP will signal this
through a disconnection event with an appropriate reason (which we'll be
able to map to a nice error message once [this
PR](#3551)
lands).
While I was at it, I also made a few cleanups and improvements:
- Removed the `UTP_TRANSPORT_2_4_ABOVE` define. NGO depends on UTP 2.4.0
so it can be safely assumed that users will have it installed. No need
to conditionally compile the code that depends on it.
- Added a test that establishes a connection using hostname resolution.
- Simplified the logic around connections and avoid bypassing the
`Connect` method when connecting to a hostname.
- Deprecated `ConnectionAddressData.ServerEndPoint`. We don't use it
anymore, it doesn't work with hostnames, and it's not providing any
value over just calling `NetworkEndpoint.Parse`. And worst of all: its
capitalization of "endpoint" doesn't match what we use elsewhere.
- Modified the listen address logic so that if a domain name is used, by
default if remote connections are allowed we will listen on :: (the IPv6
"any" address) instead of 0.0.0.0. This can still be overridden using
`SetConnectionData`. The reason for this change is that the resolver in
UTP prioritizes IPv6 addresses over IPv4. So if we listen on IPv4 by
default, we're likely to get issues if the resolver then ends up with an
IPv6 address. In current versions of UTP for instance, this causes
errors on Windows (a fix is on the way). I'm looking into changing the
behavior of the resolver to prefer IPv4, but that's an engine change so
might take a while to land. In the meantime defaulting to IPv6 seems
like the best approach.
### Changelog
- Fixed: Fixed an issue where `UnityTransport` would not accept single
words as valid hostnames (notably "localhost").
- Changed: Marked `UnityTransport.ConnectionAddressData.ServerEndPoint`
as obsolete. It can't work when using hostnames as the server address,
and its functionality can easily be replicated using
`NetworkEndpoint.Parse`.
## Documentation
No documentation changes or additions were necessary.
## Testing & QA
Tested with manual and automated tests.
## Backport
Hostname resolution is only supported in UTP 2.4+ and Unity 6.1+, so no
backport necessary.
---------
Co-authored-by: Michał Chrobot <[email protected]>
Co-authored-by: Noel Stephens <[email protected]>
Copy file name to clipboardExpand all lines: com.unity.netcode.gameobjects/CHANGELOG.md
+2-1Lines changed: 2 additions & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -14,14 +14,15 @@ Additional documentation and release notes are available at [Multiplayer Documen
14
14
15
15
### Fixed
16
16
17
+
- Fixed an issue where `UnityTransport` would not accept single words as valid hostnames (notably "localhost"). (#3591)
17
18
- Fixed issue where viewing a `NetworkBehaviour` with one or more `NetworkVariable` fields could throw an exception if running a distributed authority network topology with a local (DAHost) host and viewed on the host when the host is not the authority of the associated `NetworkObject`. (#3578)
18
19
- Fixed issue when using a distributed authority network topology and viewing a `NetworkBehaviour` with one or more `NetworkVariable` fields in the inspector view would not show editable fields. (#3578)
19
20
20
21
### Changed
21
22
23
+
- Marked `UnityTransport.ConnectionAddressData.ServerEndPoint` as obsolete. It can't work when using hostnames as the server address, and its functionality can easily be replicated using `NetworkEndpoint.Parse`. (#3591)
22
24
- Optimized `NetworkList<T>` indexer setter to skip operations when the new value equals the existing value, improving performance by avoiding unnecessary list events and network synchronization. (#3587)
/// <returns>A <see cref="NetworkConnection"/> representing the connection to the server, or an invalid connection if the connection attempt fails.</returns>
// If it's not valid, assure it meets FQDN standards
750
-
if(!IsValidFqdn(ConnectionData.Address))
751
-
{
752
-
// If not then log an error and return false
753
-
Debug.LogError($"Listen network address ({ConnectionData.Address}) is not a valid {NetworkFamily.Ipv4} or {NetworkFamily.Ipv6} address!");
754
-
}
755
-
else
756
-
{
757
-
Debug.LogError($"While ({ConnectionData.Address}) is a valid Fully Qualified Domain Name, you must use a valid {NetworkFamily.Ipv4} or {NetworkFamily.Ipv6} address when binding and listening for connections!");
758
-
}
759
706
returnfalse;
760
-
#else
761
-
Debug.LogError($"Network listen address ({ConnectionData.Address}) is {nameof(NetworkFamily.Invalid)}!");
0 commit comments