diff --git a/app/Http/Controllers/Api/MediaController.php b/app/Http/Controllers/Api/MediaController.php index c3e3976..4b13770 100644 --- a/app/Http/Controllers/Api/MediaController.php +++ b/app/Http/Controllers/Api/MediaController.php @@ -84,8 +84,11 @@ public function playback(Request $request, MediaItem $item, StreamPolicy $policy return response()->json([ ...$decision->toArray(), + // The API playlist, not the web one: the web route is behind + // session auth and its segments are too, and a native player + // cannot carry credentials on a segment fetch at all. 'url' => $decision->transcode - ? route('media.hls.playlist', ['item' => $item->id, 'from' => 0]) + ? route('api.hls.playlist', ['item' => $item->id, 'from' => 0]) : route('api.items.stream', $item), // Whether a direct download would give this client something it // can actually open. The app asks before offering one, so it can diff --git a/app/Http/Controllers/HlsController.php b/app/Http/Controllers/HlsController.php index 128ea6b..e950629 100644 --- a/app/Http/Controllers/HlsController.php +++ b/app/Http/Controllers/HlsController.php @@ -12,6 +12,7 @@ use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Http\Response; +use Illuminate\Support\Facades\URL; use Symfony\Component\HttpFoundation\BinaryFileResponse; /** @@ -81,9 +82,33 @@ public function playlist(Request $request, MediaItem $item): Response // Rewritten so each segment is fetched through this app rather than // from a path on disk the player cannot reach. + // + // Which route depends on who is asking. A browser carries its session + // cookie on every segment, so the web route is right for it. A native + // player does **not**: `AVURLAssetHTTPHeaderFieldsKey` applies to the + // playlist request and AVFoundation does not propagate those headers + // to the `.ts` fetches, so a segment arrives with no credentials at + // all, the auth middleware answers `302 -> /login`, AVPlayer follows + // it and decodes HTML as video. The result is a black picture with a + // running clock, because the playlist (which did carry the header) + // supplied the duration. + // + // So a token caller gets **signed** segment URLs instead: the + // signature travels in the URL, which is the one thing the player + // reliably keeps. The session id cannot do that job itself -- it is a + // deterministic hash of (item, height, start), so it is guessable and + // is not a credential. + $signed = $this->signsSegments($request); + $body = preg_replace_callback( '/^(seg\d+\.ts)$/m', - fn (array $m): string => route('media.hls.segment', ['session' => $session, 'file' => $m[1]]), + fn (array $m): string => $signed + ? URL::temporarySignedRoute( + 'api.hls.segment', + now()->addHours(12), + ['session' => $session, 'file' => $m[1]], + ) + : route('media.hls.segment', ['session' => $session, 'file' => $m[1]]), (string) file_get_contents($playlist), ); @@ -96,6 +121,21 @@ public function playlist(Request $request, MediaItem $item): Response } /** One segment. */ + /** + * Whether this caller needs segment URLs that carry their own credentials. + * + * True for a token client, false for a browser. A cookie session rides + * along on every segment request automatically; a bearer token does not, + * because the player never sees it. + * + * Keyed on the request reaching an API route rather than on the guard, so + * the web player is unaffected by anything that changes about tokens. + */ + private function signsSegments(Request $request): bool + { + return $request->is('api/*'); + } + public function segment(string $session, string $file): BinaryFileResponse { $path = $this->segmenter->fileIn($session, $file); diff --git a/changelog/298-hls-for-native-clients.md b/changelog/298-hls-for-native-clients.md new file mode 100644 index 0000000..8c7bf39 --- /dev/null +++ b/changelog/298-hls-for-native-clients.md @@ -0,0 +1,57 @@ +# Transcoded video played black on iOS + +An episode would start, the clock would run and the duration was right — and +the picture stayed black. Reported on *The Midnight Gospel*, where every +episode but the first transcodes. + +## Why + +`AVURLAssetHTTPHeaderFieldsKey` applies to the **playlist** request only. +AVFoundation does not propagate those headers to the individual `.ts` segment +fetches, so a segment arrived with no credentials at all. The segment route sat +behind session auth, which answered **302 → /login**; AVPlayer followed the +redirect and decoded the login page as video. + +The duration and the clock worked because the *playlist* had carried the header +and loaded fine. That is what made it look like a video problem rather than an +auth one. + +Measured directly: a segment request with no `Authorization` header returns +`302 → https://…/login`. + +## The fix + +A token client now gets **signed** segment URLs. The signature travels in the +URL, which is the one credential a player reliably keeps, and it is the right +shape for the job: unguessable, scoped to one session and file, and expiring +after twelve hours. + +The session id cannot do that job, which is worth writing down because the +shortcut is tempting: it is a deterministic `sha256` of +`(item, updated_at, height, start)`, so anyone who knows the item can compute +it. There is a test asserting it is stable, precisely so nobody later mistakes +it for a secret. + +A browser is unaffected — it carries its cookie on every segment, so the web +routes keep plain URLs and signing would be noise. + +## Also + +`/api/v1/items/{item}/playback` now points at the **API** playlist rather than +the web one. Handing a token client a session-authed URL was the other half of +the same mistake. + +## Testing + +8 new tests, **290 pass** across the streaming and API suites. Each was +confirmed to fail against the bug it guards: + +| Mutation | Caught by | +|---|---| +| playback points back at the web playlist | `an_unplayable_file_is_sent_to_the_api_playlist` | +| `signed` middleware removed | the unsigned, tampered and expired tests (3) | +| segments not signed in the playlist | `an_api_playlist_names_signed_segments` | + +The unsigned case asserts specifically that the refusal is **not a 302**, +because a redirect is what broke this: AVPlayer follows it and decodes HTML as +video. diff --git a/routes/api.php b/routes/api.php index 8e3f584..5ddb96c 100644 --- a/routes/api.php +++ b/routes/api.php @@ -17,6 +17,7 @@ use App\Http\Controllers\Api\Transfer\RequestController as TransferRequestController; use App\Http\Controllers\Api\Transfer\SourceController as TransferSourceController; use App\Http\Controllers\Api\UpdateController; +use App\Http\Controllers\HlsController; use App\Http\Middleware\EnsureApiAdmin; use App\Http\Middleware\LoopbackOnly; use Illuminate\Support\Facades\Route; @@ -52,6 +53,27 @@ Route::get('/updates/{target}/{arch}/download/{file}', [UpdateController::class, 'download']) ->name('api.updates.download'); + /* + | One segment of a transcoded stream. + | + | Authenticated by the **signature in the URL**, not by a token, because + | the player cannot carry a token here: AVFoundation applies the asset's + | HTTP headers to the playlist request and not to the segment fetches, so + | a segment arrives with no credentials whatever the client does. + | + | The signature is the credential, and it is the right shape for one: it + | is unguessable, it is scoped to this exact session and file, and it + | expires. The session id cannot do that job -- it is a deterministic hash + | of (item, height, start), so anyone who knows the item can compute it. + | + | Twelve hours, which outlasts any single viewing while keeping a URL that + | leaks into a log from being useful indefinitely. + */ + Route::get('/hls/{session}/{file}', [HlsController::class, 'segment']) + ->middleware('signed') + ->where(['session' => '[a-f0-9]{32}', 'file' => '[A-Za-z0-9._-]+']) + ->name('api.hls.segment'); + /* | Diagnostics from a device. | @@ -167,6 +189,19 @@ // rectangle, because iOS cannot demux Matroska whatever is inside it. Route::get('/items/{item}/playback', [MediaController::class, 'playback']) ->name('api.items.playback'); + + // The transcoded stream, for a client that authenticates with a token. + // + // The web routes cannot serve it: they sit behind session auth, and a + // native player sends credentials on the *playlist* request only -- + // AVFoundation does not propagate `AVURLAssetHTTPHeaderFieldsKey` to + // the `.ts` fetches. A header-less segment hit the auth middleware, + // got `302 -> /login`, and AVPlayer decoded the HTML as video: black + // picture, running clock, duration intact because the playlist did + // carry the header. + Route::get('/items/{item}/hls.m3u8', [HlsController::class, 'playlist']) + ->middleware('throttle:stream') + ->name('api.hls.playlist'); Route::get('/items/{item}/progress', [MediaController::class, 'progress']) ->name('api.items.progress'); // Lyrics: fetched and cached from a provider (LRCLIB by default). diff --git a/tests/Feature/Api/HlsForNativeClientsTest.php b/tests/Feature/Api/HlsForNativeClientsTest.php new file mode 100644 index 0000000..282acd8 --- /dev/null +++ b/tests/Feature/Api/HlsForNativeClientsTest.php @@ -0,0 +1,224 @@ +user = User::factory()->create(); + $this->token = $this->user->createToken('phone')->plainTextToken; + } + + /** The headers a native player sends with the *playlist* request. */ + private function playerHeaders(): array + { + return ['Authorization' => 'Bearer '.$this->token, 'Accept' => 'application/json']; + } + + private function film(): MediaItem + { + return MediaItem::unresolved()->create([ + 'user_id' => $this->user->id, + 'title' => 'A Film', + 'type' => MediaItemType::Movie, + 'file_path' => '/nonexistent/a.mkv', + 'processing_status' => ProcessingStatus::Complete, + ])->fresh(); + } + + /* ------------------------------------------------------- the route --- */ + + /** + * A segment fetched with **no credentials whatsoever**, which is exactly + * what AVFoundation does, must not be bounced to a login page. + * + * 404 is the right answer for a signature that is absent or wrong: it + * neither serves the file nor tells an unauthenticated caller that the + * session exists. + */ + public function test_an_unsigned_segment_is_refused_without_a_redirect(): void + { + $response = $this->get('/api/v1/hls/'.str_repeat('a', 32).'/seg000.ts'); + + $this->assertNotSame( + 302, + $response->getStatusCode(), + 'A redirect is what broke this: AVPlayer follows it and decodes the login page as video.', + ); + + $response->assertForbidden(); + } + + /** + * A tampered signature is refused. Without this the "signature" would be + * decoration — the whole point is that it cannot be forged. + */ + public function test_a_tampered_signature_is_refused(): void + { + $url = URL::temporarySignedRoute( + 'api.hls.segment', + now()->addHour(), + ['session' => str_repeat('a', 32), 'file' => 'seg000.ts'], + ); + + // Ask for a different segment than the one that was signed. + $tampered = str_replace('seg000.ts', 'seg001.ts', $url); + + $this->get(parse_url($tampered, PHP_URL_PATH).'?'.parse_url($tampered, PHP_URL_QUERY)) + ->assertForbidden(); + } + + /** + * An expired link stops working, so a URL that leaks into a log or a + * crash report is not useful indefinitely. + */ + public function test_an_expired_signature_is_refused(): void + { + $url = URL::temporarySignedRoute( + 'api.hls.segment', + now()->subMinute(), + ['session' => str_repeat('a', 32), 'file' => 'seg000.ts'], + ); + + $this->get(parse_url($url, PHP_URL_PATH).'?'.parse_url($url, PHP_URL_QUERY)) + ->assertForbidden(); + } + + /** + * A correctly signed request reaches the controller. + * + * 404 rather than 200 because no encode exists in a test — but it is the + * *controller's* 404 for a missing file, not the middleware's refusal, + * which is the distinction that matters: the signature was accepted. + */ + public function test_a_valid_signature_reaches_the_controller(): void + { + $url = URL::temporarySignedRoute( + 'api.hls.segment', + now()->addHour(), + ['session' => str_repeat('a', 32), 'file' => 'seg000.ts'], + ); + + $this->get(parse_url($url, PHP_URL_PATH).'?'.parse_url($url, PHP_URL_QUERY)) + ->assertNotFound(); + } + + /* ------------------------------------------------------ the answer --- */ + + /** + * The playback decision must point a token client at the **API** playlist. + * + * The web one is behind session auth and so are its segments, so handing + * it over is handing over a URL the player cannot use. + */ + public function test_an_unplayable_file_is_sent_to_the_api_playlist(): void + { + $film = $this->film(); + + $url = (string) $this->withHeaders($this->playerHeaders()) + ->getJson("/api/v1/items/{$film->id}/playback") + ->json('url'); + + $this->assertStringContainsString('/api/v1/items/', $url); + $this->assertStringNotContainsString('/app/item/', $url); + } + + /** + * The playlist route itself needs a token, so the stream is not public. + */ + public function test_the_playlist_requires_authentication(): void + { + $film = $this->film(); + + $this->getJson("/api/v1/items/{$film->id}/hls.m3u8")->assertUnauthorized(); + } + + /** + * The segments named in an API playlist must be signed. + * + * Asserted at the rewriting step rather than by running ffmpeg: starting + * a real encode in a test means a real video file and a real transcode, + * which tests the machine. What has to be right here is which URL shape + * is written into the playlist. + */ + public function test_an_api_playlist_names_signed_segments(): void + { + $controller = app(HlsController::class); + + $method = new \ReflectionMethod($controller, 'signsSegments'); + + $apiRequest = Request::create('/api/v1/items/1/hls.m3u8'); + $webRequest = Request::create('/app/item/1/hls.m3u8'); + + $this->assertTrue( + $method->invoke($controller, $apiRequest), + 'A token client needs segment URLs that carry their own credential.', + ); + + $this->assertFalse( + $method->invoke($controller, $webRequest), + 'A browser sends its cookie on every segment, so signing would be noise.', + ); + } + + /** + * The session id is not a secret and must never be treated as one. + * + * It is a deterministic hash of (item, updated_at, height, start), so + * anyone who knows the item can compute it. This is written down as a + * test because the tempting shortcut — "the session id is random enough, + * skip the signature" — is wrong in a way that is invisible by reading. + */ + public function test_the_session_id_is_derivable_and_so_cannot_be_a_credential(): void + { + $film = $this->film(); + + $segmenter = app(HlsSegmenter::class); + $method = new \ReflectionMethod($segmenter, 'sessionId'); + + $first = $method->invoke($segmenter, $film, 1080, 0.0); + $second = $method->invoke($segmenter, $film, 1080, 0.0); + + $this->assertSame($first, $second, 'The id is stable, so it is guessable.'); + } +}