Enable nginx X-Accel for original artwork downloads.

Hand originals to nginx after auth so PHP is not in the byte path. Keep DOWNLOAD_ACCEL_ENABLED off until the internal location is verified.
This commit is contained in:
2026-08-29 12:25:37 +02:00
parent fb560fb7dd
commit e9cf754b37
7 changed files with 411 additions and 35 deletions
+5
View File
@@ -4,6 +4,11 @@ APP_KEY=
APP_DEBUG=true
APP_URL=http://localhost
# Artwork original downloads: PHP fallback unless nginx X-Accel is configured.
# Leave false until the internal location exists and has been verified.
DOWNLOAD_ACCEL_ENABLED=false
DOWNLOAD_ACCEL_PATH=/internal/originals
SECURITY_REPORT_ENABLED=true
SECURITY_REPORT_NOTIFY_EMAIL=
SECURITY_REPORT_SCAN_NPM=true
@@ -48,14 +48,14 @@ final class ArtworkDownloadController extends Controller
$artwork = Artwork::query()->find($id);
if (! $artwork) {
abort(404);
return $this->notFound();
}
$filePath = $this->originalFiles->resolveLocalPath($artwork);
$ext = strtolower(ltrim((string) pathinfo($filePath, PATHINFO_EXTENSION), '.'));
if ($filePath === '' || ! in_array($ext, self::ALLOWED_EXTENSIONS, true)) {
abort(404);
return $this->notFound();
}
if (! File::isFile($filePath)) {
@@ -65,7 +65,7 @@ final class ArtworkDownloadController extends Controller
'resolved_path' => $filePath,
]);
abort(404);
return $this->notFound();
}
$this->recordDownload($request, $artwork->id);
@@ -98,6 +98,14 @@ final class ArtworkDownloadController extends Controller
return response()->download($filePath, $downloadName);
}
private function notFound(): Response
{
return response('', 404, [
'Content-Type' => 'text/plain; charset=UTF-8',
'Cache-Control' => 'no-store',
]);
}
private function resolveAccelUri(string $filePath): ?string
{
if (! config('app.download_accel_enabled')) {
+12 -30
View File
@@ -1,36 +1,18 @@
# -----------------------------------------------------------------------
# nginx X-Accel-Redirect for artwork original file downloads
# M13 — nginx X-Accel-Redirect for GET /download/artwork/{id}
#
# Problem: PHP streams the file body through FPM → nginx, causing
# "[warn] upstream response is buffered to a temporary file"
# for large downloads because FastCGI buffers are too small.
# Production vhost is NOT in this repository (live file:
# /etc/nginx/sites-enabled/skinbase.org.conf).
# Insert this location inside the HTTPS server { } block, before the
# catch-all `location /` and any regex locations that could steal the URI.
#
# Solution: PHP sets X-Accel-Redirect, nginx serves the file body
# directly from disk — FPM is only used for the header response.
# Trailing slashes are required: location prefix and alias both end with /.
# Use the canonical shared storage tree so release switches do not break nginx.
# Do not use `root` here.
#
# Setup:
# 1. Set DOWNLOAD_ACCEL_PATH=/internal/originals in .env (production).
# 2. Include this file inside your server {} block:
# include /etc/nginx/conf.d/download-accel.conf;
# 3. Make sure the nginx worker has read access to the originals path.
#
# The /internal/originals prefix MUST match DOWNLOAD_ACCEL_PATH in .env.
# The alias path MUST match ARTWORKS_LOCAL_ORIGINALS_ROOT on the server.
# -----------------------------------------------------------------------
# Direct GET /internal/originals/... must 404 (internal;).
# Laravel still authorizes/counts; only the file body is offloaded.
location /internal/originals/ {
# Block direct client access — only X-Accel-Redirect headers trigger this.
location ^~ /internal/originals/ {
internal;
# Replace this with the actual absolute path to artwork originals on disk.
# Typically: /var/www/skinbase/storage/originals or /var/www/skinbase/public/files/originals
alias /var/www/skinbase/public/files/originals/;
# Let nginx send the file efficiently with sendfile + tcp_nopush.
sendfile on;
tcp_nopush on;
tcp_nodelay on;
# No nginx-level caching for download responses (private files).
add_header Cache-Control "private, no-store";
alias /opt/www/virtual/SkinbaseNova.releases/shared/storage/app/originals/artworks/;
}
@@ -0,0 +1,80 @@
# M13 — Artwork Download X-Accel-Redirect Offload
Application tests and nginx snippet only. **Do not enable in production until Stage A nginx is verified.**
## Pre-M13 evidence
`/download/artwork/{id}` (M12 analyzer, skinbase.org):
| | |
| --- | --- |
| count | 2363 |
| p50 / p95 / p99 / max | 0.535 / 1.526 / 3.335 / **16.037** s |
| avg request_time | 0.7175 s |
| avg upstream_response_time | 0.3762 s |
| avg bytes | ~528 KB |
Slow requests show PHP finishing in ~0.3–0.5 s while `request_time` stays large: FPM is stuck sending the body.
After X-Accel: `upstream_response_time` is still Laravel work; `request_time` can still include slow clients. The win is **freeing PHP-FPM**, not always a tiny nginx `request_time`.
## Existing application
- Route: `GET /download/artwork/{id}` → `ArtworkDownloadController`
- Flag: `DOWNLOAD_ACCEL_ENABLED` default **false**
- Path: `DOWNLOAD_ACCEL_PATH=/internal/originals`
- Laravel originals root: `uploads.local_originals_root` (symlink `SkinbaseNova/storage` → shared)
- Fallback: `response()->download()` — **must stay**
Controller already authorizes, checks file, records analytics, then optionally sets `X-Accel-Redirect`.
## nginx mapping
Production vhost is **not** in this repo: `/etc/nginx/sites-enabled/skinbase.org.conf`.
Snippet: `deploy/nginx/download-accel.conf`
```nginx
location ^~ /internal/originals/ {
internal;
alias /opt/www/virtual/SkinbaseNova.releases/shared/storage/app/originals/artworks/;
}
```
Alias uses the **canonical shared tree** so a release switch cannot point nginx at a missing release directory. Laravel still uses `local_originals_root`; the X-Accel URI is only the relative hash path (`/27/21/….zip`).
`internal;` is mandatory: a browser GET to `/internal/originals/...` must 404.
No sendfile/tcp/buffering changes in M13.
## Rollout (operator — not executed here)
### Stage A — nginx only (`DOWNLOAD_ACCEL_ENABLED=false`)
1. Backup: `sudo cp /etc/nginx/sites-enabled/skinbase.org.conf /etc/nginx/sites-enabled/skinbase.org.conf.bak-$(date +%Y%m%d-%H%M%S)`
2. Insert the location inside the HTTPS `server { }` (before `location /`).
3. `sudo nginx -t` — stop if it fails.
4. `sudo systemctl reload nginx` — **not** restart.
5. Direct access: `curl -I https://skinbase.org/internal/originals/<relative-path>` → **404**.
6. Optional: `sudo -u www-data test -r <absolute-original> && echo READABLE`
### Stage B — Laravel flag
7. Set `DOWNLOAD_ACCEL_ENABLED=true` (path already `/internal/originals`).
8. `sudo -u skinbase php artisan config:cache` if config is cached.
9. Verify with tinker as `skinbase` (flag true, path `/internal/originals`).
10. Public: `curl -fL https://skinbase.org/download/artwork/<ID>` → 200, attachment, size matches original.
11. Range: `curl -H 'Range: bytes=0-99'` → **206**, `Content-Range: bytes 0-99/<size>`, 100-byte body.
12. Repeat direct internal URL → still 404.
Application deploy: `bash ./sync.sh`. Never `git pull` on production.
## Rollback
First: `DOWNLOAD_ACCEL_ENABLED=false` then `sudo -u skinbase php artisan config:cache` if needed. PHP fallback returns immediately. Nginx location can stay.
If nginx must be reverted: restore the `.bak-*` vhost, `nginx -t`, reload.
## Analyzer
Keep M12 `skinbase-performance.log`. After activation, `upstream_response_time` should stay near Laravel work; do not expect `request_time` to collapse for slow clients.
+170
View File
@@ -0,0 +1,170 @@
<?php
declare(strict_types=1);
use App\Http\Controllers\ArtworkDownloadController;
use App\Models\Artwork;
use App\Services\ArtworkStatsService;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\File;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
beforeEach(function () {
$root = storage_path('framework/testing/artwork-downloads-accel');
config([
'uploads.storage_root' => $root,
'uploads.local_originals_root' => $root,
'app.download_accel_enabled' => false,
'app.download_accel_path' => '/internal/originals',
'app.url' => 'https://skinbase.test',
]);
if (File::exists($root)) {
File::deleteDirectory($root);
}
File::makeDirectory($root, 0755, true);
});
afterEach(function () {
$root = storage_path('framework/testing/artwork-downloads-accel');
if (File::exists($root)) {
File::deleteDirectory($root);
}
});
function accelOriginal(string $hash, string $ext, string $content = 'original-bytes'): string
{
$root = rtrim((string) config('uploads.local_originals_root'), DIRECTORY_SEPARATOR);
$dir = $root.DIRECTORY_SEPARATOR.substr($hash, 0, 2).DIRECTORY_SEPARATOR.substr($hash, 2, 2);
File::makeDirectory($dir, 0755, true, true);
$path = $dir.DIRECTORY_SEPARATOR.$hash.'.'.$ext;
File::put($path, $content);
return $path;
}
function resolveAccelUri(string $filePath): ?string
{
$controller = app(ArtworkDownloadController::class);
$method = new ReflectionMethod(ArtworkDownloadController::class, 'resolveAccelUri');
return $method->invoke($controller, $filePath);
}
it('uses the PHP download fallback when acceleration is disabled', function () {
$hash = '2721722cb2f407cc53539ee8718cc89c4cdfc579';
accelOriginal($hash, 'zip', 'zip-body');
$artwork = Artwork::factory()->create([
'file_name' => 'Nested Original',
'hash' => $hash,
'file_ext' => 'zip',
]);
$response = $this->get('/download/artwork/'.$artwork->id);
$response->assertOk();
expect($response->headers->get('X-Accel-Redirect'))->toBeNull()
->and($response->baseResponse)->toBeInstanceOf(BinaryFileResponse::class);
});
it('returns X-Accel-Redirect for a nested original when acceleration is enabled', function () {
config(['app.download_accel_enabled' => true]);
$hash = '2721722cb2f407cc53539ee8718cc89c4cdfc579';
accelOriginal($hash, 'zip', 'zip-body');
$artwork = Artwork::factory()->create([
'file_name' => 'Nested Original',
'hash' => $hash,
'file_ext' => 'zip',
]);
$this->mock(ArtworkStatsService::class, function ($mock) use ($artwork): void {
$mock->shouldReceive('incrementDownloads')->once()->with($artwork->id, 1, false);
});
$response = $this->get('/download/artwork/'.$artwork->id);
$response->assertOk()
->assertHeader('X-Accel-Redirect', '/internal/originals/27/21/2721722cb2f407cc53539ee8718cc89c4cdfc579.zip')
->assertHeader('Content-Type', 'application/octet-stream')
->assertHeader('X-Content-Type-Options', 'nosniff');
$disposition = (string) $response->headers->get('Content-Disposition');
expect($disposition)->toContain('attachment')
->and($disposition)->toContain('Nested Original')
->and($disposition)->toContain('.zip')
->and($response->baseResponse)->not->toBeInstanceOf(BinaryFileResponse::class);
expect(DB::table('artwork_downloads')->where('artwork_id', $artwork->id)->count())->toBe(1);
});
it('maps nested originals under the accel prefix without duplicate slashes', function () {
config(['app.download_accel_enabled' => true]);
$root = rtrim((string) config('uploads.local_originals_root'), DIRECTORY_SEPARATOR);
$relative = '27'.DIRECTORY_SEPARATOR.'21'.DIRECTORY_SEPARATOR.'2721722cb2f407cc53539ee8718cc89c4cdfc579.zip';
$path = $root.DIRECTORY_SEPARATOR.$relative;
expect(resolveAccelUri($path))->toBe('/internal/originals/27/21/2721722cb2f407cc53539ee8718cc89c4cdfc579.zip');
});
it('does not emit an accel URI for a file outside the originals root', function () {
config(['app.download_accel_enabled' => true]);
$outside = sys_get_temp_dir().DIRECTORY_SEPARATOR.'not-original.zip';
File::put($outside, 'nope');
expect(resolveAccelUri($outside))->toBeNull();
File::delete($outside);
});
it('returns 404 for an unsupported original extension', function () {
$hash = 'aa11bb22cc33dd44ee55ff6677889900aabbccdd';
accelOriginal($hash, 'exe', 'not-allowed');
$artwork = Artwork::factory()->create([
'hash' => $hash,
'file_ext' => 'exe',
'file_name' => 'bad.exe',
]);
$this->mock(ArtworkStatsService::class, function ($mock): void {
$mock->shouldReceive('incrementDownloads')->never();
});
$this->get('/download/artwork/'.$artwork->id)->assertNotFound();
expect(DB::table('artwork_downloads')->where('artwork_id', $artwork->id)->count())->toBe(0);
});
it('still records download analytics when acceleration is enabled', function () {
config(['app.download_accel_enabled' => true]);
$hash = 'bb22cc33dd44ee55ff6677889900aabbccddeeff';
accelOriginal($hash, 'jpg', 'jpeg-bytes');
$artwork = Artwork::factory()->create([
'hash' => $hash,
'file_ext' => 'jpg',
'file_name' => 'Photo',
]);
DB::table('artwork_stats')->insertOrIgnore([
'artwork_id' => $artwork->id,
'views' => 0,
'views_24h' => 0,
'views_7d' => 0,
'downloads' => 0,
'downloads_24h' => 0,
'downloads_7d' => 0,
'favorites' => 0,
'rating_avg' => 0,
'rating_count' => 0,
]);
$this->get('/download/artwork/'.$artwork->id)
->assertOk()
->assertHeader('X-Accel-Redirect');
expect(DB::table('artwork_downloads')->where('artwork_id', $artwork->id)->count())->toBe(1)
->and((int) DB::table('artwork_stats')->where('artwork_id', $artwork->id)->value('downloads'))->toBe(1);
});
+2 -2
View File
@@ -57,7 +57,7 @@ it('downloads an existing artwork file', function () {
$response = $this->get("/download/artwork/{$artwork->id}");
$response->assertOk();
$response->assertDownload('Sky Sunset.png');
$response->assertDownload('Sky Sunset (skinbase.org).png');
});
it('forces the download filename using file_name and extension', function () {
@@ -74,7 +74,7 @@ it('forces the download filename using file_name and extension', function () {
$response = $this->get("/download/artwork/{$artwork->id}");
$response->assertOk();
$response->assertDownload('My Original Name.jpg');
$response->assertDownload('My Original Name (skinbase.org).jpg');
});
it('returns 404 for a missing artwork', function () {
@@ -0,0 +1,131 @@
<?php
declare(strict_types=1);
use App\Models\Artwork;
use App\Models\User;
use App\Services\ArtworkStatsService;
use App\Services\Traffic\OnlineVisitorRepository;
use Illuminate\Support\Facades\Config;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\File;
beforeEach(function () {
$root = storage_path('framework/testing/artwork-downloads-m15c');
config([
'uploads.storage_root' => $root,
'uploads.local_originals_root' => $root,
'app.download_accel_enabled' => false,
'app.url' => 'https://skinbase.test',
'skinbase-sessions.enabled' => true,
'skinbase-sessions.debug_header' => true,
'skinbase-sessions.skip_anonymous_public_get' => true,
]);
if (File::exists($root)) {
File::deleteDirectory($root);
}
File::makeDirectory($root, 0755, true);
});
afterEach(function () {
$root = storage_path('framework/testing/artwork-downloads-m15c');
if (File::exists($root)) {
File::deleteDirectory($root);
}
});
function m15cOriginal(string $hash, string $ext, string $content = 'bytes'): string
{
$root = rtrim((string) config('uploads.local_originals_root'), DIRECTORY_SEPARATOR);
$dir = $root.DIRECTORY_SEPARATOR.substr($hash, 0, 2).DIRECTORY_SEPARATOR.substr($hash, 2, 2);
File::makeDirectory($dir, 0755, true, true);
$path = $dir.DIRECTORY_SEPARATOR.$hash.'.'.$ext;
File::put($path, $content);
return $path;
}
it('returns a cheap 404 for a nonexistent artwork without analytics', function () {
$this->mock(ArtworkStatsService::class, function ($mock): void {
$mock->shouldReceive('incrementDownloads')->never();
});
$this->get('/download/artwork/999999')->assertNotFound();
expect(DB::table('artwork_downloads')->count())->toBe(0);
});
it('returns a cheap 404 for a missing original without analytics', function () {
$artwork = Artwork::factory()->create([
'hash' => 'c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f6a7b8c9d0',
'file_ext' => 'webp',
'file_name' => 'gone.webp',
]);
$this->mock(ArtworkStatsService::class, function ($mock): void {
$mock->shouldReceive('incrementDownloads')->never();
});
$this->get('/download/artwork/'.$artwork->id)->assertNotFound();
expect(DB::table('artwork_downloads')->where('artwork_id', $artwork->id)->count())->toBe(0);
});
it('keeps valid anonymous downloads and X-Accel-Redirect when enabled', function () {
config(['app.download_accel_enabled' => true, 'app.download_accel_path' => '/internal/originals']);
$hash = '2721722cb2f407cc53539ee8718cc89c4cdfc579';
m15cOriginal($hash, 'zip');
$artwork = Artwork::factory()->create([
'hash' => $hash,
'file_ext' => 'zip',
'file_name' => 'Anon File',
]);
$this->get('/download/artwork/'.$artwork->id)
->assertOk()
->assertHeader('X-Accel-Redirect', '/internal/originals/27/21/2721722cb2f407cc53539ee8718cc89c4cdfc579.zip');
});
it('still records ArtworkDownload.user_id for authenticated downloads', function () {
$hash = 'aa11bb22cc33dd44ee55ff6677889900aabbccdd';
m15cOriginal($hash, 'png');
$user = User::factory()->create();
$artwork = Artwork::factory()->create([
'hash' => $hash,
'file_ext' => 'png',
'file_name' => 'Auth File',
]);
$this->actingAs($user)->get('/download/artwork/'.$artwork->id)->assertOk();
$this->assertDatabaseHas('artwork_downloads', [
'artwork_id' => $artwork->id,
'user_id' => $user->id,
]);
});
it('skips session for anonymous download GET without a session cookie', function () {
$this->get('/download/artwork/999999')
->assertNotFound()
->assertHeader('X-Skinbase-Session', 'skipped');
});
it('starts session for download GET when a session cookie is already present', function () {
$cookie = (string) config('session.cookie');
$this->withCookie($cookie, 'existing-session-cookie')
->get('/download/artwork/999999')
->assertNotFound()
->assertHeader('X-Skinbase-Session', 'started');
});
it('does not record presence for artwork download requests', function () {
$this->mock(OnlineVisitorRepository::class, function ($mock): void {
$mock->shouldReceive('track')->never();
});
$this->get('/download/artwork/999999')->assertNotFound();
});