shadowsocks / shadowsocks-rust

A Rust port of shadowsocks
https://shadowsocks.org/
MIT License
8.37k stars 1.15k forks source link

API - Support Stream+Sink for ProxySocket? #1608

Open ibigbug opened 1 month ago

ibigbug commented 1 month ago

The tcp stream does take opaque AsyncRead and AsyncWrite as underlying io https://docs.rs/shadowsocks/latest/shadowsocks/relay/tcprelay/proxy_stream/client/struct.ProxyClientStream.html#impl-AsyncRead-for-ProxyClientStream%3CS%3E

thoughts about having the udp socket to take Stream + Sink too ?

https://docs.rs/shadowsocks/latest/shadowsocks/relay/udprelay/proxy_socket/struct.ProxySocket.html

so it can be applied to proxy more general udp transports

zonyitoo commented 1 month ago

There is no Datagram based sockets in Rust's common libraries supports Read / Write or AsyncRead / AsyncWrite , here we are just following the common design in the Rust world.

so it can be applied to proxy more general udp transports

So what exactly problem are you struggling on?

ibigbug commented 1 month ago

there's Sink and Stream https://docs.rs/futures/latest/futures/sink/trait.Sink.html

i'd like to have ss wrap another UDP transport which impls Sink and Stream

zonyitoo commented 1 month ago

You can make a wrapper that wraps ProxyClientStream and make your own Sink

ibigbug commented 1 month ago

how to pass it to https://docs.rs/shadowsocks/latest/shadowsocks/relay/udprelay/proxy_socket/struct.ProxySocket.html#method.from_socket

ibigbug commented 1 month ago

essentially i want to pass something that is Sink here https://github.com/Watfaq/clash-rs/blob/master/clash_lib/src/proxy/shadowsocks/mod.rs#L234

zonyitoo commented 1 month ago

So you want the underlying socket implements Sink and let ProxySocket::from_socket works with types that implemented Sink?

That would require quite a lot of changes in the current implementation.

  1. Let ShadowUdpSocket implements Sink
  2. Make all the APIs of ProxySocket works with interfaces provided by Stream and Sink.

I don't see the necessity here because the underlying connection is not a "stream" based connection.

ibigbug commented 1 month ago

There's nothing todo with a stream. and I think the Sink and Stream trait is the AsyncRead and AsyncWrite for Udp or any segmented io.

I know it's a fundamental change that requires amount of work.

I've had a look into the current code, it doesn't look like any udpsocket API is accessed other than the io related, which are fully replaceable by the Sink and Stream traits.

I'm happy to make the changes if it we agreed it's way to go

zonyitoo commented 1 month ago

Please make a PR.

ibigbug commented 2 weeks ago

I'm working on this and realized a lot of the io methods take tokio ToSocketAddrs https://github.com/Watfaq/shadowsocks-rust/blob/7c154d2340849cdabb5971b3b6982f187a6b7efe/crates/shadowsocks-service/src/net/mon_socket.rs#L50

which is handing the DNS to sys resolver, should we change it to take plain SocketAddr and resolve the dns_resolver in the code base?

zonyitoo commented 1 week ago

ToSocketAddrs uses tokio's builtin DNS resolver, which is the libc's builtin getaddrinfo running in a thread-pool. In this project we use hickory-dns by default, which is built upon async/await framework.

ibigbug commented 1 week ago

Yeah I get it. Should we not use sys dns and use hickory dns for all?