-
Notifications
You must be signed in to change notification settings - Fork 87
Fix: Connectivity check fails with Squid/Strict Proxies (Force CONNECT on port 80) #2524
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
base: master
Are you sure you want to change the base?
Conversation
fix proxy_online
WalkthroughThe change modifies the OutgoingProxyLib.php to use a dynamic check URL for NC SI status instead of a hardcoded value, and conditionally enables HTTP proxy tunneling based on whether the check URL uses HTTPS. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/include/OutgoingProxyLib.php`:
- Around line 61-71: The current $checkurl is hardcoded to
"http://www.msftncsi.com/ncsi.txt" which makes the $isHttps check and
curl_setopt($ch, CURLOPT_HTTPPROXYTUNNEL, $isHttps) dead code; update the code
to either (A) make $checkurl configurable (e.g., accept it from a function
parameter or configuration value) so parse_url($checkurl, PHP_URL_SCHEME) can be
'http' or 'https' at runtime and the existing $isHttps logic works, or (B) if
only HTTP will ever be used, remove the conditional and explicitly set
CURLOPT_HTTPPROXYTUNNEL to false to reflect the intended behavior; adjust
references to $checkurl and $isHttps accordingly.
| /*Define the check URL */ | ||
| $checkurl = "http://www.msftncsi.com/ncsi.txt"; | ||
| /* Initialize cURL session. */ | ||
| $ch = curl_init("http://www.msftncsi.com/ncsi.txt"); | ||
|
|
||
| $ch = curl_init($checkurl); | ||
|
|
||
| /* Determine if tunneling is needed (HTTPS only). */ | ||
| $isHttps = (parse_url($checkurl, PHP_URL_SCHEME) === 'https'); | ||
|
|
||
| /* Set cURL options. */ | ||
| curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 3); /* Timeout in seconds. */ | ||
| curl_setopt($ch, CURLOPT_HTTPPROXYTUNNEL, true); | ||
| curl_setopt($ch, CURLOPT_HTTPPROXYTUNNEL, $isHttps); /* Enable HTTP tunneling only for HTTPS to prevent CONNECT method on port 80. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
PHP CURLOPT_HTTPPROXYTUNNEL behavior CONNECT method HTTP HTTPS
💡 Result:
Short answer
- CURLOPT_HTTPPROXYTUNNEL = 0 (default): libcurl sends normal HTTP proxy requests (for HTTP URLs it sends a request with an absolute-URI; for HTTPS URLs it still uses CONNECT automatically).
- CURLOPT_HTTPPROXYTUNNEL = 1: force tunneling through the HTTP proxy — libcurl issues an HTTP CONNECT to the proxy and then pipes raw TCP (so any protocol, including HTTPS, is tunneled).
- Use tunnel mode when you need end‑to‑end TCP passthrough (e.g. FTP or non‑HTTP protocols) or when the proxy must not see/modifiy the HTTP request; note many proxies only allow CONNECT to specific ports (commonly 80/443). [1][2][3]
Examples
- Default (no tunnel): proxy receives "GET http://host/..." for HTTP targets; for HTTPS targets libcurl still uses CONNECT automatically. [1][2]
- Forced tunnel: curl_easy_setopt(..., CURLOPT_HTTPPROXYTUNNEL, 1L) — sends "CONNECT host:port" then forwards traffic. [1][3]
Sources
- curl/libcurl CURLOPT_HTTPPROXYTUNNEL documentation. [1]
- Practical explanation / proxy tunneling behavior (curl guide). [2]
- Discussion/example about CONNECT and proxy handling. [3]
🌐 Web query:
curl CONNECT tunneling proxy HTTP vs HTTPS URLs
💡 Result:
Short answer
- curl uses an HTTP proxy differently depending on the target URL:
- http:// URL → curl sends an absolute-form HTTP request to the proxy (proxy can read/modify the request/response).
- https:// URL → curl by default issues an HTTP CONNECT to the proxy and tunnels a raw TCP stream; curl then performs TLS directly with the origin (proxy cannot see decrypted HTTPS unless it MITM’s it). [1][2]
Additional notes / controls
- You can force tunneling for non-HTTPS (or all traffic) with --proxytunnel (or CURLOPT_HTTPPROXYTUNNEL in libcurl). That makes curl send CONNECT to the proxy and pass bytes through the tunnel. [2][3]
- The proxy URL scheme matters for the connection between you and the proxy:
- -x http://proxy:port → plaintext connection to the proxy.
- -x https://proxy:port → TLS to the proxy (an “HTTPS proxy”); the proxy-to-origin behavior (CONNECT for https:// targets) is the same, but the client→proxy leg is encrypted. [4][1]
Common failures
- If the proxy does not support CONNECT (or blocks the destination/port), HTTPS through that proxy will fail (405/403 or connection drop). Use -v to see the CONNECT exchange. [3]
Sources
- curl docs — HTTP proxies and tunneling. [everything.curl.dev].
- libcurl option CURLOPT_HTTPPROXYTUNNEL (man page).
- curl proxy guide / troubleshooting (examples of CONNECT behaviour).
- curl docs — HTTPS proxy explanation.
The conditional logic is correct in principle, but becomes dead code for the hardcoded HTTP URL.
The cURL behavior is verified: CURLOPT_HTTPPROXYTUNNEL=0 prevents CONNECT tunneling for HTTP URLs, while =1 enables it for HTTPS. However, since $checkurl is hardcoded to http://www.msftncsi.com/ncsi.txt, the condition parse_url($checkurl, PHP_URL_SCHEME) === 'https' will always be false, so CURLOPT_HTTPPROXYTUNNEL will always be disabled. Either remove the conditional and hardcode it to false, or make $checkurl configurable to support both HTTP and HTTPS URLs in practice.
🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/include/OutgoingProxyLib.php` around lines 61 - 71,
The current $checkurl is hardcoded to "http://www.msftncsi.com/ncsi.txt" which
makes the $isHttps check and curl_setopt($ch, CURLOPT_HTTPPROXYTUNNEL, $isHttps)
dead code; update the code to either (A) make $checkurl configurable (e.g.,
accept it from a function parameter or configuration value) so
parse_url($checkurl, PHP_URL_SCHEME) can be 'http' or 'https' at runtime and the
existing $isHttps logic works, or (B) if only HTTP will ever be used, remove the
conditional and explicitly set CURLOPT_HTTPPROXYTUNNEL to false to reflect the
intended behavior; adjust references to $checkurl and $isHttps accordingly.
|
The purpose of the test is to confirm whether the server can read http://www.msftncsi.com/ncsi.txt over the proxy. If the proxy you want to use does not support http then the proxy needs to be improved, it does not make sense to disable the http check for everyone else. Or what am I missing? |
The Issue
The current implementation forces
CURLOPT_HTTPPROXYTUNNELtotruefor all connectivity checks.The check URL is currently
http://www.msftncsi.com/ncsi.txt(Plain HTTP).When
CURLOPT_HTTPPROXYTUNNELis enabled on an HTTP URL, cURL sends aCONNECTmethod request on port 80 (instead of a standardGET).Most strict proxy configurations (like Squid's default
safe_ports) explicitly block theCONNECTmethod on non-SSL ports to prevent protocol tunneling security risks.This causes Unraid to incorrectly report "Offline" status even when connectivity is fine.
The Fix
This PR refactors
proxy_onlineto make the tunneling option conditional:$checkurl.https://-> Enable Tunneling (True).http://-> Disable Tunneling (False).This ensures compatibility with standard proxy configurations while remaining robust if the check URL is changed to HTTPS in the future.
References / Community Reports
https://forums.unraid.net/topic/190733-712-outgoing-proxy-how/
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.