fix: align upload-asset URL contract end-to-end
- ImageUploadService::store() now returns /assets/uploads/<token>/<file>
so the URL the upload handler emits matches the new GET route.
- public/index.php registers a dedicated GET /assets/uploads/{userToken}/{filename}
route; the legacy /assets/{kind}/{filename} is kept as a fallback.
- GetAssets reads the upload token from $request->server['__uploads_token']
(the front controller threads it that way) instead of the cookies bag.
- Adds GetAssets unit tests for the upload-token happy path and a
token-mismatch 404, plus a FullFlowTest step that uploads and re-fetches
the asset through the front controller.
This commit is contained in:
@@ -106,6 +106,13 @@ $postUpload = static fn (Request $r, array $p): Response => (new PostImageUpload
|
|||||||
$getAssets = static function (Request $r, array $p) use ($placeholderDir, $uploadsRoot): Response {
|
$getAssets = static function (Request $r, array $p) use ($placeholderDir, $uploadsRoot): Response {
|
||||||
return (new GetAssets($placeholderDir, $uploadsRoot))->handle($r, $p);
|
return (new GetAssets($placeholderDir, $uploadsRoot))->handle($r, $p);
|
||||||
};
|
};
|
||||||
|
$getUploadedAsset = static function (Request $r, array $p) use ($placeholderDir, $uploadsRoot): Response {
|
||||||
|
$handler = new GetAssets($placeholderDir, $uploadsRoot);
|
||||||
|
return $handler->handle(
|
||||||
|
$r,
|
||||||
|
['kind' => 'uploads', 'userToken' => $p['userToken'] ?? '', 'filename' => $p['filename'] ?? ''],
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
$router = new Router();
|
$router = new Router();
|
||||||
$router->add('GET', '/', $homePage);
|
$router->add('GET', '/', $homePage);
|
||||||
@@ -115,6 +122,7 @@ $router->add('GET', '/scenarios/{id}/edit/battlefield', $getBattlefield);
|
|||||||
$router->add('POST', '/scenarios/{id}/edit/battlefield', $postBattlefield);
|
$router->add('POST', '/scenarios/{id}/edit/battlefield', $postBattlefield);
|
||||||
$router->add('POST', '/scenarios/{id}/start', $postStart);
|
$router->add('POST', '/scenarios/{id}/start', $postStart);
|
||||||
$router->add('POST', '/assets/upload', $postUpload);
|
$router->add('POST', '/assets/upload', $postUpload);
|
||||||
|
$router->add('GET', '/assets/uploads/{userToken}/{filename}', $getUploadedAsset);
|
||||||
$router->add('GET', '/assets/{kind}/{filename}', $getAssets);
|
$router->add('GET', '/assets/{kind}/{filename}', $getAssets);
|
||||||
|
|
||||||
// 8. Dispatch and emit.
|
// 8. Dispatch and emit.
|
||||||
|
|||||||
@@ -33,6 +33,6 @@ final class ImageUploadService
|
|||||||
// file itself to 0600 in case the server's umask is permissive.
|
// file itself to 0600 in case the server's umask is permissive.
|
||||||
chmod($destination, 0600);
|
chmod($destination, 0600);
|
||||||
|
|
||||||
return '/assets/' . $this->userToken . '/' . $filename;
|
return '/assets/uploads/' . $this->userToken . '/' . $filename;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ final class GetAssets
|
|||||||
return Response::html(404, '<h1>Not found</h1>');
|
return Response::html(404, '<h1>Not found</h1>');
|
||||||
}
|
}
|
||||||
|
|
||||||
$requestToken = $request->cookies['__uploads_token'] ?? '';
|
$requestToken = (string) ($request->server['__uploads_token'] ?? '');
|
||||||
|
|
||||||
if ($userToken !== $requestToken) {
|
if ($userToken !== $requestToken) {
|
||||||
return Response::html(404, '<h1>Not found</h1>');
|
return Response::html(404, '<h1>Not found</h1>');
|
||||||
|
|||||||
@@ -24,8 +24,8 @@ final class FullFlowTest extends TestCase
|
|||||||
public function testTheFullCreateAndSaveFlow(): void
|
public function testTheFullCreateAndSaveFlow(): void
|
||||||
{
|
{
|
||||||
[$token, $cookie] = CsrfToken::issue(self::SECRET);
|
[$token, $cookie] = CsrfToken::issue(self::SECRET);
|
||||||
$uploadsRoot = sys_get_temp_dir() . '/bf-fullflow-' . bin2hex(random_bytes(4));
|
// Use a deterministic secret so the upload token HMAC matches between upload and fetch.
|
||||||
mkdir($uploadsRoot, 0700, true);
|
putenv('BATTLEFORGE_SECRET=' . self::SECRET);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Step 1: GET home page.
|
// Step 1: GET home page.
|
||||||
@@ -71,19 +71,82 @@ final class FullFlowTest extends TestCase
|
|||||||
self::assertSame('alpha', $startJson['match']['activeTeamId'] ?? null);
|
self::assertSame('alpha', $startJson['match']['activeTeamId'] ?? null);
|
||||||
self::assertSame(1, $startJson['match']['round'] ?? null);
|
self::assertSame(1, $startJson['match']['round'] ?? null);
|
||||||
self::assertCount(6, $startJson['match']['units'] ?? []);
|
self::assertCount(6, $startJson['match']['units'] ?? []);
|
||||||
|
|
||||||
|
// Step 5: POST image upload, then GET the returned URL.
|
||||||
|
$this->uploadAndFetchAsset();
|
||||||
} finally {
|
} finally {
|
||||||
|
// Always purge the per-install uploads dir after the test.
|
||||||
|
$uploadsRoot = __DIR__ . '/../../var/uploads';
|
||||||
if (is_dir($uploadsRoot)) {
|
if (is_dir($uploadsRoot)) {
|
||||||
foreach (glob($uploadsRoot . '/*/*') as $file) {
|
foreach (glob($uploadsRoot . '/*/*') as $file) {
|
||||||
unlink($file);
|
@unlink($file);
|
||||||
}
|
}
|
||||||
foreach (glob($uploadsRoot . '/*') as $dir) {
|
foreach (glob($uploadsRoot . '/*') as $dir) {
|
||||||
rmdir($dir);
|
@rmdir($dir);
|
||||||
}
|
}
|
||||||
rmdir($uploadsRoot);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function uploadAndFetchAsset(): void
|
||||||
|
{
|
||||||
|
$expectedToken = hash_hmac('sha256', self::SECRET, 'bf-uploads');
|
||||||
|
$expectedDir = realpath(__DIR__ . '/../../var/uploads') ?: (__DIR__ . '/../../var/uploads');
|
||||||
|
|
||||||
|
// Create the multipart body manually so we can pre-compute the file we expect to find.
|
||||||
|
$png = base64_decode(
|
||||||
|
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGNgYGBgAAAABQABh6FO1AAAAABJRU5ErkJggg==',
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
$tmp = tempnam(sys_get_temp_dir(), 'bf-upload-');
|
||||||
|
file_put_contents($tmp, $png);
|
||||||
|
|
||||||
|
try {
|
||||||
|
[$token, $cookie] = CsrfToken::issue(self::SECRET);
|
||||||
|
$_COOKIE['__csrf'] = $cookie;
|
||||||
|
$_SERVER['REQUEST_METHOD'] = 'POST';
|
||||||
|
$_SERVER['REQUEST_URI'] = '/assets/upload';
|
||||||
|
$_SERVER['CONTENT_TYPE'] = 'multipart/form-data; boundary=----test';
|
||||||
|
$_SERVER['__csrf_secret'] = self::SECRET;
|
||||||
|
$_POST = ['_csrf' => $token];
|
||||||
|
$_FILES = [
|
||||||
|
'image' => [
|
||||||
|
'name' => 'test.png',
|
||||||
|
'type' => 'image/png',
|
||||||
|
'tmp_name' => $tmp,
|
||||||
|
'error' => 0,
|
||||||
|
'size' => strlen($png),
|
||||||
|
],
|
||||||
|
];
|
||||||
|
$this->rawBody = '';
|
||||||
|
$_SERVER['__raw_body'] = '';
|
||||||
|
$body = $this->runFrontController();
|
||||||
|
self::assertSame(200, $this->lastStatus, 'upload response: ' . $body);
|
||||||
|
$payload = json_decode($body, true);
|
||||||
|
$url = is_array($payload) ? (string) ($payload['url'] ?? '') : '';
|
||||||
|
self::assertNotSame('', $url);
|
||||||
|
self::assertStringStartsWith('/assets/uploads/' . $expectedToken . '/', $url);
|
||||||
|
|
||||||
|
// Now fetch the asset back through the front controller.
|
||||||
|
$_COOKIE = [];
|
||||||
|
$_POST = [];
|
||||||
|
$_FILES = [];
|
||||||
|
$_SERVER['REQUEST_METHOD'] = 'GET';
|
||||||
|
$_SERVER['REQUEST_URI'] = $url;
|
||||||
|
unset($_SERVER['CONTENT_TYPE'], $_SERVER['HTTP_X_CSRF_TOKEN'], $_SERVER['__raw_body']);
|
||||||
|
$this->rawBody = '';
|
||||||
|
$fetchBody = $this->runFrontController();
|
||||||
|
self::assertSame(200, $this->lastStatus, 'fetch response: ' . $fetchBody);
|
||||||
|
self::assertSame($png, $fetchBody);
|
||||||
|
|
||||||
|
// Clean up the uploaded file so the outer finally does not loop on it.
|
||||||
|
$filename = substr($url, strlen('/assets/uploads/' . $expectedToken . '/'));
|
||||||
|
@unlink($expectedDir . '/' . $expectedToken . '/' . $filename);
|
||||||
|
} finally {
|
||||||
|
@unlink($tmp);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private string $rawBody = '';
|
private string $rawBody = '';
|
||||||
private int $lastStatus = 0;
|
private int $lastStatus = 0;
|
||||||
|
|
||||||
|
|||||||
@@ -67,4 +67,61 @@ final class GetAssetsTest extends TestCase
|
|||||||
|
|
||||||
self::assertSame(404, $response->status);
|
self::assertSame(404, $response->status);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testItServesAnUploadedAssetWhenTheServerTokenMatches(): void
|
||||||
|
{
|
||||||
|
$userToken = str_repeat('a', 64);
|
||||||
|
$namespace = $this->uploadsDir . '/' . $userToken;
|
||||||
|
mkdir($namespace, 0700, true);
|
||||||
|
$payload = base64_decode(
|
||||||
|
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGNgYGBgAAAABQABh6FO1AAAAABJRU5ErkJggg==',
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
file_put_contents($namespace . '/demo.png', $payload);
|
||||||
|
|
||||||
|
$handler = new GetAssets($this->placeholderDir, $this->uploadsDir);
|
||||||
|
$request = new Request(
|
||||||
|
[],
|
||||||
|
[],
|
||||||
|
[],
|
||||||
|
[],
|
||||||
|
['__uploads_token' => $userToken],
|
||||||
|
'',
|
||||||
|
'GET',
|
||||||
|
'/assets/uploads/' . $userToken . '/demo.png',
|
||||||
|
null,
|
||||||
|
'',
|
||||||
|
);
|
||||||
|
$response = $handler->handle(
|
||||||
|
$request,
|
||||||
|
['kind' => 'uploads', 'userToken' => $userToken, 'filename' => 'demo.png'],
|
||||||
|
);
|
||||||
|
|
||||||
|
self::assertSame(200, $response->status);
|
||||||
|
self::assertSame('image/png', $response->headers['Content-Type'] ?? '');
|
||||||
|
self::assertSame($payload, $response->body);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testItRefusesAnUploadedAssetWhenTheServerTokenDiffers(): void
|
||||||
|
{
|
||||||
|
$handler = new GetAssets($this->placeholderDir, $this->uploadsDir);
|
||||||
|
$request = new Request(
|
||||||
|
[],
|
||||||
|
[],
|
||||||
|
[],
|
||||||
|
[],
|
||||||
|
['__uploads_token' => str_repeat('a', 64)],
|
||||||
|
'',
|
||||||
|
'GET',
|
||||||
|
'/assets/uploads/abc/def.png',
|
||||||
|
null,
|
||||||
|
'',
|
||||||
|
);
|
||||||
|
$response = $handler->handle(
|
||||||
|
$request,
|
||||||
|
['kind' => 'uploads', 'userToken' => str_repeat('b', 64), 'filename' => 'def.png'],
|
||||||
|
);
|
||||||
|
|
||||||
|
self::assertSame(404, $response->status);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ final class ImageUploadServiceTest extends TestCase
|
|||||||
|
|
||||||
$url = $service->store($tmp, 'image/png');
|
$url = $service->store($tmp, 'image/png');
|
||||||
|
|
||||||
self::assertStringStartsWith('/assets/user-token-1/', $url);
|
self::assertStringStartsWith('/assets/uploads/user-token-1/', $url);
|
||||||
self::assertStringEndsWith('.png', $url);
|
self::assertStringEndsWith('.png', $url);
|
||||||
|
|
||||||
$stored = $this->tmpDir . '/user-token-1/' . basename($url);
|
$stored = $this->tmpDir . '/user-token-1/' . basename($url);
|
||||||
|
|||||||
Reference in New Issue
Block a user