8000 [8.0] Final for no proxy settings for all php versions by gergokee · Pull Request #3164 · guzzle/guzzle · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[8.0] Final for no proxy settings for all php versions #3164

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy stateme 8000 nt. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 12 commits into
base: 8.0
Choose a base branch
from
Open

[8.0] Final for no proxy settings for all php versions #3164

wants to merge 12 commits into from

Conversation

gergokee
Copy link
Contributor
@gergokee gergokee commented Aug 18, 2023 •
edited
Loading

Fix for issue mentioned in #3163

@gergokee gergokee changed the title Fie Fix for https://github.com/guzzle/guzzle/issues/3163 Aug 18, 2023
@gergokee gergokee changed the title Fix for https://github.com/guzzle/guzzle/issues/3163 Final for for no proxy settings for all php versions Aug 18, 2023
@GrahamCampbell
Copy link
Member

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.

@oostapov
Copy link

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.
cURL version: 8.2.1 (hint: php -i | grep cURL) but it worked the same wrong on earlier 8.1 too

@GrahamCampbell
Copy link
Member
GrahamCampbell commented Aug 27, 2023 •
edited
Loading

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)

@oostapov
Copy link

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.

@oostapov
Copy link

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

@gergokee
Copy link
Contributor Author
gergokee commented Aug 28, 2023 •
edited
Loading

@GrahamCampbell

In short: my old fix was not a real fix. :/

Explanation:

The reason is that first i added $conf[\CURLOPT_PROXY] = ''; . This should have been okay. But then tests failed, so i accidentally added this: unset($conf[\CURLOPT_PROXY]); . Which is not a real fix, since we wanted to specifically set CURLOPT_PROXY to be empty, unsetting it does nothing :/

The reason now i want both CURLOPT_PROXY ='' and CURLOPT_NOPROXY set to the added values cause in older php versions CURLOPT_NOPROXY is not working. So setting both is the ultimate solution.

@gergokee gergokee changed the title Final for for no proxy settings for all php versions Final for no proxy settings for all php versions Feb 28, 2024
@GrahamCampbell
Copy link
Member

I'm wondering if Guzzle 8.0 is the best place to fix this. I'm very conscious of breaking things for people.

@GrahamCampbell GrahamCampbell changed the title Final for no proxy settings for all php versions [8.0] Final for no proxy settings for all php versions Mar 31, 2024
@GrahamCampbell GrahamCampbell changed the base branch from 7.7 to 8.0 March 31, 2024 19:44
TravisWhitehead
TravisWhitehead reviewed Apr 19, 2024
View reviewed changes
@stale Stale
Copy link
stale bot commented Feb 25, 2025

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale No activity for a long time label Feb 25, 2025
@GrahamCampbell GrahamCampbell added lifecycle/keep-open Issues that should not be closed and removed lifecycle/stale No activity for a long time labels Feb 25, 2025
@gergokee gergokee mentioned this pull request Mar 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TravisWhitehead TravisWhitehead TravisWhitehead left review comments

@oostapov oostapov oostapov approved these changes

Assignees
No one assigned
Labels
lifecycle/keep-open Issues that should not be closed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gergokee @GrahamCampbell @oostapov @TravisWhitehead
0