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

Issue overwriting default Http Client options using withOptions #40952

Closed
nhedger opened this issue Feb 11, 2022 · 1 comment
Closed

Issue overwriting default Http Client options using withOptions #40952

nhedger opened this issue Feb 11, 2022 · 1 comment

Comments

@nhedger
Copy link
Contributor

nhedger commented Feb 11, 2022

  • Laravel Version: 9.0.2 (8.x also affected)
  • PHP Version: 8.1.0
  • Database Driver & Version: N/A

Description:

Using the bundled Http Client, when passing a custom option using the withOptions method, if a default value is already set for that option, they will be placed into an array containing both values instead of overwriting the default value with the provided one. This behavior is due to the use of array_merge_recursive, which merges string keys with the same name into an array, as documented.

If the input arrays have the same string keys, then the values for these keys are merged together into an array, and this is done recursively, so that if one of the values is an array itself, the function will merge it with a corresponding entry in another array too.

https://www.php.net/manual/en/function.array-merge-recursive.php

Since Laravel 9, a default connect_timeout value is set, so specifiying a custom connect_timeout with withOptions results in Laravel passing an array of timeouts to Guzzle instead of passing our custom connect_timeout. This problems is already present in 8.x versions but since only the http_errors options had a default value, it may have gone unnoticed.

Steps To Reproduce:

Repository to demonstrate the issue: https://github.com/nhedger/bug-report-http-client

I've placed the code in the default route.

Route::get('/', function () {
    return Http::withOptions([
        'connect_timeout' => 10
    ])->get('https://example.com');
});

Browse to the / route to see error.

Possible solution

Using array_replace_recursive instead of array_merge_recursive seems to do the job.

If a key from the first array exists in the second array, its value will be replaced by the value from the second array. If the key exists in the second array, and not the first, it will be created in the first array. If a key only exists in the first array, it will be left as is.

https://www.php.net/array_replace_recursive

@driesvints
Copy link
Member

Hey @nhedger, I think you're correct. I've sent in a PR for this although I'm not 100% it's a correct fix. Let's see how it goes.

#40954

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

No branches or pull requests

2 participants