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.

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

Http client parses request body with parse_str and runs into input variables exceeded limit. #36976

Closed
briedis opened this issue Apr 13, 2021 · 4 comments
Closed

Http client parses request body with parse_str and runs into input variables exceeded limit. #36976

briedis opened this issue Apr 13, 2021 · 4 comments
Labels
bug

Comments

@briedis
Copy link

briedis commented Apr 13, 2021

  • Laravel Version: 8.37.0
  • PHP Version: 8.0.2
  • Database Driver & Version: Does not apply

Description:

\Illuminate\Http\Client\PendingRequest::parseRequestData method uses parse_str if laravelData variable is a string, even if the contents are not a query string. This method is restricted by PHP ini setting max_input_vars. With some payloads (XML, e.g.) this can be easily exceeded by accident.

Steps To Reproduce:

Generate a valid XML with "looks-like-query-params" contents.

$content = str_repeat("<tag>&amp;=</tag>", 1000);
$xml = '<?xml version="1.0" encoding="UTF-8"?><body>'. $content. '</body>';
ini_set('max_input_vars', '1000'); // This is usually the default setting
\Http::withBody($xml, 'application/octet-stream')->post('does-not-matter');

This code will trigger an exception: parse_str(): Input variables exceeded 1000. To increase the limit change max_input_vars in php.ini.
@briedis
Copy link
Author

briedis commented Apr 13, 2021 •
edited
Loading

I tried to pass a resource, since PHPDOC clearly stated that the withBody method accepts either resource or string.

$content = str_repeat("<tag>&amp;=</tag>", 1000);
$xml = '<?xml version="1.0" encoding="UTF-8"?><body>'. $content. '</body>';

// Create a stream instead of a regular string
$xmlStream = fopen('php://memory', 'r+');
fwrite($xmlStream, $xml);
rewind($xmlStream);
        
\Http::withBody($xmlStream, 'application/octet-stream')->post('does-not-matter');

image

But this dies with a different error somewhere really deep:

TypeError : Illuminate\Http\Client\Request::withData(): Argument #1 ($data) must be of type array, resource given, called in /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php on line 899
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/Request.php:240
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:899
 /path/vendor/laravel/framework/src/Illuminate/Support/helpers.php:263
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:902
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:806
 /path/vendor/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php:64
 /path/vendor/guzzlehttp/guzzle/src/Middleware.php:37
 /path/vendor/guzzlehttp/guzzle/src/RedirectMiddleware.php:71
 /path/vendor/guzzlehttp/guzzle/src/Middleware.php:61
 /path/vendor/guzzlehttp/guzzle/src/HandlerStack.php:75
 /path/vendor/guzzlehttp/guzzle/src/Client.php:331
 /path/vendor/guzzlehttp/guzzle/src/Client.php:168
 /path/vendor/guzzlehttp/guzzle/src/Client.php:187
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:717
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:655
 /path/vendor/laravel/framework/src/Illuminate/Support/helpers.php:234
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:665
 /path/vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php:552

This probably deserves a different ticket.

@taylorotwell
Copy link
Member

Are you able to send a PR with a fix?

@briedis
Copy link
Author

briedis commented Apr 14, 2021 •
edited
Loading

Unfortunately not, as I don't fully understand the purpose of parsing the string.

I think that the purpose of it is to handle cases when it's a regular form body in the var=value&var=value format.
Maybe for such cases, we should check the content-type value? If it's empty (dev has not provided any specific type like XML or JSON) or application/x-www-form-urlencoded, only then attempt to parse it. If it's something else, like text/xml, parsing should not be done.

Edit: One moment, trying to make a PR.

@briedis briedis mentioned this issue Apr 14, 2021
Closed
@briedis
Copy link
Author

briedis commented Apr 14, 2021

It appears that my PR also solves the issue with a resource/stream as the body, added a test for that case too.

@driesvints driesvints added the bug label Apr 15, 2021
This was referenced May 12, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[8.x] Http client fix to prevent str_parse for bodies that are not encoded forms briedis/framework
3 participants
@taylorotwell @briedis @driesvints