Move createAuthorizationHeader function to separate AuthorizationHeaderBuilder for easier re-use and testing (#267)

jellyfin-api/api/jellyfin-api.api

@@ -1,5 +1,6 @@
 public abstract interface class org/jellyfin/sdk/api/client/ApiClient {
-	public abstract fun createAuthorizationHeader ()Ljava/lang/String;
+	public static final field Companion Lorg/jellyfin/sdk/api/client/ApiClient$Companion;
+	public static final field QUERY_ACCESS_TOKEN Ljava/lang/String;
 	public abstract fun createUrl (Ljava/lang/String;Ljava/util/Map;Ljava/util/Map;Z)Ljava/lang/String;
 	public abstract fun getAccessToken ()Ljava/lang/String;
 	public abstract fun getBaseUrl ()Ljava/lang/String;
@@ -12,6 +13,10 @@ public abstract interface class org/jellyfin/sdk/api/client/ApiClient {
 	public abstract fun setDeviceInfo (Lorg/jellyfin/sdk/model/DeviceInfo;)V
 }
 
+public final class org/jellyfin/sdk/api/client/ApiClient$Companion {
+	public static final field QUERY_ACCESS_TOKEN Ljava/lang/String;
+}
+
 public final class org/jellyfin/sdk/api/client/ApiClient$DefaultImpls {
 	public static synthetic fun createUrl$default (Lorg/jellyfin/sdk/api/client/ApiClient;Ljava/lang/String;Ljava/util/Map;Ljava/util/Map;ZILjava/lang/Object;)Ljava/lang/String;
 }
@@ -38,7 +43,6 @@ public final class org/jellyfin/sdk/api/client/HttpClientOptions {
 public class org/jellyfin/sdk/api/client/KtorClient : org/jellyfin/sdk/api/client/ApiClient {
 	public fun <init> (Ljava/lang/String;Ljava/lang/String;Lorg/jellyfin/sdk/model/ClientInfo;Lorg/jellyfin/sdk/model/DeviceInfo;Lorg/jellyfin/sdk/api/client/HttpClientOptions;)V
 	public synthetic fun <init> (Ljava/lang/String;Ljava/lang/String;Lorg/jellyfin/sdk/model/ClientInfo;Lorg/jellyfin/sdk/model/DeviceInfo;Lorg/jellyfin/sdk/api/client/HttpClientOptions;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
-	public fun createAuthorizationHeader ()Ljava/lang/String;
 	public fun createUrl (Ljava/lang/String;Ljava/util/Map;Ljava/util/Map;Z)Ljava/lang/String;
 	public fun getAccessToken ()Ljava/lang/String;
 	public fun getBaseUrl ()Ljava/lang/String;
@@ -108,6 +112,15 @@ public final class org/jellyfin/sdk/api/client/extensions/UserApiExtensionsKt {
 	public static final fun authenticateUserByName (Lorg/jellyfin/sdk/api/operations/UserApi;Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 }
 
+public final class org/jellyfin/sdk/api/client/util/AuthorizationHeaderBuilder {
+	public static final field AUTHORIZATION_SCHEME Ljava/lang/String;
+	public static final field INSTANCE Lorg/jellyfin/sdk/api/client/util/AuthorizationHeaderBuilder;
+	public final fun buildHeader (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;
+	public static synthetic fun buildHeader$default (Lorg/jellyfin/sdk/api/client/util/AuthorizationHeaderBuilder;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Ljava/lang/String;
+	public final fun buildParameter (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;
+	public final fun encodeParameterValue (Ljava/lang/String;)Ljava/lang/String;
+}
+
 public final class org/jellyfin/sdk/api/client/util/PathBuilder {
 	public static final field INSTANCE Lorg/jellyfin/sdk/api/client/util/PathBuilder;
 	public static final field TOKEN_BRACKET_CLOSE C

jellyfin-api/src/main/kotlin/org/jellyfin/sdk/api/client/ApiClient.kt

@@ -4,6 +4,14 @@ import org.jellyfin.sdk.model.ClientInfo
 import org.jellyfin.sdk.model.DeviceInfo
 
 public interface ApiClient {
+	public companion object {
+		/**
+		 * The query parameter to use for access tokens. Used in the [createUrl] function when includeCredentials is
+		 * true.
+		 */
+		public const val QUERY_ACCESS_TOKEN: String = "ApiKey"
+	}
+
 	/**
 	 * URL to use as base for API endpoints. Should include the protocol and may contain a path.
 	 */
@@ -34,18 +42,13 @@ public interface ApiClient {
 	 * Create a complete url based on the [baseUrl] and given parameters.
 	 * Uses [PathBuilder] to create the path from the [pathTemplate] and [pathParameters].
 	 *
-	 * When [includeCredentials] is true it will add the access token to the
-	 * url to make an authenticated request.
+	 * When [includeCredentials] is true it will add the access token as query parameter using [QUERY_ACCESS_TOKEN]
+	 * to the url to make an authenticated request.
 	 */
 	public fun createUrl(
 		pathTemplate: String,
 		pathParameters: Map<String, Any?> = emptyMap(),
 		queryParameters: Map<String, Any?> = emptyMap(),
-		includeCredentials: Boolean = false
+		includeCredentials: Boolean = false,
 	): String
-
-	/**
-	 * Create the value of the "Authorization" header.
-	 */
-	public fun createAuthorizationHeader(): String?
 }

jellyfin-api/src/main/kotlin/org/jellyfin/sdk/api/client/KtorClient.kt

@@ -15,11 +15,11 @@ import kotlinx.serialization.json.Json
 import org.jellyfin.sdk.api.client.exception.InvalidContentException
 import org.jellyfin.sdk.api.client.exception.InvalidStatusException
 import org.jellyfin.sdk.api.client.exception.TimeoutException
+import org.jellyfin.sdk.api.client.util.AuthorizationHeaderBuilder
 import org.jellyfin.sdk.api.client.util.PathBuilder
 import org.jellyfin.sdk.model.ClientInfo
 import org.jellyfin.sdk.model.DeviceInfo
 import org.slf4j.LoggerFactory
-import kotlin.collections.set
 
 public open class KtorClient(
 	override var baseUrl: String? = null,
@@ -89,59 +89,10 @@ public open class KtorClient(
 				}
 
 			if (includeCredentials)
-				parameters.append("ApiKey", checkNotNull(accessToken))
+				parameters.append(ApiClient.QUERY_ACCESS_TOKEN, checkNotNull(accessToken))
 		}.buildString()
 	}
 
-	/**
-	 * This is a temporary workaround until we have a proper way to send device names to the server.
-	 * It will filter out all special characters.
-	 */
-	private fun String.encodeAuthorizationHeaderValue() = replace("[^\\w\\s]".toRegex(), "").trim()
-
-	override fun createAuthorizationHeader(): String? {
-		val params = mutableMapOf(
-			"client" to clientInfo.name,
-			"version" to clientInfo.version,
-			"deviceId" to deviceInfo.id,
-			// Only encode the device name as it is user input
-			// other fields should be validated manually by the client
-			"device" to deviceInfo.name.encodeAuthorizationHeaderValue()
-		)
-
-		// Only set access token when it's not null
-		accessToken?.let { params["token"] = it }
-
-		// Format: `MediaBrowser key1="value1", key2="value2"`
-		return params.entries.joinToString(
-			separator = ", ",
-			prefix = "MediaBrowser ",
-			transform = {
-				// Check for bad strings to prevent endless hours debugging why the server throws http 500 errors
-				require(!it.key.contains('=')) {
-					"Key ${it.key} can not contain the = character in the authorization header"
-				}
-				require(!it.key.contains(',')) {
-					"Key ${it.key} can not contain the , character in the authorization header"
-				}
-				require(!it.key.startsWith('"') && !it.key.endsWith('"')) {
-					"Key ${it.key} can not start or end with the \" character in the authorization header"
-				}
-				require(!it.value.contains('=')) {
-					"Value ${it.value} (for key ${it.key}) can not contain the = character in the authorization header"
-				}
-				require(!it.value.contains(',')) {
-					"Value ${it.value} (for key ${it.key}) can not contain the , character in the authorization header"
-				}
-				require(!it.value.startsWith('"') && !it.value.endsWith('"')) {
-					"Value ${it.value} (for key ${it.key}) can not start or end with the \" character in the authorization header"
-				}
-
-				// key="value"
-				"""${it.key.capitalize()}="${it.value}""""
-			})
-	}
-
 	public suspend inline fun <reified T> request(
 		method: HttpMethod = HttpMethod.Get,
 		pathTemplate: String,
@@ -161,7 +112,16 @@ public open class KtorClient(
 			val response = client.request<HttpResponse> {
 				this.method = method
 				url(url)
-				header(HttpHeaders.Authorization, createAuthorizationHeader())
+				header(
+					key = HttpHeaders.Authorization,
+					value = AuthorizationHeaderBuilder.buildHeader(
+						clientName = clientInfo.name,
+						clientVersion = clientInfo.version,
+						deviceId = deviceInfo.id,
+						deviceName = deviceInfo.name,
+						accessToken = accessToken
+					)
+				)
 
 				if (requestBody != null) body = defaultSerializer().write(requestBody)
 			}

jellyfin-api/src/main/kotlin/org/jellyfin/sdk/api/client/util/AuthorizationHeaderBuilder.kt

@@ -0,0 +1,62 @@
+package org.jellyfin.sdk.api.client.util
+
+public object AuthorizationHeaderBuilder {
+	public const val AUTHORIZATION_SCHEME: String = "MediaBrowser"
+	private val ENCODING_REGEX = "[^\\w\\s]".toRegex()
+
+	public fun encodeParameterValue(raw: String): String = raw.replace(ENCODING_REGEX, "").trim()
+
+	public fun buildParameter(key: String, value: String): String {
+		// Check for bad strings to prevent endless hours debugging why the server throws http 500 errors
+		require(!key.contains('=')) {
+			"Key $key can not contain the = character in the authorization header"
+		}
+		require(!key.contains(',')) {
+			"Key $key can not contain the , character in the authorization header"
+		}
+		require(!key.startsWith('"') && !key.endsWith('"')) {
+			"Key $key can not start or end with the \" character in the authorization header"
+		}
+		require(!value.contains('=')) {
+			"Value $value (for key $key) can not contain the = character in the authorization header"
+		}
+		require(!value.contains(',')) {
+			"Value $value (for key $key) can not contain the , character in the authorization header"
+		}
+		require(!value.startsWith('"') && !value.endsWith('"')) {
+			"Value $value (for key $key) can not start or end with the \" character in the authorization header"
+		}
+
+		// key="value"
+		return """${key}="$value""""
+	}
+
+	public fun buildHeader(
+		clientName: String,
+		clientVersion: String,
+		deviceId: String,
+		deviceName: String,
+		accessToken: String? = null,
+	): String {
+		val params = arrayOf(
+			"Client" to clientName,
+			"Version" to clientVersion,
+			"DeviceId" to deviceId,
+			// Only encode the device name as it is user input
+			// other fields should be validated manually by the client
+			"Device" to encodeParameterValue(deviceName),
+			"Token" to accessToken
+		)
+
+		// Format: `MediaBrowser key1="value1", key2="value2"`
+		return params
+			// Drop null values (token)
+			.filterNot { (_, value) -> value == null }
+			// Join parts
+			.joinToString(
+				separator = ", ",
+				prefix = "$AUTHORIZATION_SCHEME ",
+				transform = { (key, value) -> buildParameter(key, value!!) }
+			)
+	}
+}

jellyfin-api/src/test/kotlin/org/jellyfin/sdk/api/client/AuthorizationHeaderBuilderTests.kt

@@ -0,0 +1,37 @@
+package org.jellyfin.sdk.api.client
+
+import org.jellyfin.sdk.api.client.util.AuthorizationHeaderBuilder.buildHeader
+import org.jellyfin.sdk.api.client.util.AuthorizationHeaderBuilder.buildParameter
+import org.jellyfin.sdk.api.client.util.AuthorizationHeaderBuilder.encodeParameterValue
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+public class AuthorizationHeaderBuilderTests {
+	@Test
+	public fun `encodeParameter removes special characters`() {
+		assertEquals("test", encodeParameterValue("""test"""))
+		assertEquals("test", encodeParameterValue("""test+"""))
+		assertEquals("test", encodeParameterValue("""'test'"""))
+		assertEquals("", encodeParameterValue("""今日は"""))
+		assertEquals("", encodeParameterValue("""水母"""))
+	}
+
+	@Test
+	public fun `buildParameter creates a valid header with access token`() {
+		assertEquals("""key="val"""", buildParameter("key", "val"))
+		assertEquals("""one="two"""", buildParameter("one", "two"))
+		assertEquals("""1="2"""", buildParameter("1", "2"))
+	}
+
+	@Test
+	public fun `buildAuthorizationHeader creates a valid header with access token`() {
+		val value = buildHeader("a", "b", "c", "d", "e")
+		assertEquals("""MediaBrowser Client="a", Version="b", DeviceId="c", Device="d", Token="e"""", value)
+	}
+
+	@Test
+	public fun `buildAuthorizationHeader creates a valid header without access token`() {
+		val value = buildHeader("a", "b", "c", "d")
+		assertEquals("""MediaBrowser Client="a", Version="b", DeviceId="c", Device="d"""", value)
+	}
+}