diff --git a/lib/screens/settings/add_jellyfin_screen.dart b/lib/screens/settings/add_jellyfin_screen.dart index 8604bf63..d0a0f493 100644 --- a/lib/screens/settings/add_jellyfin_screen.dart +++ b/lib/screens/settings/add_jellyfin_screen.dart @@ -33,6 +33,24 @@ import 'async_form_state_mixin.dart'; import 'connection_persistence.dart'; import '../../widgets/loading_indicator_box.dart'; +@visibleForTesting +Future resolveJellyfinClientVersion({Future Function()? packageInfoLoader}) async { + const fallbackVersion = '1.0'; + try { + final packageInfo = await (packageInfoLoader == null ? PackageInfo.fromPlatform() : packageInfoLoader()); + final version = packageInfo.version.trim(); + if (version.isNotEmpty) return version; + appLogger.w('Package version is empty; using Jellyfin client version $fallbackVersion'); + } catch (error, stackTrace) { + appLogger.w( + 'Failed to resolve package version; using Jellyfin client version $fallbackVersion', + error: error, + stackTrace: stackTrace, + ); + } + return fallbackVersion; +} + @visibleForTesting bool shouldCreateLocalJellyfinProfile({ required Profile? targetProfile, @@ -426,9 +444,9 @@ class _AddJellyfinScreenState extends State with AsyncFormSta Future _buildAuthService() async { final authServiceFactory = widget._authServiceFactory; if (authServiceFactory != null) return await authServiceFactory(); - final pkg = await PackageInfo.fromPlatform(); + final clientVersion = await resolveJellyfinClientVersion(); final deviceName = await _resolveDeviceName(); - return JellyfinConnectionAuthService(clientName: 'Plezy', clientVersion: pkg.version, deviceName: deviceName); + return JellyfinConnectionAuthService(clientName: 'Plezy', clientVersion: clientVersion, deviceName: deviceName); } Future _resolveDeviceName() async { diff --git a/lib/services/jellyfin_auth_header.dart b/lib/services/jellyfin_auth_header.dart index 0a82aae5..624a1b4b 100644 --- a/lib/services/jellyfin_auth_header.dart +++ b/lib/services/jellyfin_auth_header.dart @@ -1,10 +1,14 @@ +import '../utils/device_identity.dart'; + /// Build the `MediaBrowser` Authorization header value the way the Jellyfin /// SDK formats it. Used at auth time and on every authenticated request so /// the server sees a consistent client identity. /// -/// Values are quote-stripped: the header grammar has no escape for `"`, so a -/// device name like `My "cool" TV` would otherwise corrupt every field after -/// it. +/// Unsafe header characters and embedded quotes are removed. Jellyfin requires +/// non-empty client, device, and version fields when creating a session, so +/// those values use stable fallbacks. An empty device ID is omitted for +/// authenticated requests, where Jellyfin can recover it from the token; +/// unauthenticated entry points must call [requireJellyfinDeviceId]. String buildJellyfinAuthHeader({ required String clientName, required String clientVersion, @@ -12,13 +16,35 @@ String buildJellyfinAuthHeader({ required String deviceId, String? accessToken, }) { - String quoted(String value) => '"${value.replaceAll('"', '')}"'; + String clean(String value) => (sanitizeHeaderValue(value) ?? '').replaceAll('"', ''); + + final client = clean(clientName); + final effectiveClient = client.isEmpty ? 'Plezy' : client; + final device = clean(deviceName); + final effectiveDevice = device.isEmpty ? effectiveClient : device; + final id = clean(deviceId); + final version = clean(clientVersion); + final token = accessToken == null ? '' : clean(accessToken); + String quoted(String value) => '"$value"'; + final parts = [ - 'Client=${quoted(clientName)}', - 'Device=${quoted(deviceName)}', - 'DeviceId=${quoted(deviceId)}', - 'Version=${quoted(clientVersion)}', - if (accessToken != null && accessToken.isNotEmpty) 'Token=${quoted(accessToken)}', + 'Client=${quoted(effectiveClient)}', + 'Device=${quoted(effectiveDevice)}', + if (id.isNotEmpty) 'DeviceId=${quoted(id)}', + 'Version=${quoted(version.isEmpty ? '1.0' : version)}', + if (token.isNotEmpty) 'Token=${quoted(token)}', ]; return 'MediaBrowser ${parts.join(', ')}'; } + +/// Validates the stable device identity required by unauthenticated Jellyfin +/// session creation. Never substitute a placeholder: Jellyfin keys sessions +/// and access tokens by this value, so a shared fallback would collide across +/// installations. +String requireJellyfinDeviceId(String deviceId) { + final sanitized = sanitizeHeaderValue(deviceId); + if (sanitized == null || sanitized != deviceId || sanitized.contains('"')) { + throw ArgumentError.value(deviceId, 'deviceId', 'must be a non-empty HTTP-safe value'); + } + return sanitized; +} diff --git a/lib/services/jellyfin_auth_service.dart b/lib/services/jellyfin_auth_service.dart index 5dac5ee2..ba7c7020 100644 --- a/lib/services/jellyfin_auth_service.dart +++ b/lib/services/jellyfin_auth_service.dart @@ -114,6 +114,7 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { required String deviceId, JellyfinServerInfo? serverInfo, }) async { + final validDeviceId = requireJellyfinDeviceId(deviceId); final normalised = _normaliseBaseUrl(baseUrl); final info = serverInfo ?? await probe(normalised); @@ -121,7 +122,7 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { clientName: clientName, clientVersion: clientVersion, deviceName: deviceName, - deviceId: deviceId, + deviceId: validDeviceId, ); final client = _buildHttpClient( baseUrl: normalised, @@ -147,7 +148,7 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { userId: auth.userId, userName: auth.userName, accessToken: auth.accessToken, - deviceId: deviceId, + deviceId: validDeviceId, isAdministrator: auth.isAdministrator, ); } finally { @@ -181,12 +182,13 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { required String baseUrl, required String deviceId, }) async { + final validDeviceId = requireJellyfinDeviceId(deviceId); final normalised = _normaliseBaseUrl(baseUrl); final authHeader = buildJellyfinAuthHeader( clientName: clientName, clientVersion: clientVersion, deviceName: deviceName, - deviceId: deviceId, + deviceId: validDeviceId, ); final client = _buildHttpClient(baseUrl: normalised, headers: {'Authorization': authHeader}); try { @@ -234,6 +236,7 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { Duration timeout = const Duration(minutes: 5), bool Function()? shouldCancel, }) async { + final validDeviceId = requireJellyfinDeviceId(deviceId); final normalised = _normaliseBaseUrl(baseUrl); final info = serverInfo ?? await probe(normalised); LogRedactionManager.registerCustomValue(secret); @@ -242,7 +245,7 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { clientName: clientName, clientVersion: clientVersion, deviceName: deviceName, - deviceId: deviceId, + deviceId: validDeviceId, ); // Reuse a single client across the polling loop — opening one per tick // would churn TCP connections needlessly on a 5-minute window. @@ -312,7 +315,7 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { userId: auth.userId, userName: auth.userName, accessToken: auth.accessToken, - deviceId: deviceId, + deviceId: validDeviceId, isAdministrator: auth.isAdministrator, ); } finally { diff --git a/lib/utils/device_identity.dart b/lib/utils/device_identity.dart index 863af9a7..c722ee19 100644 --- a/lib/utils/device_identity.dart +++ b/lib/utils/device_identity.dart @@ -99,13 +99,14 @@ class DeviceIdentityService { } /// Makes a free-form device name safe to send as an HTTP header value: -/// drops CR/LF and any code unit above 0xFF (dart:io's HttpHeaders throws a -/// FormatException on non-latin-1 — an emoji in an iPhone name would -/// otherwise kill every request), trims, and returns null when nothing -/// usable remains. +/// drops HTTP control characters and code units above 0xFF (dart:io's +/// HttpHeaders rejects them), trims, and returns null when nothing usable +/// remains. String? sanitizeHeaderValue(String? value) { if (value == null) return null; - final filtered = String.fromCharCodes(value.codeUnits.where((unit) => unit != 0x0D && unit != 0x0A && unit <= 0xFF)); + final filtered = String.fromCharCodes( + value.codeUnits.where((unit) => unit >= 0x20 && unit != 0x7F && unit <= 0xFF), + ); final trimmed = filtered.trim(); return trimmed.isEmpty ? null : trimmed; } diff --git a/test/screens/settings/add_jellyfin_screen_test.dart b/test/screens/settings/add_jellyfin_screen_test.dart index 08ac3338..f7edd369 100644 --- a/test/screens/settings/add_jellyfin_screen_test.dart +++ b/test/screens/settings/add_jellyfin_screen_test.dart @@ -6,6 +6,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; +import 'package:package_info_plus/package_info_plus.dart'; import 'package:plezy/connection/connection.dart'; import 'package:plezy/connection/connection_registry.dart'; import 'package:plezy/database/app_database.dart'; @@ -312,6 +313,38 @@ class _RouteHarness { Future> _noLocalServers() async => const []; void main() { + group('resolveJellyfinClientVersion', () { + PackageInfo packageInfo(String version) => PackageInfo( + appName: 'Plezy', + packageName: 'com.example.plezy', + version: version, + buildNumber: '1', + ); + + test('uses a non-empty package version', () async { + final version = await resolveJellyfinClientVersion( + packageInfoLoader: () async => packageInfo(' 2.9.1 '), + ); + expect(version, '2.9.1'); + }); + + test('falls back when the package version is empty', () async { + for (final packageVersion in ['', ' ']) { + final version = await resolveJellyfinClientVersion( + packageInfoLoader: () async => packageInfo(packageVersion), + ); + expect(version, '1.0'); + } + }); + + test('falls back when package metadata lookup throws', () async { + final version = await resolveJellyfinClientVersion( + packageInfoLoader: () async => throw StateError('version metadata unavailable'), + ); + expect(version, '1.0'); + }); + }); + tearDown(() { TvDetectionService.debugSetAppleTVOverride(null); TvDetectionService.setForceTVSync(false); diff --git a/test/services/jellyfin_auth_header_test.dart b/test/services/jellyfin_auth_header_test.dart index 3c51bdb7..1da891fe 100644 --- a/test/services/jellyfin_auth_header_test.dart +++ b/test/services/jellyfin_auth_header_test.dart @@ -40,5 +40,36 @@ void main() { ); expect(header, contains('Device="My cool TV"')); }); + + test('uses non-empty fallbacks for required session identity fields', () { + final header = buildJellyfinAuthHeader( + clientName: '', + clientVersion: ' ', + deviceName: '\u0000\u007f', + deviceId: 'dev-1', + ); + + expect(header, 'MediaBrowser Client="Plezy", Device="Plezy", DeviceId="dev-1", Version="1.0"'); + }); + + test('omits an empty device ID instead of emitting a malformed field', () { + final header = buildJellyfinAuthHeader( + clientName: 'Plezy', + clientVersion: '1.2.3', + deviceName: 'Living Room', + deviceId: '', + accessToken: 'tok', + ); + + expect(header, isNot(contains('DeviceId='))); + expect(header, contains('Token="tok"')); + }); + + test('rejects an empty or unsafe unauthenticated device ID', () { + for (final deviceId in ['', ' dev-1 ', 'dev\u0000-1', '"dev-1"']) { + expect(() => requireJellyfinDeviceId(deviceId), throwsArgumentError); + } + expect(requireJellyfinDeviceId('dev-1'), 'dev-1'); + }); }); } diff --git a/test/services/jellyfin_auth_service_test.dart b/test/services/jellyfin_auth_service_test.dart index b9450e22..58bbcf78 100644 --- a/test/services/jellyfin_auth_service_test.dart +++ b/test/services/jellyfin_auth_service_test.dart @@ -673,4 +673,74 @@ void main() { expect(fired, isFalse); }); }); + + group('Jellyfin authentication request identity', () { + test('password login sends the complete MediaBrowser header', () async { + late http.BaseRequest request; + final svc = _service( + handler: (captured) { + request = captured; + return _ok({ + 'AccessToken': 'tok-new', + 'User': {'Id': 'user-7', 'Name': 'edde'}, + }); + }, + ); + + await svc.authenticateByName( + baseUrl: 'https://jf.example.com', + username: 'edde', + password: 'pw', + deviceId: 'dev-xyz', + serverInfo: _serverInfo, + ); + + expect(request.method, 'POST'); + expect( + request.headers['authorization'], + 'MediaBrowser Client="Plezy", Device="TestDevice", DeviceId="dev-xyz", Version="test"', + ); + expect(request.headers['content-type'], 'application/json'); + }); + + test('Quick Connect sends the same complete MediaBrowser header', () async { + late http.BaseRequest request; + final svc = _service( + handler: (captured) { + request = captured; + return _ok({'Code': 'ABCDE', 'Secret': 'sec-xyz'}); + }, + ); + + await svc.initiateQuickConnect(baseUrl: 'https://jf.example.com', deviceId: 'dev-xyz'); + + expect(request.method, 'GET'); + expect( + request.headers['authorization'], + 'MediaBrowser Client="Plezy", Device="TestDevice", DeviceId="dev-xyz", Version="test"', + ); + }); + + test('rejects an empty device ID before sending a request', () async { + var requests = 0; + final svc = _service( + handler: (_) { + requests++; + return _status(500); + }, + ); + + await expectLater( + svc.authenticateByName( + baseUrl: 'https://jf.example.com', + username: 'edde', + password: 'pw', + deviceId: '', + serverInfo: _serverInfo, + ), + throwsArgumentError, + ); + expect(requests, 0); + }); + }); } diff --git a/test/utils/device_identity_test.dart b/test/utils/device_identity_test.dart index 507849a5..e52d7fd8 100644 --- a/test/utils/device_identity_test.dart +++ b/test/utils/device_identity_test.dart @@ -13,7 +13,8 @@ void main() { expect(sanitizeHeaderValue('电视'), isNull); }); - test('strips CR/LF', () { + test('strips HTTP control characters', () { + expect(sanitizeHeaderValue('\u0000Living\u001f Room\u007f TV'), 'Living Room TV'); expect(sanitizeHeaderValue('evil\r\nX-Injected: 1'), 'evilX-Injected: 1'); });