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

1. Pass options to SIP003 plug-in through the Agruments property; 2. … #1684

Closed
wants to merge 1 commit into from

Conversation

ylhyh
Copy link

@ylhyh ylhyh commented Feb 17, 2018

…The Port of SIP003's "LocalEndPoint" should be Remote Server's port when a plug-in enabled.

…The Port of SIP003's "LocalEndPoint" should be Remote Server's port when a plug-in enabled.
@@ -55,6 +55,7 @@ private Sip003Plugin(string plugin, string pluginOpts, string serverAddress, int
StartInfo = new ProcessStartInfo
{
FileName = plugin,
Arguments = pluginOpts,
Copy link
Contributor

Choose a reason for hiding this comment

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

To whom also reviewing this PR:
as mentioned in shadowsocks/shadowsocks-org#28

  1. Passing arguments to a plugin
    A plugin accepts arguments through environment variables.

Standard SIP003 implementation not only take care on Windows, Linux or Mac, but also need to provide maximum compatibility to embedded system such as routers, or cellphones. That's why it chooses environment variables.

@@ -86,7 +87,7 @@ public bool StartIfNeeded()
return false;
}

var localPort = GetNextFreeTcpPort();
var localPort = int.Parse(_pluginProcess.StartInfo.Environment["SS_REMOTE_PORT"]);
Copy link
Contributor

Choose a reason for hiding this comment

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

SS_LOCAL_HOST SS_LOCAL_PORT: the plugin listens SS native connection.
SS_REMOTE_HOST SS_REMOTE_PORT: the plugin then connect to the remote server.

SS ===> to plugin's LOCAL HOST/PORT ===> plug-in data processing ===> send to REMOTE HOST/PORT

@ylhyh
Copy link
Author

ylhyh commented Feb 21, 2018

@celeron533 , I agree with you that Plugin should following the standard.
I am using kcptun (https://github.com/xtaci/kcptun) in Shadowsocks, it seems kcptun never use the environment variable %SS_LOCAL_PORT%, so it always listened on the default port 12948, I have to create a bat file to bypass this issue:
%~dp0client_windows_amd64.exe -l 127.0.0.1:%SS_LOCAL_PORT% -r xxx.xxx.xxx.xxx:xxxx -crypt none -mtu 1400 -sndwnd 2048 -rcvwnd 2048 -mode fast2

The only issue is that I can not specify the listen port of kcptun, because it's generated by Shadowsocks.

@celeron533
Copy link
Contributor

@ylhyh ,
Both designs has their pros and cons.
GetNextFreeTcpPort() makes sure that the port has no conflict and free to use.
SS_REMOTE_PORT (or "SS_LOCAL_PORT") lock the port as pre-defined.

BTW, in your code change it is SS_REMOTE_PORT but in your comment it is variable %SS_LOCAL_PORT%. Which one do you want?

SIP003 is a standard. You 'should' always uses the plugin programs which follow the standard.
#1672 is talking about mapping the environment variables to parameter to give maximum compatibility to normal plugin programs.

@ylhyh
Copy link
Author

ylhyh commented Mar 2, 2018

  1. It seems %SS_REMOTE_PORT% is useless when a plugin being launched, so I use it to specify the plugin's listening port.
  2. Yes, I did same thing as Add an option to let user wrap SIP003 environment variables as normal in-line arguments #1672 mentioned.

@ylhyh
Copy link
Author

ylhyh commented Mar 2, 2018

anyway, cancel the request now.

@ylhyh ylhyh closed this Mar 2, 2018
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.

2 participants