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

[9.x] Skip parameter parsing for raw post body in HTTP Client #42364

Merged

Conversation

Krisell
Copy link
Contributor

@Krisell Krisell commented May 12, 2022

Fixes #42349
Related to #36976

When sending an Http::withBody() using the Http client, the raw body is parsed as query/form parameters. This seems wrong and is both unnecessary work and risks hitting the default limit of 1000 "input variables" in php.

Http::withBody(str_repeat('A thousand &. ', 1000), 'text/plain')->post('https://some-url.com');
ErrorException

parse_str(): Input variables exceeded 1000. To increase the limit change max_input_vars in php.ini.

The limit of 1000 is quite easily hit by sending HTML, XML or similar text.

The suggested solution here skips all parameter parsing of the body, also saving a fair bit of work. All tests pass, but is there a specific reason to parse the body this way, or might that have been introduced by mistake in order to parse query and form-data parameters?

Our use case is sending user-provided HTML to an external service (for text analysis) and while being very rare, we sometimes see the 1000 limit being hit via Bugsnag.

Thanks to @pelmered for suggestion of simplified flow (early return).

@driesvints driesvints changed the title Skip parameter parsing for raw post body in HTTP Client [9.x] Skip parameter parsing for raw post body in HTTP Client May 12, 2022
@taylorotwell taylorotwell merged commit 17f1974 into laravel:9.x May 12, 2022
@driesvints
Copy link
Member

Thanks @Krisell

chu121su12 pushed a commit to chu121su12/framework that referenced this pull request May 20, 2022
…l#42364)

* Skip parameter parsing for raw post body in HTTP Client

* Reduce test string length
@Kuzuto
Copy link

Kuzuto commented Oct 6, 2022

Please add this update to Laravel 8.x

@driesvints
Copy link
Member

@Kuzuto 8.x isn't maintained anymore. Please upgrade to 9.x

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.

HTTP Client unable to post raw body with more than a thousand ampersands
4 participants