Files
projectsend/tests/Feature/Files/DownloadDispositionTest.php
denkfabrik-li 5a9133bb07 Give a download's presigned URL a minute rather than an hour
StoredFileResponse hands external storage a presigned URL for an hour,
whatever the delivery is for. That URL is a bearer credential: whoever
holds it fetches the file without passing any of the caller's checks
again, and it outlives them. A download cap spent in the meantime, an
expires_at that falls inside the hour, an assignment withdrawn -- none of
them reach it, and nothing here can revoke one. It is also forwardable,
which the local path is not: X-Accel-Redirect authorises one response to
one request.

The two deliveries do not need the same window, so they no longer share
one.

A download has to survive being followed -- a redirect and a request --
which a minute covers with room to spare. An object store checks the
signature when the request arrives rather than while it runs, so a
transfer that starts inside the window finishes however long it takes.

A preview keeps the hour, because it is watched rather than fetched: the
player holds the URL and issues a Range request every time somebody seeks
past the buffer, so a minute would break playback of anything longer than
a minute. The class docblock now says that this is the trade being made,
instead of leaving it in a single number.

Two tests, one per window. Without the fix the download link is an hour
long.
2026-08-28 06:40:53 +02:00

133 lines
5.0 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Files\Models\File;
use App\Support\ContentDisposition;
use Illuminate\Http\UploadedFile;
use Illuminate\Support\Facades\Storage;
beforeEach(function () {
Storage::fake('files');
$this->admin = User::factory()->create();
});
test('ascii filenames keep the exact quoted header format the app has always sent', function () {
expect(ContentDisposition::attachment('contract.pdf'))
->toBe('attachment; filename="contract.pdf"')
->and(ContentDisposition::inline('photo.jpg'))
->toBe('inline; filename="photo.jpg"');
});
test('non-ascii filenames add an RFC 6266 filename* ext-value with an ascii fallback', function () {
$header = ContentDisposition::attachment('informe año 2026.pdf');
expect($header)
->toStartWith('attachment; filename="')
->toContain("; filename*=utf-8''")
->toContain(rawurlencode('informe año 2026.pdf'));
// The legacy filename= parameter must stay pure ASCII — that is the
// whole point of the split.
preg_match('/filename="((?:[^"\\\\]|\\\\.)*)"/', $header, $matches);
expect(preg_match('/^[\x20-\x7E]*$/', $matches[1]))->toBe(1);
});
test('quotes are escaped and path separators neutralised', function () {
expect(ContentDisposition::attachment('a"b.txt'))
->toBe('attachment; filename="a\"b.txt"')
->and(ContentDisposition::attachment('../../etc/passwd'))
->toBe('attachment; filename=".._.._etc_passwd"');
});
test('a filename that transliterates to nothing still offers a usable fallback', function () {
$header = ContentDisposition::attachment('中文.txt');
// Whatever the transliterator makes of the CJK part, the fallback is
// non-empty ASCII and the true name always travels in filename*.
preg_match('/filename="((?:[^"\\\\]|\\\\.)*)"/', $header, $matches);
expect(trim($matches[1]))->not->toBe('')
->and($header)->toContain("; filename*=utf-8''".rawurlencode('中文.txt'));
});
test('downloads of files with non-ascii names send both header forms on the wire', function () {
$file = uploadDocumentFile($this->admin, 'año contable — resumen.pdf');
$header = $this->actingAs($this->admin)
->get("/files/{$file->id}/download")
->assertOk()
->headers->get('Content-Disposition');
expect($header)
->toContain('attachment; filename="')
->toContain("filename*=utf-8''".rawurlencode('año contable — resumen.pdf'));
});
/*
|--------------------------------------------------------------------------
| How long a presigned URL stays usable
|--------------------------------------------------------------------------
|
| On the local disk a delivery is an X-Accel-Redirect: it authorises one
| response, to one request. On external storage it is a presigned URL,
| which is a bearer credential -- forwardable, and valid whatever happens
| to the file or the caller's checks in the meantime. Nothing can revoke
| one, so its lifetime is the only dial there is.
|
*/
test('a download link outlives the redirect and not much else', function () {
$seen = [];
Storage::fake('files_external');
Storage::disk('files_external')->buildTemporaryUrlsUsing(
function (string $path, $expiration, array $options) use (&$seen): string {
$seen[] = $expiration;
return 'https://storage.example.test/'.$path;
}
);
$file = uploadDocumentFile($this->admin, 'report.pdf');
$file->update(['disk' => 'files_external']);
$this->actingAs($this->admin)->get("/files/{$file->id}/download")->assertRedirect();
expect($seen)->toHaveCount(1)
->and($seen[0]->getTimestamp())->toBeLessThanOrEqual(now()->addSeconds(60)->getTimestamp())
->and($seen[0]->getTimestamp())->toBeGreaterThan(now()->addSeconds(30)->getTimestamp());
});
test('a preview link lasts as long as somebody might watch', function () {
// The other half of the trade: a player holds this URL and asks it for
// ranges every time the viewer seeks past the buffer, so a minute would
// break playback of anything longer than a minute.
$seen = [];
Storage::fake('files_external');
Storage::disk('files_external')->buildTemporaryUrlsUsing(
function (string $path, $expiration, array $options) use (&$seen): string {
$seen[] = $expiration;
return 'https://storage.example.test/'.$path;
}
);
// Uploaded rather than factory-made, so it is a real previewable file;
// FilePreviewTest's helper is private to that file (Pest globals).
$this->actingAs($this->admin)->post('/files', [
'file' => UploadedFile::fake()->create('clip.mp4', 16, 'video/mp4'),
'name' => '',
'description' => '',
]);
$file = File::query()->latest('id')->firstOrFail();
$file->update(['disk' => 'files_external']);
$this->actingAs($this->admin)->get("/files/{$file->id}/preview")->assertRedirect();
expect($seen)->toHaveCount(1)
->and($seen[0]->getTimestamp())->toBeGreaterThan(now()->addMinutes(50)->getTimestamp());
});