Skip to content

HttpClientDataCollector fails if proc_open is disabled via php.ini #58700

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 statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ZaneCEO opened this issue Oct 28, 2024 · 3 comments
Closed

HttpClientDataCollector fails if proc_open is disabled via php.ini #58700

ZaneCEO opened this issue Oct 28, 2024 · 3 comments
Labels
Bug HttpClient Status: Needs Review

Comments

@ZaneCEO
Copy link
Contributor

ZaneCEO commented Oct 28, 2024

Symfony version(s) affected

7.1.6

Description

HttpClientDataCollector::escapePayload creates a new Process() -> it fails if proc_open is disabled:

The Process class relies on proc_open, which is not available on your PHP installation.

How to reproduce

        $request =
            $this->httpClient->request('POST', 'https://www.linkedin.com/oauth/v2/accessToken', [
                "body"  => [
                    "grant_type"    => "authorization_code",
                    "code"          => "11111111",
                    "redirect_uri"  => 'https://example.com',
                    "client_id"     => 'abc',
                    "client_secret" => 'ddd'
                ]
            ]);

        $content = $request->getContent(false);

        throw new \Exception();

Possible Solution

escapePayload could be modified to avoid new Process() and generate the command in-place.

Additional Context

escapePayload is called only if the request has either a json content or a body.
@ZaneCEO ZaneCEO added the Bug label Oct 28, 2024
@nicolas-grekas
Copy link
Member

Ah indeed.

The fix should be this:

if ($useProcess ??= function_exists('proc_open') && class_exists(Process::class)) {

Can you please send a PR targetting branch 5.4?

@ZaneCEO
Copy link
Contributor Author

ZaneCEO commented Oct 29, 2024

Great, thanks for the feedback!

I'll gladly prepare the PR later today

ZaneCEO added a commit to ZaneCEO/symfony that referenced this issue Oct 29, 2024
@ZaneCEO
fix for HttpClientDataCollector fails if proc_open is disabled via ph…
48980a2 
…p.ini . Closes symfony#58700
@ZaneCEO
Copy link
Contributor Author

ZaneCEO commented Oct 29, 2024 •
edited
Loading

The function is not present in:

It looks like it was introduced in 6.3

I submitted the PR

@ZaneCEO ZaneCEO mentioned this issue Oct 29, 2024
Merged
nicolas-grekas added a commit that referenced this issue Oct 30, 2024
@nicolas-grekas
bug #58704 [HttpClient] fix for HttpClientDataCollector fails if proc…
22df28a 
…_open is disabled via php.ini (ZaneCEO)

This PR was merged into the 6.4 branch.

Discussion
----------

[HttpClient] fix for HttpClientDataCollector fails if proc_open is disabled via php.ini

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #58700
| License       | MIT

[HttpClientDataCollector::escapePayload](https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php#L256) creates a `new Process()` -> it fails if `proc_open` is disabled:

> The Process class relies on proc_open, which is not available on your PHP installation.

#58700

Commits
-------

e28af34 [HttpClient] Fix Process-based escaping in HttpClientDataCollector
48980a2 fix for HttpClientDataCollector fails if proc_open is disabled via php.ini . Closes #58700
symfony-splitter pushed a commit to symfony/http-client that referenced this issue Oct 30, 2024
@nicolas-grekas
bug #58704 [HttpClient] fix for HttpClientDataCollector fails if proc…
224124d 
…_open is disabled via php.ini (ZaneCEO)

This PR was merged into the 6.4 branch.

Discussion
----------

[HttpClient] fix for HttpClientDataCollector fails if proc_open is disabled via php.ini

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #58700
| License       | MIT

[HttpClientDataCollector::escapePayload](https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php#L256) creates a `new Process()` -> it fails if `proc_open` is disabled:

> The Process class relies on proc_open, which is not available on your PHP installation.

symfony/symfony#58700

Commits
-------

e28af345004 [HttpClient] Fix Process-based escaping in HttpClientDataCollector
48980a2f05a fix for HttpClientDataCollector fails if proc_open is disabled via php.ini . Closes #58700
nicolas-grekas added a commit that referenced this issue Oct 30, 2024
@nicolas-grekas
Merge branch '6.4' into 7.1
cafbbf2 
* 6.4:
  [HttpClient] Fix Process-based escaping in HttpClientDataCollector
  fix for HttpClientDataCollector fails if proc_open is disabled via php.ini . Closes #58700
  Bump Symfony version to 6.4.14
  Update VERSION for 6.4.13
  Update CHANGELOG for 6.4.13
  Bump Symfony version to 5.4.46
  Update VERSION for 5.4.45
  Update CONTRIBUTORS for 5.4.45
  Update CHANGELOG for 5.4.45
nicolas-grekas added a commit that referenced this issue Oct 30, 2024
@nicolas-grekas
Merge branch '7.1' into 7.2
c1a3c42 
* 7.1:
  [HttpClient] Fix Process-based escaping in HttpClientDataCollector
  fix for HttpClientDataCollector fails if proc_open is disabled via php.ini . Closes #58700
  Bump Symfony version to 7.1.7
  Update VERSION for 7.1.6
  Update CHANGELOG for 7.1.6
  Bump Symfony version to 6.4.14
  Update VERSION for 6.4.13
  Update CHANGELOG for 6.4.13
  Bump Symfony version to 5.4.46
  Update VERSION for 5.4.45
  Update CONTRIBUTORS for 5.4.45
  Update CHANGELOG for 5.4.45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpClient Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @xabbuh @ZaneCEO @carsonbot and others