-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[8.0] Final for no proxy settings for all php versions #3164
base: 8.0
Are you sure you want to change the base?
Conversation
Is this an issue with curl or PHP? I am reluctant to revert this and re-break the issue this was fixing. Do you know the exact last version of libcurl that has this issue, if this a PHP issue, the exact last version of PHP to have this issue? I'll leave this PR on hold till we have that information. |
It is a curl issue, not php. |
Ok, so what is the latest version of curl where this is broken, so we can decide if this is actually something we want to support? (not what is the latest version of curl) |
As I said it is broken on the 8.2.1 which is currently latest stable. It has been broken and worked this way for years. 7.29 is the oldest I can test with and since that version it has never worked correctly. |
I believe it has nothing to do with cURL as the cURL behaves quite predictable. It is an improper handling of the proxy settings by the guzzle. If we want to not use proxy for some domain we should force the curl having it in NO_PROXY setting instead of leaving it unset which leads to falling back to the ENV settings |
In short: my old fix was not a real fix. :/ Explanation: The reason is that first i added The reason now i want both |
I'm wondering if Guzzle 8.0 is the best place to fix this. I'm very conscious of breaking things for people. |
} else { | ||
$conf[\CURLOPT_PROXY] = $options['proxy'][$scheme]; | ||
} |
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.
Although this covers the case where our request matches the ['proxy']['no']
configuration (and we want it not to use the proxy), it misses an inverse case. :)
Image that in the environment variables there's NO_PROXY="foo.bar"
/ no_proxy="foo.bar"
, and we want to configure guzzle to override/ignore this (to proxy everything, or at least proxy requests to foo.bar
).
If we set:
'proxy' => [
'http' => 'http://example:8080',
'https' => 'http://example:8080',
# Send everything through the proxy
'no' => []
]
Then the environment's behavior will still get used and requests to foo.bar
will (unexpectedly) not go through the proxy.
Perhaps something like this in the else branch?
} else { | |
$conf[\CURLOPT_PROXY] = $options['proxy'][$scheme]; | |
} | |
} else { | |
$conf[\CURLOPT_PROXY] = $options['proxy'][$scheme]; | |
$conf[\CURLOPT_NOPROXY] = ""; | |
} |
(Related comment on the issue: #3163 (comment))
Fix for issue mentioned in #3163