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

🎳 Refine socks5 server UdpAssociate response behavior #523

Conversation

database64128
Copy link
Contributor

  • Previously, without specifying the server IP, the remote address in the response to a UdpAssoicate command is 127.0.0.1, which might break UDP for non-localhost clients.
  • This PR changes it so that, localhost clients get responses with the corresponding loopback IP, non-localhost clients get responses with the corresponding net.AnyIP or net.AnyIPv6.
  • The new behavior is also consistent with many other implementations. So the compatibility is guaranteed. It also makes specifying server IP optional.

@database64128 database64128 force-pushed the socks-server-udp-associate-response-remote-address branch from 370577f to b6f1d8c Compare December 16, 2020 05:41
- Previously, without specifying the server IP, the remote address in the response to a UdpAssoicate command is `127.0.0.1`, which might break UDP for non-localhost clients.
- This commit changes it so that, localhost clients get responses with the corresponding loopback IP, non-localhost clients get responses with the corresponding `net.AnyIP` or `net.AnyIPv6`.
- The new behavior is also consistent with many other implementations. So the compatibility is guaranteed. It also makes specifying server IP optional.
@database64128 database64128 force-pushed the socks-server-udp-associate-response-remote-address branch from b6f1d8c to 7b5b41d Compare December 16, 2020 05:45
mzz2017 added a commit to mzz2017/glider that referenced this pull request Dec 16, 2020
When server returns an any ip (0.0.0.0 or [::0]), we should use conventional ip to replace the any ip given (0.0.0.0 or [::0]).
This behaviour adapts to most situations.

See v2fly/v2ray-core#523
@kslr kslr merged commit 29f16cd into v2fly:master Dec 16, 2020
@kslr
Copy link
Contributor

kslr commented Dec 16, 2020

Thanks for your work.

@database64128 database64128 deleted the socks-server-udp-associate-response-remote-address branch December 16, 2020 09:08
lucifer9 pushed a commit to lucifer9/v2ray-core that referenced this pull request Dec 18, 2020
- Previously, without specifying the server IP, the remote address in the response to a UdpAssoicate command is `127.0.0.1`, which might break UDP for non-localhost clients.
- This commit changes it so that, localhost clients get responses with the corresponding loopback IP, non-localhost clients get responses with the corresponding `net.AnyIP` or `net.AnyIPv6`.
- The new behavior is also consistent with many other implementations. So the compatibility is guaranteed. It also makes specifying server IP optional.
lucifer9 pushed a commit to lucifer9/v2ray-core that referenced this pull request Dec 18, 2020
- Previously, without specifying the server IP, the remote address in the response to a UdpAssoicate command is `127.0.0.1`, which might break UDP for non-localhost clients.
- This commit changes it so that, localhost clients get responses with the corresponding loopback IP, non-localhost clients get responses with the corresponding `net.AnyIP` or `net.AnyIPv6`.
- The new behavior is also consistent with many other implementations. So the compatibility is guaranteed. It also makes specifying server IP optional.
nadoo pushed a commit to nadoo/glider that referenced this pull request Feb 7, 2021
* fix(socks5): should not dial returned bind addr directly
When server returns an any ip (0.0.0.0 or [::0]), we should use conventional ip to replace the any ip given (0.0.0.0 or [::0]).
This behaviour adapts to most situations.

See v2fly/v2ray-core#523

* fix: splithostport
database64128 added a commit to v2fly/v2fly-github-io that referenced this pull request Jun 26, 2021
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