Skip to content

Commit 4a05752

Browse files
bug #46700 [HttpClient] Prevent "Fatal error" in data collector (fmata)
This PR was merged into the 6.1 branch. Discussion ---------- [HttpClient] Prevent "Fatal error" in data collector | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46604 | License | MIT Since "copy as cURL" has been added in Profiler for HttpClient usage, when payloads exceed the size allowed by escapeshellarg() we have a Fatal error 😞 The right allowed size depends on OS and is not leaked in PHP land so I prefer to be conservative and allow the lower usual value as explained in PHP source code https://github.com/php/php-src/blob/9458f5f2c8a8e3d6c65cc181747a5a75654b7c6e/ext/standard/exec.c#L77 I only added check where I think it's relevant. Commits ------- 601ef63f46 [HttpClient] Prevent "Fatal error" on escapeshellarg() call when the size of its argument is big
2 parents 5dc4bf0 + 1e23ccc commit 4a05752

File tree

2 files changed

+43
-2
lines changed

2 files changed

+43
-2
lines changed

DataCollector/HttpClientDataCollector.php

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,15 @@ private function getCurlCommand(array $trace): ?string
193193
$dataArg = [];
194194

195195
if ($json = $trace['options']['json'] ?? null) {
196-
$dataArg[] = '--data '.escapeshellarg(self::jsonEncode($json));
196+
if (!$this->argMaxLengthIsSafe($payload = self::jsonEncode($json))) {
197+
return null;
198+
}
199+
$dataArg[] = '--data '.escapeshellarg($payload);
197200
} elseif ($body = $trace['options']['body'] ?? null) {
198201
if (\is_string($body)) {
202+
if (!$this->argMaxLengthIsSafe($body)) {
203+
return null;
204+
}
199205
try {
200206
$dataArg[] = '--data '.escapeshellarg($body);
201207
} catch (\ValueError) {
@@ -204,7 +210,10 @@ private function getCurlCommand(array $trace): ?string
204210
} elseif (\is_array($body)) {
205211
$body = explode('&', self::normalizeBody($body));
206212
foreach ($body as $value) {
207-
$dataArg[] = '--data '.escapeshellarg(urldecode($value));
213+
if (!$this->argMaxLengthIsSafe($payload = urldecode($value))) {
214+
return null;
215+
}
216+
$dataArg[] = '--data '.escapeshellarg($payload);
208217
}
209218
} else {
210219
return null;
@@ -240,4 +249,14 @@ private function getCurlCommand(array $trace): ?string
240249

241250
return implode(" \\\n ", $command);
242251
}
252+
253+
/**
254+
* Let's be defensive : we authorize only size of 8kio on Windows for escapeshellarg() argument to avoid a fatal error
255+
*
256+
* @see https://github.com/php/php-src/blob/9458f5f2c8a8e3d6c65cc181747a5a75654b7c6e/ext/standard/exec.c#L397
257+
*/
258+
private function argMaxLengthIsSafe(string $payload): bool
259+
{
260+
return \strlen($payload) < ('\\' === \DIRECTORY_SEPARATOR ? 8100 : 256000);
261+
}
243262
}

Tests/DataCollector/HttpClientDataCollectorTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,28 @@ public function testItDoesNotGeneratesCurlCommandsForNotEncodableBody()
417417
self::assertNull($curlCommand);
418418
}
419419

420+
/**
421+
* @requires extension openssl
422+
*/
423+
public function testItDoesNotGeneratesCurlCommandsForTooBigData()
424+
{
425+
$sut = new HttpClientDataCollector();
426+
$sut->registerClient('http_client', $this->httpClientThatHasTracedRequests([
427+
[
428+
'method' => 'POST',
429+
'url' => 'http://localhost:8057/json',
430+
'options' => [
431+
'body' => str_repeat('1', 257000),
432+
],
433+
],
434+
]));
435+
$sut->collect(new Request(), new Response());
436+
$collectedData = $sut->getClients();
437+
self::assertCount(1, $collectedData['http_client']['traces']);
438+
$curlCommand = $collectedData['http_client']['traces'][0]['curlCommand'];
439+
self::assertNull($curlCommand);
440+
}
441+
420442
private function httpClientThatHasTracedRequests($tracedRequests): TraceableHttpClient
421443
{
422444
$httpClient = new TraceableHttpClient(new NativeHttpClient());

0 commit comments

Comments
 (0)