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

socks5 support #113

Closed
wants to merge 9 commits into from
Closed

socks5 support #113

wants to merge 9 commits into from

Conversation

1zun4
Copy link
Contributor

@1zun4 1zun4 commented Nov 29, 2023

Introduces socks5 support to azalea. I am using this myself and it works well.
Fixes #98

Might not be ideally implemented.

Copy link
Collaborator

@mat-1 mat-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing this!

I think most of the functions that take in Proxy as an argument (Client::join, ping_server, Connection::new, ClientBuilder::start, Swarm::add, Swarm::add_with_exponential_backoff) should be split into separate functions to keep them simple since it's a lot more common for the user to not be using a proxy (and because I might want to add a feature gate to optionally disable the socks5-impl dependency in the future).
For example Connection::new could be split into new(address: &SocketAddr), new_with_proxy(address: &SocketAddr, proxy: Option<Proxy>), and new_from_stream(stream: TcpStream).

#[derive(Debug, Clone)]
pub struct Proxy {
pub addr: SocketAddr,
pub auth: Option<(String, String)>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a socks5_impl::protocol::UserKey instead of (String, String)

@1zun4
Copy link
Contributor Author

1zun4 commented Nov 30, 2023

Thank you for writing this!

I think most of the functions that take in Proxy as an argument (Client::join, ping_server, Connection::new, ClientBuilder::start, Swarm::add, Swarm::add_with_exponential_backoff) should be split into separate functions to keep them simple since it's a lot more common for the user to not be using a proxy (and because I might want to add a feature gate to optionally disable the socks5-impl dependency in the future). For example Connection::new could be split into new(address: &SocketAddr), new_with_proxy(address: &SocketAddr, proxy: Option<Proxy>), and new_from_stream(stream: TcpStream).

I completely agree with your suggestion to split the functions that take a Proxy as an argument into separate functions for simplicity, such as adding a feature gate to optionally disable the socks5-impl dependency.

I initially implemented it in a quick-and-dirty manner for the sake of expediency, but your feedback is spot-on. Expect these changes soon.

@mat-1
Copy link
Collaborator

mat-1 commented Dec 15, 2023

I don't mean to bother you, but do you still plan on finishing up this PR? If not then I'd be willing to finish it myself.

@1zun4
Copy link
Contributor Author

1zun4 commented Dec 15, 2023

I don't mean to bother you, but do you still plan on finishing up this PR? If not then I'd be willing to finish it myself.

Go ahead. I was going to, but I don't have enough time at the moment.

mat-1 added a commit that referenced this pull request Apr 20, 2024
@mat-1
Copy link
Collaborator

mat-1 commented Apr 20, 2024

Merged in 353eda2 :)

I did a few changes: separating the socks5 and non-socks5 functions, making it easier to set which proxy a bot will use and removing the randomization, and generally cleaning it up a little so things like setting a custom join address for a bot and its proxy is set from a single options struct

@mat-1 mat-1 closed this Apr 20, 2024
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.

Socks5 proxy support for client
2 participants