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

Use Ipv4 address for port checker #2396

Merged
merged 7 commits into from
Jan 3, 2023

Conversation

dylansama
Copy link
Contributor

@dylansama dylansama commented Oct 22, 2022

Pass --ipv4 parameter to curl to use Ipv4 address, when checking for ports. The port checker websites do not support Ipv6.

@dylansama
Copy link
Contributor Author

Technically the preferIpv4 feature could be expanded to http portion of Snoopy client as well, since curl options are exclusively for https requests. It seems the only major changes would be in the connect() function, when creating a socket context you would bind to 0:0 which will only match ipv4 addresses, whereas [0]:0 (or possibly [::]:0) will match an ipv6 address per docs.

If there is any desire for that as well. I see very few things nowadays not using https though.

@dylansama dylansama changed the title Plugin: check_port allow preferring ipv4 addresses Plugin: check_port enhancement - allow preferring ipv4 addresses Nov 2, 2022
@dylansama
Copy link
Contributor Author

Fixes #2402

@dylansama
Copy link
Contributor Author

Added support for preferIpv4 on plain http requests as well. Switching to the more diverse function stream_socket_client over fsockopen for all requests. Guessing the function check for stream_socket_client was from very ancient PHP 4.x days. As noted before simply need to set bindTo option to 0:0 rather than letting system choose.

plugins/check_port/conf.php Outdated Show resolved Hide resolved
php/Snoopy.class.inc Outdated Show resolved Hide resolved
php/Snoopy.class.inc Outdated Show resolved Hide resolved
php/Snoopy.class.inc Outdated Show resolved Hide resolved
@stickz stickz added the WIP label Dec 26, 2022
@stickz
Copy link
Collaborator

stickz commented Dec 26, 2022

Added Work In Progress label. Looking for changes and to disable by default to reduce the chances of a regression.

Anther pull request will be required with improvements to enable the feature by default. Snoppy we need to be improved. 0:0 is not a good way to bind to the IPV4 address. This will create a regression if a static IP address is assigned for torrents. Please don't address this issue in the current pull request. We want to merge a more minimal set of changes first into ruTorrent.

@stickz
Copy link
Collaborator

stickz commented Jan 2, 2023

Target changed to version 4.1 stable. Version 4.0 stable is being released next week.

dylansama added 2 commits January 2, 2023 10:08
@dylansama
Copy link
Contributor Author

dylansama commented Jan 2, 2023

Added Work In Progress label. Looking for changes and to disable by default to reduce the chances of a regression.

Anther pull request will be required with improvements to enable the feature by default. Snoppy we need to be improved. 0:0 is not a good way to bind to the IPV4 address. This will create a regression if a static IP address is assigned for torrents. Please don't address this issue in the current pull request. We want to merge a more minimal set of changes first into ruTorrent.

Yeah perhaps we could just skip the feature altogether for insecure http requests? I was just toying around to make it "feature complete" for Snoopy client between both http and https. Specifically this PR is really just for fixing the check_port plugin and both of the sites in the plugin use only https anyways. So all the stream context binding stuff is really un-used, the only truly useful change is the curl paramter --ipv4.

@stickz stickz changed the title Plugin: check_port enhancement - allow preferring ipv4 addresses Use Ipv4 address for port checker Jan 3, 2023
@stickz stickz added the bug label Jan 3, 2023
@stickz
Copy link
Collaborator

stickz commented Jan 3, 2023

@dylansama I edited your pull request and squash merged the changes into version 4.0 stable. I wanted to show you how to create an effective pull request. The useIpv4 feature is enabled by default. There is no risk of regression.

If you would like to make additional improvements, please create a new pull request.

@stickz stickz merged commit 3da85a1 into Novik:master Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants