Implement server_poll_timeout for socks instead of hard-coded 5 sec

[5andr0]

People have been struggling to run openvpn over slow proxies. There was a bug #328 submitted 10 years ago. To fix this, the server-poll-timeout config value was introduced, but has only been implemented for http proxies. For socks the timeouts are still hard-coded to 5 sec. That's why people are still having problems, see issue #267

I submitted a patch to the mailing list, but since this was my first time using a mailing list over git, I failed to provide details in the mail.

With this patch, establishing a socks connection uses the timeout set by server-poll-timeout (default: 120s). I used the same logic like it was implemented for http proxies, which includes the socks handshake for the timeout.

So here's a PR with more details, in hope that it gets reviewed and implemented soon.

[cron2]

A commit needs to have a good description on what is changed and why - there is no difference between "PR" and "on the list" in this. Your commit has a summary, but the description that is here in the PR ("the same logic that was implemented for http proxies") needs to go into the actual commit message - when people look at code change some years from now, github might long be gone (so whatever is written in the PR comments is long history), but the actual git commit message will be still there.

Also, this PR has a merge commit, which it shouldn't have (so always rebase on master before creating a patch).

... but do not despair, we do have the patch on the list, and with the description from the PR, I can construct a more verbose commit message. I just want a review, preferrably from @schwabe who understands all this timeout stuff (having rewritten much of it a few years ago)...

[5andr0]

A commit needs to have a good description on what is changed and why - there is no difference between "PR" and "on the list" in this. Your commit has a summary, but the description that is here in the PR ("the same logic that was implemented for http proxies") needs to go into the actual commit message - when people look at code change some years from now, github might long be gone (so whatever is written in the PR comments is long history), but the actual git commit message will be still there.

I highly appreciate you taking the time to explain how it's done properly. From now on I'll always write multi-line commit messages.

Also, this PR has a merge commit, which it shouldn't have (so always rebase on master before creating a patch).

Rebased and amended a detailed commit message 👍🏻

... but do not despair, we do have the patch on the list, and with the description from the PR, I can construct a more verbose commit message. I just want a review, preferrably from @schwabe who understands all this timeout stuff (having rewritten much of it a few years ago)...

Thanks for taking over!

[ValdikSS]

@cron2 LGTM, tested on 2.5.8

[cron2]

Thanks for testing. By convention, we never merge GH PRs, but merge patches from the mailing list.

This has now been done

commit b3a68b85a729628ca8b97f9f0c2813f795289cfc (master) commit 94bfb712366ece1ca3605d18e99580f482f0232b (release/2.6) Author: 5andr0 Date: Fri Mar 15 17:20:11 2024 +0100

 Implement server_poll_timeout for socks

• so, thanks to all involved.