Skip to content

KTOR-9168 Add regression test for case-insensitive Set-Cookie merging in CIO#5579

Open
fru1tworld wants to merge 1 commit intoktorio:mainfrom
fru1tworld:fix/9168-cio-cookie-case
Open

KTOR-9168 Add regression test for case-insensitive Set-Cookie merging in CIO#5579
fru1tworld wants to merge 1 commit intoktorio:mainfrom
fru1tworld:fix/9168-cio-cookie-case

Conversation

@fru1tworld
Copy link
Copy Markdown
Contributor

Subsystem

Client / CIO

Motivation

KTOR-9168 (CIO dropping case-different Set-Cookie headers) was inadvertently fixed by #5379. Add a wire-level regression test to guard the case-insensitive dedup behavior.

Solution

Test only — no production code changes.

Closes KTOR-9168

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a4d36725-ab1f-42ef-96e4-41f51d47521f

📥 Commits

Reviewing files that changed from the base of the PR and between a1651e3 and acd6ff1.

📒 Files selected for processing (1)
  • ktor-client/ktor-client-cio/jvm/test/io/ktor/client/engine/cio/CookieHeaderCaseTest.kt

📝 Walkthrough

Walkthrough

A new test file is added to verify that the CIO HTTP client engine correctly preserves Set-Cookie response headers regardless of casing in the header name (e.g., "Set-Cookie" vs "set-cookie").

Changes

Set-Cookie Header Case Handling Test

Layer / File(s) Summary
Test Implementation
ktor-client/ktor-client-cio/jvm/test/io/ktor/client/engine/cio/CookieHeaderCaseTest.kt
New test class that spins up a mock HTTP server using ServerSocket, responds with two Set-Cookie headers using different casing, and asserts the CIO client correctly collects both cookies via response.headers.getAll(HttpHeaders.SetCookie).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • osipxd
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding a regression test for case-insensitive Set-Cookie header handling in CIO.
Description check ✅ Passed The description includes all required template sections (Subsystem, Motivation, Solution) with clear explanations of the bug, the fix, and the test addition.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@bjhham bjhham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants