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

Support the cURL (http://) scheme for StreamHandler proxies #2850

Merged
merged 5 commits into from Mar 7, 2021

Conversation

TimWolla
Copy link
Contributor

@TimWolla TimWolla commented Feb 1, 2021

Fixes #2848

TimWolla added a commit to WoltLab/WCF that referenced this pull request Feb 1, 2021
@@ -803,7 +803,7 @@ Pass a string to specify a proxy for all protocols.

.. code-block:: php

$client->request('GET', '/', ['proxy' => 'tcp://localhost:8125']);
$client->request('GET', '/', ['proxy' => 'http://localhost:8125']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the previous code still work too?

Copy link
Contributor Author

@TimWolla TimWolla Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short answer: Yes.

Long answer:

tcp:// never worked with cURL, only with the StreamHandler.

For the StreamHandler the tcp:// version still works, because in parse_proxy I specifically check for the http scheme and return the URL as-is if it is not using the http scheme (https://github.com/guzzle/guzzle/pull/2850/files#diff-0cdc13d47562373b13de07ef6ea57235f3c5dc23a7cdf33fa47cd96c55ad8bacR424).

However it is not recommended to use tcp:// variant any longer. It is worse in every regard, because it does not support authentication and it is incompatible with cURL. That's why I adjusted the Docs.

@GrahamCampbell
Copy link
Member

Thanks for taking the time to work on this PR. :)

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Feb 11, 2021

Once another member of the team has reviewed this and is happy, this can be merged. :shipit:

@Nyholm
Copy link
Member

Nyholm commented Mar 7, 2021

Thank you

@GrahamCampbell GrahamCampbell added this to the 7.3.0 milestone Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proxy syntax not consistent across handlers
3 participants