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

feat: [CURLRequest] add option for Proxy #7632

Merged
merged 2 commits into from
Jun 29, 2023

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jun 28, 2023

Description

  • add request option proxy

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added enhancement PRs that improve existing functionalities 4.4 labels Jun 28, 2023
@kenjis kenjis mentioned this pull request Jun 28, 2023
5 tasks
public function testProxyuOption()
{
$this->request->request('get', 'http://example.com', [
'proxy' => 'http://localhost:3128',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think proxy should be an array that can set CURLOPT_PROXY, CURLOPT_PROXYTYPE , CURLOPT_PROXYPORT and etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems Guzzle does not have such options.
https://docs.guzzlephp.org/en/stable/request-options.html#proxy

Copy link
Collaborator

@ddevsr ddevsr left a comment

Choose a reason for hiding this comment

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

Can flexible option with basic below?

[
    'proxy' => [
        'host' => '127.0.0.1:1337',
        'type' => 'sock5', // sock5 or sock4 or default with CURLPROXY_HTTP
        'auth' => 'user:pass', // or array
    ]
]

@datamweb
Copy link
Contributor

Can flexible option with basic below?

It seems not. Document Guzzle gives an example as below:

$client->request('GET', '/', [
    'proxy' => [
        'http'  => 'http://localhost:8125', // Use this proxy with "http"
        'https' => 'http://localhost:9124', // Use this proxy with "https",
        'no' => ['.mit.edu', 'foo.com']    // Don't use a proxy with these
    ]
]);

However, I think the documents of this PR should clearly indicate how to set these things.

@kenjis
Copy link
Member Author

kenjis commented Jun 28, 2023

@ddevsr

This class is modeled after the Guzzle HTTP Client library since it is one of the more widely used libraries. Where possible, the syntax has been kept the same so that if your application needs something a little more powerful than what this library provides, you will have to change very little to move over to use Guzzle.
https://codeigniter4.github.io/CodeIgniter4/libraries/curlrequest.html

I think CURLRequest is a lightweight Guzzle, and if you need more functionality, use Guzzle.
Therefore, we should avoid extensions of our own.

Copy link
Collaborator

@ddevsr ddevsr left a comment

Choose a reason for hiding this comment

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

Btw dont forget adding note for auth like http://username:[email protected]:10

@jozefrebjak
Copy link
Contributor

@kenjis thanks for this. I really need this as soon as possible in my app.

@kenjis kenjis merged commit b241ca8 into codeigniter4:4.4 Jun 29, 2023
@kenjis kenjis deleted the feat-curlrequest-proxy branch June 29, 2023 23:10
@kenjis
Copy link
Member Author

kenjis commented Jun 29, 2023

@ddevsr If someone needs it, s/he can implement and send a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.4 enhancement PRs that improve existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants