fix: RemoteFileFetcher::fetch() prüft die URL selbst; Doppelprüfung im Schriften-Upload entfällt
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
bfede30d33
commit
942e4220ed
@@ -325,12 +325,6 @@ final class DiviFontsUploadAbility implements Ability, RequiresDiviFeatures
|
|||||||
$problems = [];
|
$problems = [];
|
||||||
$unreachable = false;
|
$unreachable = false;
|
||||||
foreach ($urls as $format => $url) {
|
foreach ($urls as $format => $url) {
|
||||||
// fetch() does not check the URL itself: a refused URL is never requested (same answer as a failed one).
|
|
||||||
if (! $this->fetcher->allows($url)) {
|
|
||||||
$problems['files.' . $format] = 'nicht abrufbar';
|
|
||||||
$unreachable = true;
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
$tmp = (string) wp_tempnam('lit-mcp-font');
|
$tmp = (string) wp_tempnam('lit-mcp-font');
|
||||||
if ($tmp === '') {
|
if ($tmp === '') {
|
||||||
return Errors::writeFailed('Die temporäre Datei konnte nicht angelegt werden.');
|
return Errors::writeFailed('Die temporäre Datei konnte nicht angelegt werden.');
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ class RemoteFileFetcher
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Streams $url into $tmp; the caller checks allows() first and the size of $tmp afterwards (a body above
|
* Streams $url into $tmp; fetch() checks the URL itself (allows()), the caller checks the size of $tmp afterwards (a body above
|
||||||
* $maxBytes is cut off at $maxBytes + 1 bytes).
|
* $maxBytes is cut off at $maxBytes + 1 bytes).
|
||||||
*
|
*
|
||||||
* @param string $context Names the download in the failure log, e.g. "media import".
|
* @param string $context Names the download in the failure log, e.g. "media import".
|
||||||
@@ -50,6 +50,10 @@ class RemoteFileFetcher
|
|||||||
*/
|
*/
|
||||||
public function fetch(string $url, string $tmp, int $maxBytes, string $context): ?array
|
public function fetch(string $url, string $tmp, int $maxBytes, string $context): ?array
|
||||||
{
|
{
|
||||||
|
if (! $this->allows($url)) {
|
||||||
|
return $this->failed($context, $url, 'URL refused');
|
||||||
|
}
|
||||||
|
|
||||||
// IPv4 only, for this request only: wp_http_validate_url() checks the IPv4 address
|
// IPv4 only, for this request only: wp_http_validate_url() checks the IPv4 address
|
||||||
// (gethostbyname), so an AAAA record must not lead cURL to an unchecked IPv6 target.
|
// (gethostbyname), so an AAAA record must not lead cURL to an unchecked IPv6 target.
|
||||||
$ipv4 = static function ($handle): void {
|
$ipv4 = static function ($handle): void {
|
||||||
|
|||||||
@@ -118,6 +118,9 @@ final class DiviFontsUploadAbilityTest extends TestCase
|
|||||||
/** The fake fetcher's download: writes the body of $url into $tmp. */
|
/** The fake fetcher's download: writes the body of $url into $tmp. */
|
||||||
public function fakeFetch(string $url, string $tmp, int $maxBytes, string $context): ?array
|
public function fakeFetch(string $url, string $tmp, int $maxBytes, string $context): ?array
|
||||||
{
|
{
|
||||||
|
if (! $this->allowed($url)) { // like the real fetch(): a refused URL is never requested
|
||||||
|
return null;
|
||||||
|
}
|
||||||
$this->fetches[] = ['url' => $url, 'tmp' => $tmp, 'max' => $maxBytes, 'context' => $context];
|
$this->fetches[] = ['url' => $url, 'tmp' => $tmp, 'max' => $maxBytes, 'context' => $context];
|
||||||
$body = $this->bodies[$url] ?? null;
|
$body = $this->bodies[$url] ?? null;
|
||||||
if ($body === null) {
|
if ($body === null) {
|
||||||
@@ -258,7 +261,7 @@ final class DiviFontsUploadAbilityTest extends TestCase
|
|||||||
|
|
||||||
self::assertToolError($result, 'font_fetch_failed');
|
self::assertToolError($result, 'font_fetch_failed');
|
||||||
self::assertSame(['files.ttf' => 'nicht abrufbar'], self::errorFields($result));
|
self::assertSame(['files.ttf' => 'nicht abrufbar'], self::errorFields($result));
|
||||||
self::assertSame([self::WOFF2_URL], array_column($this->fetches, 'url'), 'fetch() skipped the refused URL');
|
self::assertSame([self::WOFF2_URL], array_column($this->fetches, 'url'), 'the refused URL was never requested');
|
||||||
$this->assertTempFilesRemoved();
|
$this->assertTempFilesRemoved();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -84,6 +84,15 @@ final class RemoteFileFetcherTest extends TestCase
|
|||||||
self::assertSame('wOF2DATA', file_get_contents($this->tmp));
|
self::assertSame('wOF2DATA', file_get_contents($this->tmp));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testARefusedUrlIsNullAndNeverRequested(): void
|
||||||
|
{
|
||||||
|
Functions\expect('wp_safe_remote_get')->never();
|
||||||
|
|
||||||
|
self::assertNull($this->fetcher(['10.0.0.5'])->fetch('https://intern.example.com/x.woff2', $this->tmp, 2048, 'x'));
|
||||||
|
self::assertNull($this->fetcher()->fetch('file:///etc/passwd', $this->tmp, 2048, 'x'));
|
||||||
|
self::assertStringContainsString('URL refused', (string) file_get_contents($this->errorLog));
|
||||||
|
}
|
||||||
|
|
||||||
public function testAFailedRequestIsNullAndOnlyTheLogNamesTheReasonAndTheContext(): void
|
public function testAFailedRequestIsNullAndOnlyTheLogNamesTheReasonAndTheContext(): void
|
||||||
{
|
{
|
||||||
Functions\when('wp_safe_remote_get')->justReturn(new WP_Error('http_request_failed', 'Connection refused'));
|
Functions\when('wp_safe_remote_get')->justReturn(new WP_Error('http_request_failed', 'Connection refused'));
|
||||||
|
|||||||
Reference in New Issue
Block a user