http/auth: simplify the challenge parser and Digest qop helper

[OmarAlJarrah]

Related: #111 tracks a separate AuthChallengeParser hardening bug (a stray trailing token after a valid auth-param); the cleanups below are independent and in the same file.

While reading through the WWW-Authenticate / Proxy-Authenticate handling in sdk-core's http/auth package, I came across a handful of small, self-contained simplifications. They're all behavior-preserving and confined to two files — AuthChallengeParser and DigestChallengeHandler — but together they remove some duplicated parsing logic and a little avoidable per-character overhead on the hot parse path. Grouping them here since they all live in the same subsystem.

Everything below touches only private members, so there's no public-API movement and apiCheck is unaffected. The existing AuthChallengeParserTest / DigestChallengeHandlerTest suites already exercise these paths.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/AuthChallengeParser.kt:272-290 — Reuse the quoted-string reader when recovering past a bad challenge

Old code:

        /** Advances to the next top-level comma (skipping over quoted strings). */
        fun recoverToNextChallenge() {
            while (position < len) {
                when (src[position]) {
                    ',' -> return
                    '"' -> {
                        // skip the quoted string — but if it's unterminated, just
                        // jump to EOF.
                        position++
                        while (position < len && src[position] != '"') {
                            if (src[position] == '\\' && position + 1 < len) position++
                            position++
                        }
                        if (position < len) position++ // closing quote
                    }
                    else -> position++
                }
            }
        }

New code:

        /** Advances to the next top-level comma (skipping over quoted strings). */
        fun recoverToNextChallenge() {
            while (position < len) {
                when (src[position]) {
                    ',' -> return
                    // Reuse the escape-aware reader; on an unterminated string it
                    // consumes through to EOF, exactly where recovery wants to land.
                    '"' -> readQuotedString()
                    else -> position++
                }
            }
        }

Usage — before → after: representative call site in parseChallenge (unchanged):

        val rawScheme =
            cursor.readToken() ?: run {
                cursor.recoverToNextChallenge()
                return null
            }

        val rawScheme =
            cursor.readToken() ?: run {
                cursor.recoverToNextChallenge()
                return null
            }

Call sites unchanged — behavior-preserving.

Why: The recovery loop carried a second, subtly-different quoted-string skip that has to stay byte-for-byte in step with readQuotedString() — same opening-quote skip, same \-escape handling, same run-to-EOF on an unterminated string, same landing one past the closing quote. Calling the existing reader (and discarding its result) keeps that logic in one place so a future fix can't drift across two copies.

API / Build: Private cursor method; no apiCheck impact. The String? result is intentionally discarded — a plain unused expression statement, not a flagged call.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/AuthChallengeParser.kt:143-178 — Hoist the duplicated token68 rewind into one local helper

Old code:

    private fun parseAuthParamOrToken68(cursor: Cursor): Pair<String, String>? {
        val saved = cursor.position
        val name = cursor.readToken() ?: return null
        cursor.skipOws()
        if (!cursor.hasMore() || cursor.peek() != '=') {
            // No `=` after the token — it's a token68.
            cursor.position = saved
            val token68 = cursor.readToken68() ?: return null
            return "token68" to token68
        }
        cursor.advance() // consume the first `=`

        // Token68 may carry one or more `=` pad chars (e.g. `cmVhbA==`). After
        // consuming the first `=`, if the very next character is another `=`, we
        // are clearly looking at token68 padding rather than the BWS-allowed gap
        // before an auth-param value — rewind and reparse as token68 so the
        // entire `cmVhbA==` is recovered. Without this branch a doubly-padded
        // base64 token would be silently dropped.
        if (cursor.hasMore() && cursor.peek() == '=') {
            cursor.position = saved
            val token68 = cursor.readToken68() ?: return null
            return "token68" to token68
        }
        cursor.skipOws()

        // We've already consumed the `=`; check whether what follows is a value.
        if (!cursor.hasMore() || cursor.peek() == ',') {
            // looked like `key=` with nothing after — try to treat the whole
            // thing as token68 (rewind and read it as such).
            cursor.position = saved
            val token68 = cursor.readToken68() ?: return null
            return "token68" to token68
        }
        val value = cursor.readTokenOrQuotedString() ?: return null
        return name.lowercase(Locale.US) to value
    }

New code:

    private fun parseAuthParamOrToken68(cursor: Cursor): Pair<String, String>? {
        val saved = cursor.position
        fun rewindAsToken68(): Pair<String, String>? {
            cursor.position = saved
            val token68 = cursor.readToken68() ?: return null
            return "token68" to token68
        }

        val name = cursor.readToken() ?: return null
        cursor.skipOws()
        if (!cursor.hasMore() || cursor.peek() != '=') {
            // No `=` after the token — it's a token68.
            return rewindAsToken68()
        }
        cursor.advance() // consume the first `=`

        // Token68 may carry one or more `=` pad chars (e.g. `cmVhbA==`). After
        // consuming the first `=`, if the very next character is another `=`, we
        // are clearly looking at token68 padding rather than the BWS-allowed gap
        // before an auth-param value — rewind and reparse as token68 so the
        // entire `cmVhbA==` is recovered. Without this branch a doubly-padded
        // base64 token would be silently dropped.
        if (cursor.hasMore() && cursor.peek() == '=') {
            return rewindAsToken68()
        }
        cursor.skipOws()

        // We've already consumed the `=`; check whether what follows is a value.
        if (!cursor.hasMore() || cursor.peek() == ',') {
            // looked like `key=` with nothing after — try to treat the whole
            // thing as token68 (rewind and read it as such).
            return rewindAsToken68()
        }
        val value = cursor.readTokenOrQuotedString() ?: return null
        return name.lowercase(Locale.US) to value
    }

Usage — before → after: the sole caller in parseChallenge (unchanged):

        val first =
            parseAuthParamOrToken68(cursor) ?: run {
                cursor.recoverToNextChallenge()
                return null
            }

        val first =
            parseAuthParamOrToken68(cursor) ?: run {
                cursor.recoverToNextChallenge()
                return null
            }

Call sites unchanged — behavior-preserving.

Why: The "rewind to saved, read a token68, yield "token68" to … (or null)" sequence was copy-pasted across all three disambiguation branches. A local function closing over saved and cursor states it once, so the three branches now read as the decision they actually make.

API / Build: Private method internals; no signature change, no apiCheck impact. The local function inherits the method's @Suppress("ReturnCount"), and on its own (two returns) it is within detekt's default threshold anyway.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/AuthChallengeParser.kt:293-303 — Back the token-char sets with String constants

Old code:

    private val TOKEN_PUNCTUATION: Set<Char> = "!#$%&'*+-.^_`|~".toSet()

    private val TOKEN68_PUNCTUATION: Set<Char> = "-._~+/".toSet()

    /** RFC 7230 token char: ALPHA / DIGIT / one of the punctuation set. */
    private fun isTokenChar(c: Char): Boolean =
        (c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') || c in TOKEN_PUNCTUATION

    /** RFC 7235 token68 char (excluding the trailing "=" pad, handled separately). */
    private fun isToken68Char(c: Char): Boolean =
        (c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') || c in TOKEN68_PUNCTUATION

New code:

    private const val TOKEN_PUNCTUATION = "!#$%&'*+-.^_`|~"

    private const val TOKEN68_PUNCTUATION = "-._~+/"

    /** RFC 7230 token char: ALPHA / DIGIT / one of the punctuation set. */
    private fun isTokenChar(c: Char): Boolean =
        (c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') || c in TOKEN_PUNCTUATION

    /** RFC 7235 token68 char (excluding the trailing "=" pad, handled separately). */
    private fun isToken68Char(c: Char): Boolean =
        (c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') || c in TOKEN68_PUNCTUATION

Usage — before → after: the hot consumer Cursor.readToken (source unchanged):

        fun readToken(): String? {
            val start = position
            while (position < len && isTokenChar(src[position])) position++
            return if (position == start) null else src.substring(start, position)
        }

        fun readToken(): String? {
            val start = position
            while (position < len && isTokenChar(src[position])) position++
            return if (position == start) null else src.substring(start, position)
        }

Call sites unchanged — behavior-preserving. The membership test c in TOKEN_PUNCTUATION now resolves to CharSequence.contains instead of Set.contains.

Why: isTokenChar/isToken68Char run once per input character, and every fallthrough boxed the Char to probe a HashSet. Against a String constant the same c in … expression compiles to CharSequence.contains — a direct scan of a ≤14-char literal — removing the per-character boxing and HashSet lookup, plus the one-time Set<Char> allocation at object init.

API / Build: Private members — no apiCheck impact. The $% in TOKEN_PUNCTUATION is a literal $ (not a template, since % can't start an identifier), so the literal is a valid compile-time const initializer; both names already use the SCREAMING_SNAKE_CASE ktlint expects for constants.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/DigestChallengeHandler.kt:375-381 — Collapse qopSupportsAuth to a single expression

Old code:

            /** True if [qop] is absent (legacy) or contains the `auth` token (case-insensitive). */
            private fun qopSupportsAuth(qop: String?): Boolean {
                if (qop == null) return true
                for (token in qop.split(',')) {
                    if (token.trim().equals("auth", ignoreCase = true)) return true
                }
                return false
            }

New code:

            /** True if [qop] is absent (legacy) or contains the `auth` token (case-insensitive). */
            private fun qopSupportsAuth(qop: String?): Boolean {
                if (qop == null) return true
                return qop.split(',').any { it.trim().equals("auth", ignoreCase = true) }
            }

Usage — before → after: the filter call in toCandidate (unchanged):

            val isInvalidChallenge =
                !challenge.scheme.equals("Digest", ignoreCase = true) ||
                    challenge.parameters["realm"] == null ||
                    challenge.parameters["nonce"] == null ||
                    !qopSupportsAuth(challenge.parameters["qop"])

            val isInvalidChallenge =
                !challenge.scheme.equals("Digest", ignoreCase = true) ||
                    challenge.parameters["realm"] == null ||
                    challenge.parameters["nonce"] == null ||
                    !qopSupportsAuth(challenge.parameters["qop"])

Call sites unchanged — behavior-preserving.

Why: The accumulate-and-return loop is exactly Iterable.any { … } with the same short-circuiting. The single expression states the intent directly, keeping only the legacy null guard.

Build: ignoreCase = true keeps spaces around = for the named argument (ktlint operator spacing), matching the existing equals(..., ignoreCase = true) call sites elsewhere in this file.