fix: expose safe remote upload errors
This commit is contained in:
@@ -67,7 +67,7 @@ class UploadImage extends \sammo\BaseAPI
|
|||||||
RemoteUserIconUploadClient::uploadContentConfigured($imgFullName, $contentType, $imageData);
|
RemoteUserIconUploadClient::uploadContentConfigured($imgFullName, $contentType, $imageData);
|
||||||
$remotePath = RemoteUserIconUploadClient::getConfiguredContentPublicUrl($imgFullName);
|
$remotePath = RemoteUserIconUploadClient::getConfiguredContentPublicUrl($imgFullName);
|
||||||
} catch (\Throwable $error) {
|
} catch (\Throwable $error) {
|
||||||
error_log('Remote content image upload failed: ' . $error->getMessage());
|
RemoteUserIconUploadClient::logFailure('content-image', $error);
|
||||||
return '원격 이미지 저장소 업로드에 실패했습니다!';
|
return '원격 이미지 저장소 업로드에 실패했습니다!';
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -90,7 +90,7 @@ if(!is_uploaded_file($image['tmp_name'])) {
|
|||||||
$newPicName = "users/core/{$remoteName}";
|
$newPicName = "users/core/{$remoteName}";
|
||||||
$storedRemotely = true;
|
$storedRemotely = true;
|
||||||
} catch (\Throwable $error) {
|
} catch (\Throwable $error) {
|
||||||
error_log('Remote user icon upload failed: ' . $error->getMessage());
|
RemoteUserIconUploadClient::logFailure('user-icon', $error);
|
||||||
$storedRemotely = false;
|
$storedRemotely = false;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -2,8 +2,27 @@
|
|||||||
|
|
||||||
namespace sammo;
|
namespace sammo;
|
||||||
|
|
||||||
|
final class RemoteImageUploadException extends \RuntimeException
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
final class RemoteUserIconUploadClient
|
final class RemoteUserIconUploadClient
|
||||||
{
|
{
|
||||||
|
private const SAFE_EXACT_ERRORS = [
|
||||||
|
'Invalid image upload client',
|
||||||
|
'Image upload secret must be at least 32 characters',
|
||||||
|
'Invalid image upload URL',
|
||||||
|
'Image upload URL must use HTTPS except for loopback tests',
|
||||||
|
'Remote user icon upload is not configured',
|
||||||
|
'Remote user icon upload secret file is not configured',
|
||||||
|
'Remote user icon upload secret file cannot be read',
|
||||||
|
'Remote user icon upload secret is too short',
|
||||||
|
'Unable to initialize image upload request',
|
||||||
|
'Image upload returned invalid JSON',
|
||||||
|
'Image upload returned an unsuccessful response',
|
||||||
|
'Image upload returned an unexpected path',
|
||||||
|
];
|
||||||
|
|
||||||
public static function isConfiguredEnabled(): bool
|
public static function isConfiguredEnabled(): bool
|
||||||
{
|
{
|
||||||
return property_exists(ServConfig::class, 'remoteUserIconUploadEnabled')
|
return property_exists(ServConfig::class, 'remoteUserIconUploadEnabled')
|
||||||
@@ -42,22 +61,84 @@ final class RemoteUserIconUploadClient
|
|||||||
return "{$baseUrl}/uploads/core/{$filename}";
|
return "{$baseUrl}/uploads/core/{$filename}";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Record a caught upload failure in both the PHP service log and the
|
||||||
|
* operator-facing SQLite log without persisting request arguments, response
|
||||||
|
* bodies, headers, or secret values.
|
||||||
|
*/
|
||||||
|
public static function logFailure(
|
||||||
|
string $operation,
|
||||||
|
\Throwable $error,
|
||||||
|
?callable $systemLogger = null,
|
||||||
|
?callable $structuredLogger = null
|
||||||
|
): void {
|
||||||
|
$label = match ($operation) {
|
||||||
|
'user-icon' => 'Remote user icon upload',
|
||||||
|
'content-image' => 'Remote content image upload',
|
||||||
|
default => 'Remote image upload',
|
||||||
|
};
|
||||||
|
$reason = self::safeFailureReason($error);
|
||||||
|
$message = "{$label} failed: {$reason}";
|
||||||
|
|
||||||
|
if ($systemLogger === null) {
|
||||||
|
error_log($message);
|
||||||
|
} else {
|
||||||
|
$systemLogger($message);
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
$arguments = [
|
||||||
|
'RemoteImageUploadFailure',
|
||||||
|
$message,
|
||||||
|
$error->getFile() . ':' . $error->getLine(),
|
||||||
|
[],
|
||||||
|
];
|
||||||
|
if ($structuredLogger === null) {
|
||||||
|
logError(...$arguments);
|
||||||
|
} else {
|
||||||
|
$structuredLogger(...$arguments);
|
||||||
|
}
|
||||||
|
} catch (\Throwable $loggingError) {
|
||||||
|
error_log('Remote image upload structured logging failed: ' . get_debug_type($loggingError));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static function safeFailureReason(\Throwable $error): string
|
||||||
|
{
|
||||||
|
if ($error instanceof RemoteImageUploadException || $error instanceof \InvalidArgumentException) {
|
||||||
|
$message = $error->getMessage();
|
||||||
|
if (in_array($message, self::SAFE_EXACT_ERRORS, true)
|
||||||
|
|| ($error instanceof RemoteImageUploadException
|
||||||
|
&& preg_match('/^(?:Image upload rejected \([1-5][0-9]{2}\)|Image upload request failed \(cURL [0-9]+\))$/D', $message))) {
|
||||||
|
return $message;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 'Unexpected ' . get_debug_type($error);
|
||||||
|
}
|
||||||
|
|
||||||
/** @return array{string,string} */
|
/** @return array{string,string} */
|
||||||
private static function configuredBaseUrlAndSecret(): array
|
private static function configuredBaseUrlAndSecret(): array
|
||||||
{
|
{
|
||||||
if (!self::isConfiguredEnabled()
|
if (!self::isConfiguredEnabled()
|
||||||
|| !property_exists(ServConfig::class, 'remoteUserIconUploadPath')
|
|| !property_exists(ServConfig::class, 'remoteUserIconUploadPath')
|
||||||
|| !property_exists(ServConfig::class, 'remoteUserIconUploadSecretFile')) {
|
|| !property_exists(ServConfig::class, 'remoteUserIconUploadSecretFile')) {
|
||||||
throw new \RuntimeException('Remote user icon upload is not configured');
|
throw new RemoteImageUploadException('Remote user icon upload is not configured');
|
||||||
}
|
}
|
||||||
$secretPath = ServConfig::$remoteUserIconUploadSecretFile;
|
$secretPath = ServConfig::$remoteUserIconUploadSecretFile;
|
||||||
if (!is_string($secretPath) || $secretPath === '' || str_contains($secretPath, "\0")) {
|
if (!is_string($secretPath) || $secretPath === '' || str_contains($secretPath, "\0")) {
|
||||||
throw new \RuntimeException('Remote user icon upload secret file is not configured');
|
throw new RemoteImageUploadException('Remote user icon upload secret file is not configured');
|
||||||
}
|
}
|
||||||
if ($secretPath[0] !== '/') {
|
if ($secretPath[0] !== '/') {
|
||||||
$secretPath = ROOT . '/' . $secretPath;
|
$secretPath = ROOT . '/' . $secretPath;
|
||||||
}
|
}
|
||||||
$secret = trim((string)file_get_contents($secretPath));
|
$secretContents = @file_get_contents($secretPath);
|
||||||
|
if ($secretContents === false) {
|
||||||
|
throw new RemoteImageUploadException('Remote user icon upload secret file cannot be read');
|
||||||
|
}
|
||||||
|
$secret = trim($secretContents);
|
||||||
|
if (strlen($secret) < 32) {
|
||||||
|
throw new RemoteImageUploadException('Remote user icon upload secret is too short');
|
||||||
|
}
|
||||||
return [rtrim((string)ServConfig::$remoteUserIconUploadPath, '/'), $secret];
|
return [rtrim((string)ServConfig::$remoteUserIconUploadPath, '/'), $secret];
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -113,7 +194,7 @@ final class RemoteUserIconUploadClient
|
|||||||
$request = self::buildRequest($url, $client, $secret, $contentType, $body);
|
$request = self::buildRequest($url, $client, $secret, $contentType, $body);
|
||||||
$curl = curl_init($url);
|
$curl = curl_init($url);
|
||||||
if ($curl === false) {
|
if ($curl === false) {
|
||||||
throw new \RuntimeException('Unable to initialize image upload request');
|
throw new RemoteImageUploadException('Unable to initialize image upload request');
|
||||||
}
|
}
|
||||||
curl_setopt_array($curl, [
|
curl_setopt_array($curl, [
|
||||||
CURLOPT_CUSTOMREQUEST => 'PUT',
|
CURLOPT_CUSTOMREQUEST => 'PUT',
|
||||||
@@ -125,14 +206,21 @@ final class RemoteUserIconUploadClient
|
|||||||
]);
|
]);
|
||||||
$response = curl_exec($curl);
|
$response = curl_exec($curl);
|
||||||
$status = curl_getinfo($curl, CURLINFO_RESPONSE_CODE);
|
$status = curl_getinfo($curl, CURLINFO_RESPONSE_CODE);
|
||||||
$error = curl_error($curl);
|
$curlErrorNumber = curl_errno($curl);
|
||||||
curl_close($curl);
|
curl_close($curl);
|
||||||
if ($response === false) {
|
if ($response === false) {
|
||||||
throw new \RuntimeException("Image upload request failed: {$error}");
|
throw new RemoteImageUploadException("Image upload request failed (cURL {$curlErrorNumber})");
|
||||||
}
|
}
|
||||||
$decoded = Json::decode($response);
|
if ($status < 200 || $status >= 300) {
|
||||||
if ($status < 200 || $status >= 300 || !($decoded['ok'] ?? false)) {
|
throw new RemoteImageUploadException("Image upload rejected ({$status})");
|
||||||
throw new \RuntimeException("Image upload rejected ({$status}): " . ($decoded['reason'] ?? 'unknown error'));
|
}
|
||||||
|
try {
|
||||||
|
$decoded = Json::decode($response);
|
||||||
|
} catch (\Throwable $error) {
|
||||||
|
throw new RemoteImageUploadException('Image upload returned invalid JSON', previous: $error);
|
||||||
|
}
|
||||||
|
if (!($decoded['ok'] ?? false)) {
|
||||||
|
throw new RemoteImageUploadException('Image upload returned an unsuccessful response');
|
||||||
}
|
}
|
||||||
$filename = basename((string)parse_url($url, PHP_URL_PATH));
|
$filename = basename((string)parse_url($url, PHP_URL_PATH));
|
||||||
$category = str_contains((string)parse_url($url, PHP_URL_PATH), '/content/') ? 'content' : 'user-icons';
|
$category = str_contains((string)parse_url($url, PHP_URL_PATH), '/content/') ? 'content' : 'user-icons';
|
||||||
@@ -140,7 +228,7 @@ final class RemoteUserIconUploadClient
|
|||||||
? "uploads/{$client}/{$filename}"
|
? "uploads/{$client}/{$filename}"
|
||||||
: "icons/users/{$client}/{$filename}";
|
: "icons/users/{$client}/{$filename}";
|
||||||
if (($decoded['path'] ?? null) !== $expectedPath) {
|
if (($decoded['path'] ?? null) !== $expectedPath) {
|
||||||
throw new \RuntimeException('Image upload returned an unexpected path');
|
throw new RemoteImageUploadException('Image upload returned an unexpected path');
|
||||||
}
|
}
|
||||||
return $decoded;
|
return $decoded;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
<?php
|
<?php
|
||||||
|
|
||||||
use PHPUnit\Framework\TestCase;
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use sammo\RemoteImageUploadException;
|
||||||
use sammo\RemoteUserIconUploadClient;
|
use sammo\RemoteUserIconUploadClient;
|
||||||
|
|
||||||
require_once dirname(__DIR__) . '/src/sammo/RemoteUserIconUploadClient.php';
|
require_once dirname(__DIR__) . '/src/sammo/RemoteUserIconUploadClient.php';
|
||||||
@@ -56,4 +57,68 @@ final class RemoteUserIconUploadClientTest extends TestCase
|
|||||||
);
|
);
|
||||||
self::assertStringContainsString('X-Image-Client: core', implode("\n", $request['headers']));
|
self::assertStringContainsString('X-Image-Client: core', implode("\n", $request['headers']));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testLogFailureWritesSafeOperatorEntryWithoutTraceOrSecret(): void
|
||||||
|
{
|
||||||
|
$secret = 'secret-' . str_repeat('z', 64);
|
||||||
|
$systemMessages = [];
|
||||||
|
$structuredEntries = [];
|
||||||
|
|
||||||
|
RemoteUserIconUploadClient::logFailure(
|
||||||
|
'user-icon',
|
||||||
|
new RuntimeException("request failed with {$secret}"),
|
||||||
|
static function (string $message) use (&$systemMessages): void {
|
||||||
|
$systemMessages[] = $message;
|
||||||
|
},
|
||||||
|
static function (string $type, string $message, string $path, array $trace) use (&$structuredEntries): void {
|
||||||
|
$structuredEntries[] = compact('type', 'message', 'path', 'trace');
|
||||||
|
}
|
||||||
|
);
|
||||||
|
|
||||||
|
self::assertSame(['Remote user icon upload failed: Unexpected RuntimeException'], $systemMessages);
|
||||||
|
self::assertCount(1, $structuredEntries);
|
||||||
|
self::assertSame('RemoteImageUploadFailure', $structuredEntries[0]['type']);
|
||||||
|
self::assertSame($systemMessages[0], $structuredEntries[0]['message']);
|
||||||
|
self::assertSame([], $structuredEntries[0]['trace']);
|
||||||
|
self::assertStringNotContainsString($secret, json_encode($structuredEntries, JSON_THROW_ON_ERROR));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testLogFailureKeepsOnlyClientGeneratedSafeReason(): void
|
||||||
|
{
|
||||||
|
$structuredEntries = [];
|
||||||
|
RemoteUserIconUploadClient::logFailure(
|
||||||
|
'content-image',
|
||||||
|
new RemoteImageUploadException('Image upload rejected (401)'),
|
||||||
|
static function (): void {},
|
||||||
|
static function (string $type, string $message, string $path, array $trace) use (&$structuredEntries): void {
|
||||||
|
$structuredEntries[] = compact('type', 'message', 'path', 'trace');
|
||||||
|
}
|
||||||
|
);
|
||||||
|
|
||||||
|
self::assertSame(
|
||||||
|
'Remote content image upload failed: Image upload rejected (401)',
|
||||||
|
$structuredEntries[0]['message']
|
||||||
|
);
|
||||||
|
self::assertSame([], $structuredEntries[0]['trace']);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testLogFailureRejectsAnUnapprovedClientExceptionMessage(): void
|
||||||
|
{
|
||||||
|
$secret = 'secret-' . str_repeat('q', 64);
|
||||||
|
$structuredEntries = [];
|
||||||
|
RemoteUserIconUploadClient::logFailure(
|
||||||
|
'user-icon',
|
||||||
|
new RemoteImageUploadException("unexpected response {$secret}"),
|
||||||
|
static function (): void {},
|
||||||
|
static function (string $type, string $message, string $path, array $trace) use (&$structuredEntries): void {
|
||||||
|
$structuredEntries[] = compact('type', 'message', 'path', 'trace');
|
||||||
|
}
|
||||||
|
);
|
||||||
|
|
||||||
|
self::assertSame(
|
||||||
|
'Remote user icon upload failed: Unexpected sammo\\RemoteImageUploadException',
|
||||||
|
$structuredEntries[0]['message']
|
||||||
|
);
|
||||||
|
self::assertStringNotContainsString($secret, json_encode($structuredEntries, JSON_THROW_ON_ERROR));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user