From 837fdf40319ff6f0182f7f69ff4b245d65aecaf8 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sat, 4 Jul 2026 19:43:32 +0200 Subject: [PATCH] fix(playback): canonicalize base URL scheme case FFmpeg's protocol lookup is case-sensitive, so a stored "Https://" base URL reaches mpv verbatim through the direct-play string concat and fails with "Protocol not found" (API calls survive because Dart's Uri lowercases the scheme). Canonicalize at Jellyfin URL intake and in the connection constructor so persisted configs self-heal on load, and register mpv-escaped (https\://) redaction variants so option-value logs stop leaking the server host. close #1465 --- lib/connection/connection.dart | 12 +++--- lib/services/jellyfin_endpoint_discovery.dart | 4 +- lib/utils/log_redaction_manager.dart | 23 ++++++++--- lib/utils/url_utils.dart | 15 ++++++++ test/services/jellyfin_client_urls_test.dart | 10 +++++ test/utils/log_redaction_manager_test.dart | 10 +++++ test/utils/url_utils_test.dart | 38 +++++++++++++++++++ 7 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 test/utils/url_utils_test.dart diff --git a/lib/connection/connection.dart b/lib/connection/connection.dart index d310fdf1..5bb4bc76 100644 --- a/lib/connection/connection.dart +++ b/lib/connection/connection.dart @@ -1,6 +1,7 @@ import '../media/media_backend.dart'; import '../models/plex/plex_home_user.dart'; import '../services/plex_auth_service.dart'; +import '../utils/url_utils.dart'; /// Identifier of a backend kind a [Connection] points at. Lighter-weight than /// [MediaBackend] for places that only care about persistence/auth shape @@ -233,7 +234,7 @@ class JellyfinConnection extends Connection { JellyfinConnection({ required this.id, - required this.baseUrl, + required String baseUrl, List? baseUrls, required this.serverName, required this.serverMachineId, @@ -245,7 +246,8 @@ class JellyfinConnection extends Connection { this.status = ConnectionStatus.unknown, required this.createdAt, this.lastAuthenticatedAt, - }) : baseUrls = _normalizeBaseUrls(baseUrl, baseUrls); + }) : baseUrl = canonicalizeBaseUrl(baseUrl), + baseUrls = _normalizeBaseUrls(baseUrl, baseUrls); @override ConnectionKind get kind => ConnectionKind.jellyfin; @@ -273,9 +275,9 @@ class JellyfinConnection extends Connection { final seen = {}; void add(String url) { - final trimmed = url.trim(); - if (trimmed.isEmpty || !seen.add(trimmed)) return; - result.add(trimmed); + final normalized = canonicalizeBaseUrl(url); + if (normalized.isEmpty || !seen.add(normalized)) return; + result.add(normalized); } add(activeBaseUrl); diff --git a/lib/services/jellyfin_endpoint_discovery.dart b/lib/services/jellyfin_endpoint_discovery.dart index b2adac6a..7f9f2043 100644 --- a/lib/services/jellyfin_endpoint_discovery.dart +++ b/lib/services/jellyfin_endpoint_discovery.dart @@ -263,12 +263,12 @@ class JellyfinEndpointDiscovery { } /// Normalizes a concrete Jellyfin base URL without inventing a scheme or port. - static String normalizeBaseUrl(String input) => stripTrailingSlash(input); + static String normalizeBaseUrl(String input) => canonicalizeBaseUrl(input); /// Expands a user-typed add/edit form entry into temporary probe candidates. /// These guesses are for discovery only; failed guesses should not be stored. static List expandInputToBaseUrls(String input) { - final trimmed = stripTrailingSlash(input); + final trimmed = canonicalizeBaseUrl(input); if (trimmed.isEmpty) return const []; if (_hasScheme(trimmed)) return [trimmed]; diff --git a/lib/utils/log_redaction_manager.dart b/lib/utils/log_redaction_manager.dart index 6fb31ce6..24f32773 100644 --- a/lib/utils/log_redaction_manager.dart +++ b/lib/utils/log_redaction_manager.dart @@ -3,7 +3,9 @@ import 'url_utils.dart'; class LogRedactionManager { // Size limits for bounded sets (FIFO eviction when exceeded) static const int _maxTokens = 50; - static const int _maxUrls = 20; + // Each server registers up to 8 literals (slash/origin/mpv-escaped + // variants), so keep headroom for several servers before FIFO eviction. + static const int _maxUrls = 40; static const int _maxCustomValues = 50; // Use LinkedHashSet for FIFO ordering @@ -74,22 +76,33 @@ class LogRedactionManager { final strippedSlash = stripTrailingSlash(normalized); if (strippedSlash.isNotEmpty) { - _addWithLimit(_urls, strippedSlash, _maxUrls); - _addWithLimit(_urls, '$strippedSlash/', _maxUrls); + _addUrl(strippedSlash); + _addUrl('$strippedSlash/'); } // Capture origin and host-level strings as well to cover most cases. if (uri != null && uri.host.isNotEmpty) { final origin = '${uri.scheme.isEmpty ? 'https' : uri.scheme}://${uri.host}${uri.hasPort ? ':${uri.port}' : ''}'; - _addWithLimit(_urls, origin, _maxUrls); + _addUrl(origin); if (origin.endsWith('/')) { - _addWithLimit(_urls, origin.substring(0, origin.length - 1), _maxUrls); + _addUrl(origin.substring(0, origin.length - 1)); } } _rebuildCombinedPattern(); } + /// Registers a URL literal plus the form mpv uses when echoing list-option + /// values (`:` escaped as `\:`, e.g. `sub-files=https\://host/...`), which + /// would otherwise slip past the literal match and leak the host. + static void _addUrl(String url) { + _addWithLimit(_urls, url, _maxUrls); + final escaped = url.replaceAll(':', r'\:'); + if (escaped != url) { + _addWithLimit(_urls, escaped, _maxUrls); + } + } + /// Convenience: register a server's URL and access token together. /// Call this before any HTTP traffic so the very first probe URL doesn't /// leak credentials verbatim. diff --git a/lib/utils/url_utils.dart b/lib/utils/url_utils.dart index 87fcb5ba..f352f03d 100644 --- a/lib/utils/url_utils.dart +++ b/lib/utils/url_utils.dart @@ -13,3 +13,18 @@ String stripTrailingSlash(String input) { } return trimmed; } + +final RegExp _schemePattern = RegExp(r'^[A-Za-z][A-Za-z\d+.-]*://'); + +/// Canonicalizes a server base URL: trims, strips one trailing `/`, and +/// lowercases the scheme (`Https://host` → `https://host`). Dart's `Uri` +/// normalizes scheme case for API requests, but URLs handed to the player as +/// raw strings don't get that treatment, and FFmpeg's protocol lookup is +/// case-sensitive — a mixed-case scheme fails with "Protocol not found". +/// Everything after `://` is left untouched. +String canonicalizeBaseUrl(String input) { + final stripped = stripTrailingSlash(input); + final match = _schemePattern.firstMatch(stripped); + if (match == null) return stripped; + return stripped.replaceRange(0, match.end, match.group(0)!.toLowerCase()); +} diff --git a/test/services/jellyfin_client_urls_test.dart b/test/services/jellyfin_client_urls_test.dart index 5392f77b..a3dca817 100644 --- a/test/services/jellyfin_client_urls_test.dart +++ b/test/services/jellyfin_client_urls_test.dart @@ -84,6 +84,16 @@ void main() { expect(Uri.parse(url).path, '/Videos/folder%2Fitem%20%231%3Fx/stream'); }); + test('buildDirectStreamUrl canonicalizes a mixed-case scheme from stored config', () async { + // This URL bypasses Dart's Uri normalization on its way to the player, + // and FFmpeg's protocol lookup is case-sensitive — a stored + // "Https://..." base URL fails with "Protocol not found" (#1465). + final mixedCase = await JellyfinClient.create(_conn(baseUrl: 'Https://jf.example.com/')); + addTearDown(mixedCase.close); + final url = mixedCase.buildDirectStreamUrl('item-99'); + expect(url, startsWith('https://jf.example.com/Videos/')); + }); + test('fetchSortOptions exposes the broad Jellyfin sort set', () async { final sorts = await client.fetchSortOptions('lib-1'); expect(sorts.map((sort) => sort.key).toList(), [ diff --git a/test/utils/log_redaction_manager_test.dart b/test/utils/log_redaction_manager_test.dart index 0a2d54e8..3145183c 100644 --- a/test/utils/log_redaction_manager_test.dart +++ b/test/utils/log_redaction_manager_test.dart @@ -177,6 +177,16 @@ void main() { expect(r1.contains('server.example.com'), isFalse); expect(r2.contains('server.example.com'), isFalse); }); + + test('redacts the mpv-escaped form used in option-value logs', () { + LogRedactionManager.registerServerUrl('https://server.example.com'); + // mpv echoes list options like sub-files with ':' escaped as '\:'. + final result = LogRedactionManager.redact( + r"Setting option 'sub-files' = 'https\://server.example.com/Videos/1/Subtitles/0/0/Stream.srt' (flags = 16)", + ); + expect(result.contains('server.example.com'), isFalse); + expect(result.contains('[REDACTED_URL]'), isTrue); + }); }); group('registerCustomValue', () { diff --git a/test/utils/url_utils_test.dart b/test/utils/url_utils_test.dart new file mode 100644 index 00000000..505fc3ed --- /dev/null +++ b/test/utils/url_utils_test.dart @@ -0,0 +1,38 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/utils/url_utils.dart'; + +void main() { + group('stripTrailingSlash', () { + test('removes a single trailing slash', () { + expect(stripTrailingSlash('https://host/'), 'https://host'); + }); + + test('trims whitespace and leaves slashless input unchanged', () { + expect(stripTrailingSlash(' https://host '), 'https://host'); + expect(stripTrailingSlash(''), ''); + }); + }); + + group('canonicalizeBaseUrl', () { + test('lowercases a mixed-case scheme (#1465)', () { + // FFmpeg's protocol lookup is case-sensitive; "Https://" reaching the + // player as a raw string fails with "Protocol not found". + expect(canonicalizeBaseUrl('Https://jellyfin.example.com'), 'https://jellyfin.example.com'); + expect(canonicalizeBaseUrl('HTTPS://jellyfin.example.com'), 'https://jellyfin.example.com'); + }); + + test('only touches the scheme, not host/path/query', () { + expect(canonicalizeBaseUrl('HTTP://Host.Example.com:8096/JellyFin'), 'http://Host.Example.com:8096/JellyFin'); + }); + + test('strips trailing slash and trims whitespace', () { + expect(canonicalizeBaseUrl(' Https://host:8096/jellyfin/ '), 'https://host:8096/jellyfin'); + }); + + test('leaves schemeless input unchanged', () { + expect(canonicalizeBaseUrl('host:8096'), 'host:8096'); + expect(canonicalizeBaseUrl('Host.example.com'), 'Host.example.com'); + expect(canonicalizeBaseUrl(''), ''); + }); + }); +}