fix false success from SOCKS server when Dispatch() fails

[basinilya]

Postpone SOCKS server ok reply to a TCP Connect command until Dispatch() actually creates an upstream Link. Previously it was sent immediately inside Handshake(). UDP works as before.

Tested in a standalone go program that only starts a V2Ray SOCKS server.

before:

$ curl -v --socks5-hostname localhost:1080 http://localhost
* Host localhost:1080 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:1080...
* SOCKS5 connect to localhost:80 (remotely resolved)
* SOCKS5 request granted.
* Connected to localhost () port 1080
* using HTTP/1.x
* Connected to localhost (::1) port 1080
* using HTTP/1.x
> GET / HTTP/1.1
> Host: localhost
> User-Agent: curl/8.11.0
> Accept: */*
>
* Request completely sent off
* Recv failure: Connection reset by peer
* closing connection #0
curl: (56) Recv failure: Connection reset by peer

Results(V5): Succeeded (0)

after:

$ curl -v --socks5-hostname localhost:1080 http://localhost
* Host localhost:1080 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:1080...
* SOCKS5 connect to localhost:80 (remotely resolved)
* cannot complete SOCKS5 connection to localhost. (5)
* closing connection #0
curl: (97) cannot complete SOCKS5 connection to localhost. (5)

_
Results(V5): Connection refused (5)

[xiaokangwang]

Thanks for your contribution. Would you mind making this behavior adjustable via a configuration flag?

The reason socks inbound return an successful reply to the remote end before the link is actually setup is there to make sure that the initial content of traffic is available for 1. routing decisions, 2. bundling protocol handshake with first payload message to reduce rtt and fingerprint overhead-free.

For those reasons, the current behavior is the default and expected action for a specific reasons, and cannot be changed without impacting existing use cases. I assume you wish to know whether a connection has been successfully created before sending traffic content that lacks reentrancy, and I am happy to discuss alternative solution of the issue you are interested.

[basinilya]

I got your point. Give me some time

[database64128]

Not sure if it's easy to do in v2ray, but in my project shadowsocks-go, the behavior is determined by a config option "disableInitialPayloadWait" and client capability. For example, if a "direct" client does not have TCP Fast Open enabled, it will indicate to the relay service that it does not consume initial payloads, then the relay service will not immediately ask the server to send a successful response, and will pass the actual connection result to the server. Shadowsocks proxy clients, on the other hand, will always advertise initial payload support, and will require "disableInitialPayloadWait": true to disable it.

Of course, in v2ray this would be further complicated by the potential use of sniffers, whereas in shadowsocks-go the router does not consume payload.

[database64128]

Also you might want to implement the same thing for HTTP CONNECT requests.

[xiaokangwang]

and client capability.

Thanks @database64128. In my opinion due to V2Ray's complexity, I don't recommend any contributor to implement such feature as it would require specialized knowledge and tools to implement it correctly, and significantly increase the difficulty to make application's behavior predictable.

Also you might want to implement the same thing for HTTP CONNECT requests.

If such thing does get implemented, please submit under a separate pull request.

[basinilya]

I added a flag to ServerConfig.