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

Visibility of methods in shadowsocks #1613

Closed
botylev opened this issue Aug 12, 2024 · 4 comments
Closed

Visibility of methods in shadowsocks #1613

botylev opened this issue Aug 12, 2024 · 4 comments

Comments

@botylev
Copy link

botylev commented Aug 12, 2024

Hello,

I'm building my own personal proxy tool and would like to use the existing shadowsocks-rust project as a library.
I faced a small problem with a function in ProxyServerStream::from_stream - it is marked as pub(crate) and therefore I cannot reuse it in my code. I know about ProxyListener built in shadowsocks crate but I would really prefer to use ProxyServerStream with already accepted TcpStream from Tokio because it is more convenient to work with in my application context.

Can you please elaborate on pub(crate) visibility on this method? Can it be done just like pub?

Also I would like to mention CryptoStream::set_request_nonce_with_received and CryptoStream::current_data_chunk_remaining because they are also marked as pub(crate) and I think they can be just pub.

Thanks

@botylev botylev changed the title Vii of shadowsocks as library Visibility of methods in shadowsocks Aug 12, 2024
@zonyitoo
Copy link
Collaborator

Make them public means that I have to maintain API compatibility in the future upgrades, but they are really should not being considered as "stable" APIs.

@botylev
Copy link
Author

botylev commented Aug 13, 2024

I see that ProxyClientStream::from_stream is public, so you maintain compatibility for this method but not for server side variant? Is there any particular reason for that? Thanks

@zonyitoo
Copy link
Collaborator

Well, because the Server stream can only be built by Listener, so it doesn't need to be public. The Client stream's from_stream was used in the service crate so it has to be public.

I am Ok to make Server stream's from_stream public, but the user_manager shouldn't be there because it is not a required attribute, which should be set with a setter.

@botylev
Copy link
Author

botylev commented Aug 13, 2024

Thank you very much, that will be enough for me.

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

No branches or pull requests

2 participants