ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt (2)

297-338: Same missing-message issue as above.

Please add messages for UnsupportedOperationException/IllegalStateException in this iterator as well.

As per coding guidelines, Make error messages actionable by including the problematic value and context.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 297 -
338, The ValueCollection iterator throws bare exceptions; update
ValueCollection.add (the add(element: Value) override) to throw
UnsupportedOperationException with a clear message (e.g., "add not supported on
CaseInsensitiveMap.ValueCollection") and update the iterator.remove() path to
throw IllegalStateException with context when lastKey is null (e.g., "remove()
called before next() or after already removed for key=<lastKey?>"), and when
performing removal call this@CaseInsensitiveMap.remove(key) ensure the thrown
IllegalStateException includes the problematic key or state; reference the
ValueCollection class, its add(element: Value) method and the iterator's
remove() and lastKey variable.

---

340-381: Same missing-message issue as above.

Please add messages for UnsupportedOperationException/IllegalStateException in this iterator as well.

As per coding guidelines, Make error messages actionable by including the problematic value and context.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 340 -
381, Iterator exceptions in EntrySet lack actionable messages; update the throws
to include context and problematic values: in EntrySet.add(element) replace the
plain UnsupportedOperationException with one that includes the attempted element
and that EntrySet doesn't support add (e.g., "EntrySet.add not supported:
attempted to add $element"), and in EntrySet.iterator().remove() replace the
plain IllegalStateException with one that describes the invalid remove context
and includes the lastKey when available (e.g., "remove() called without a
preceding next() on EntrySet" or "remove() called after already removed for
key=$lastKey"). Ensure you update the throw sites in the EntrySet class (add and
iterator.remove) to include these messages.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

gradlew.bat (1)

72-74: Three consecutive blank lines between the two @rem markers.

Lines 72–74 leave three empty lines between @rem Setup the command line and @rem Execute Gradle, which are leftover whitespace from removed setup commands.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@gradlew.bat` around lines 72 - 74, Remove the three consecutive blank lines
left between the two `@rem` markers by collapsing the whitespace so there is at
most one blank line (or none) between "@rem Setup the command line" and "@rem
Execute Gradle"; edit the gradlew.bat section containing those `@rem` comments to
delete the extra empty lines so the file has no leftover blank-line gap.

ktor-utils/common/src/io/ktor/util/StringValues.kt (1)

183-205: values getter always reconstructs a case-sensitive LinkedHashMap, even when caseInsensitiveName = true.

Subclasses that relied on case-insensitive lookups via the values map will silently regress to case-sensitive behavior. This concern was already raised for the previous implementation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/StringValues.kt` around lines 183 - 205,
The values getter currently reconstructs a case-sensitive LinkedHashMap
regardless of caseInsensitiveName; update the getter in StringValues (the
protected val values) to preserve case-insensitive semantics by normalizing keys
or using a case-insensitive Map when caseInsensitiveName is true (e.g., insert
keys.lowercase() or use a map implementation that compares keys ignoring case)
so subclasses that depend on case-insensitive lookups keep the same behavior;
ensure you still preserve insertion order (LinkedHashMap semantics) and return
emptyMap() when entryCount == 0.

ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt (2)

289-290: Bare exception constructors flagged by detekt at lines 290, 333, and 376 — missing actionable messages.

All three IllegalStateException() calls in the iterator remove() methods of KeySet, ValueCollection, and EntrySet lack messages. As per coding guidelines, error messages should include the problematic value and context. The static-analysis tool (detekt ThrowingExceptionsWithoutMessageOrCause) flags all three. The UnsupportedOperationException() calls in the add() overrides (lines 253, 301, 344) share the same issue.

💡 Suggested fix (pattern to apply to all three iterators)

-    val key = lastKey ?: throw IllegalStateException()
+    val key = lastKey ?: throw IllegalStateException("remove() called before next() or after already removed")

-    override fun add(element: String): Boolean =
-        throw UnsupportedOperationException()
+    override fun add(element: String): Boolean =
+        throw UnsupportedOperationException("CaseInsensitiveMap key set is read-only; add '$element' not permitted")

Also applies to: 332-333, 375-376

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 289 -
290, Replace bare exception constructors with descriptive messages: in the
iterator remove() implementations for KeySet, ValueCollection, and EntrySet (the
methods named remove() that throw IllegalStateException()), include a clear
message that identifies the failed operation and the problematic key/value (use
lastKey/lastValue or equivalent local variable in the message). Similarly,
update the add() override methods that throw UnsupportedOperationException() to
provide a message stating the collection is immutable/unsupported and include
the attempted element (e.g., the parameter element) for context; keep using the
same exception types but pass a descriptive string to the constructor.

---

168-180: equals() only accepts CaseInsensitiveMap — violates the Map.equals symmetry contract.

other !is CaseInsensitiveMap<*> returns false for any regular Map, creating an asymmetry: a HashMap with identical entries can return true when compared against this map, while this map returns false against the HashMap.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 168 -
180, The equals implementation in CaseInsensitiveMap currently only accepts
CaseInsensitiveMap and breaks Map.equals symmetry; update
CaseInsensitiveMap.equals to accept any Map<*,*>: if other is this return true,
if other !is Map<*,*> return false, then if sizes differ return false, normalize
keys for comparison (use the same case-insensitive normalization this map uses)
and compare values for each key; reference keyStorage, valueStorage and _size to
iterate this map’s entries and compare against a normalized view of other (e.g.,
build a temporary map of other’s keys lowered/normalized when keys are String)
so lookups honor case-insensitivity and preserve Map.equals contract.

ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt (1)

413-419: caseInsensitiveHashCode is duplicated verbatim in StringValues.kt (lines 327–332).

Both companion objects contain an identical private implementation. Extracting it to a shared internal top-level function (e.g., in a new CaseInsensitiveUtils.kt in the same package) would eliminate the duplication and ensure any future fix (e.g., handling of Unicode case folding) applies consistently.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 413 -
419, The private function caseInsensitiveHashCode duplicated in
CaseInsensitiveMap.kt and StringValues.kt should be extracted to a single
internal top-level helper (e.g., create CaseInsensitiveUtils.kt in the same
package with an internal fun caseInsensitiveHashCode(s: String): Int) and both
companion objects should call that shared function; remove the duplicate private
implementations in CaseInsensitiveMap.Companion and StringValues.Companion so
the hash logic is centralized and future Unicode/case-folding fixes apply
consistently.

ktor-utils/common/src/io/ktor/util/StringValues.kt (1)

215-216: Unchecked casts on lines 215–216 are missing @Suppress("UNCHECKED_CAST").

arrayOfNulls<String>(entryCount) as Array<String> and arrayOfNulls<List<String>>(entryCount) as Array<List<String>> produce compiler warnings. Every similar cast in CaseInsensitiveMap.kt is annotated consistently; these two should be as well to keep linting clean.

🧹 Suggested fix

+            `@Suppress`("UNCHECKED_CAST")
             keyStorage = arrayOfNulls<String>(entryCount) as Array<String>
+            `@Suppress`("UNCHECKED_CAST")
             valueStorage = arrayOfNulls<List<String>>(entryCount) as Array<List<String>>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/StringValues.kt` around lines 215 - 216,
Add the missing suppression for the unchecked casts used to initialize
keyStorage and valueStorage: annotate the cast expressions (or the containing
property) with `@Suppress`("UNCHECKED_CAST") so the lines using
arrayOfNulls<String>(entryCount) as Array<String> and
arrayOfNulls<List<String>>(entryCount) as Array<List<String>> no longer emit
warnings, matching the pattern used in CaseInsensitiveMap.kt; ensure the
suppression targets the same scope (the keyStorage/valueStorage initializations)
and leaves the cast logic unchanged.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

ktor-utils/common/src/io/ktor/util/StringValues.kt (3)

227-227: Use value.toList() instead of List(value.size) { value[it] } for idiomatic defensive copy.

Both produce the same result (an independent ArrayList), but value.toList() is the idiomatic Kotlin idiom and avoids indexed access on a List<String> interface (which is O(n) for some implementations).

♻️ Proposed change

-                valueStorage[i] = List(value.size) { value[it] }
+                valueStorage[i] = value.toList()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/StringValues.kt` at line 227, Replace the
defensive copy creation in StringValues.kt that currently uses List(value.size)
{ value[it] } with the idiomatic value.toList() to avoid O(n) indexed access on
generic List implementations; locate the assignment to valueStorage[i] in the
StringValues logic and change the copy expression so it calls value.toList()
(preserving the independent list semantics).

---

180-181: Constructor parameter values silently shadows the protected val values property in init.

Both values: Map<String, List<String>> (constructor parameter) and protected val values (computed property) share the same unqualified name. In Kotlin, primary-constructor parameters are in scope throughout init blocks and, as the more-local binding, shadow the class property — so values.size on line 209 resolves to the constructor parameter, not the getter. The code is correct, but the shadowing is a readability trap: a future reader or maintainer may mistake values inside init for a this.values call (which would invoke the getter with entryCount still at its default 0, silently constructing an always-empty instance).

Consider renaming the constructor parameter to initialValues to make the scoping unambiguous.

♻️ Proposed rename

 public open class StringValuesImpl(
     final override val caseInsensitiveName: Boolean = false,
-    values: Map<String, List<String>> = emptyMap()
+    initialValues: Map<String, List<String>> = emptyMap()
 ) : StringValues {
     ...
     init {
-        entryCount = values.size
+        entryCount = initialValues.size
         if (entryCount == 0) {
             ...
         } else {
             ...
-            for ((key, value) in values) {
+            for ((key, value) in initialValues) {

Also applies to: 208-209

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/StringValues.kt` around lines 180 - 181,
The constructor parameter named values shadows the protected val values property
inside the init block (so expressions like values.size refer to the parameter,
not the property); rename the constructor parameter (e.g., to initialValues) and
update all usages inside the init and constructor signature to use that new
name, leaving the protected val values getter untouched (also update any
references to entryCount initialization that rely on the property) so there is
no ambiguity between the parameter and the computed property.

---

328-334: caseInsensitiveHashCode is duplicated verbatim from CaseInsensitiveMap.kt.

The same function body (identical algorithm, identical variable names) exists in the private companion object of CaseInsensitiveMap.kt (lines 426–432 per the relevant snippet). Extracting it to a shared internal utility avoids the two implementations diverging silently.

♻️ Suggested extraction

Create an internal top-level function (e.g., in ktor-utils/common/src/io/ktor/util/Strings.kt or a new CaseInsensitiveHashing.kt):

internal fun caseInsensitiveHashCode(s: String): Int {
    var h = 0
    for (i in 0 until s.length) {
        h = 31 * h + s[i].lowercaseChar().code
    }
    return h
}

Then remove the private copies from both StringValues.kt and CaseInsensitiveMap.kt companion objects and call the shared function directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/StringValues.kt` around lines 328 - 334,
Duplicate private function caseInsensitiveHashCode exists in StringValues.kt and
the companion object of CaseInsensitiveMap; extract it into a single internal
top-level function (e.g., internal fun caseInsensitiveHashCode(s: String): Int)
placed in a new utility source file, then remove the private copies from both
StringValues's companion and CaseInsensitiveMap's companion and update their
callers to call the shared caseInsensitiveHashCode function; keep the
implementation identical to preserve behavior and mark the new function internal
so it’s usable across the module.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

ktor-utils/common/src/io/ktor/util/StringValues.kt (1)

280-287: ⚠️ Potential issue | 🟠 Major

names() still returns a case-sensitive set, breaking case-insensitive lookup semantics.

The current implementation uses linkedSetOf<String>() regardless of caseInsensitiveName. This causes names().contains("content-type") to return false when the stored key is "Content-Type", even though the instance is case-insensitive.

This is inconsistent with the values getter (line 200-201), which correctly uses caseInsensitiveMap() conditionally.

Proposed fix to match the values getter pattern

 override fun names(): Set<String> {
     if (entryCount == 0) return emptySet()
-    val result = linkedSetOf<String>()
+    val result: MutableMap<String, Unit> =
+        if (caseInsensitiveName) caseInsensitiveMap() else LinkedHashMap(entryCount)
     for (i in 0 until entryCount) {
-        result.add(keyStorage[i])
+        result[keyStorage[i]] = Unit
     }
-    return result
+    return result.keys
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/StringValues.kt` around lines 280 - 287,
The names() implementation returns a case-sensitive linkedSet regardless of
caseInsensitiveName, breaking case-insensitive semantics; update names() (which
reads entryCount and keyStorage) to create the result set using the same
conditional used by the values getter—i.e., when caseInsensitiveName is true,
construct a case-insensitive set (using the project's case-insensitive
collection factory used elsewhere, e.g., caseInsensitiveMap()/set variant) and
otherwise use linkedSetOf<String>(), then add keyStorage[i] into that set and
return it.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt (1)

174-186: ⚠️ Potential issue | 🟠 Major

equals is constrained to CaseInsensitiveMap only, which is surprising for a MutableMap implementation.

This blocks normal cross-Map equality expectations and should either be aligned with Map<*, *> comparison semantics or explicitly documented as an intentional divergence in public KDoc.

Kotlin Map.equals contract for custom MutableMap implementations: should equality be based on mapping equivalence across different Map implementations?

Based on learnings: Applies to **/*.kt : All public/protected API changes must be intentional and well-documented.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 174 -
186, The equals implementation in CaseInsensitiveMap is incorrectly constrained
to CaseInsensitiveMap and should follow Map<*,*> semantics: change equals(other:
Any?) in class CaseInsensitiveMap to accept any Map implementation, return true
when mappings are equivalent regardless of concrete type, and preserve
referential equality and size checks; specifically, if other is a Map<*,*> check
size == _size and then for each non-null key in keyStorage compare
valueStorage[i] to other[normalizedKey] (handling nulls correctly) or
alternately iterate other.entries and ensure this[key] == entry.value for all
entries; update logic to handle keys absent vs present-with-null values and keep
early-return for inequality. Reference: equals, CaseInsensitiveMap, keyStorage,
valueStorage, _size.

ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt (1)

419-424: Consider extracting the case-insensitive hash function to a shared internal utility.

CaseInsensitiveMap.kt and StringValues.kt currently carry identical logic; centralizing avoids drift between two hash-based data structures that are meant to stay compatible.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-utils/common/src/io/ktor/util/CaseInsensitiveMap.kt` around lines 419 -
424, Extract the duplicate case-insensitive hashing logic into a single internal
utility function (e.g., internal fun caseInsensitiveHashCode(s: String): Int)
and remove the copies from CaseInsensitiveMap.kt and StringValues.kt; update
both CaseInsensitiveMap (where caseInsensitiveHashCode was used) and the
StringValues implementation to call the new shared utility so both data
structures remain bit‑for‑bit compatible, keep the function internal/visible to
the module, preserve the current algorithm (lowercaseChar()/code and 31
multiplier) and run/update any relevant tests that depend on hashing behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes