Skip to content

#5033 Fix scoped Sessions plugin not sending Set-Cookie without call.respond()#5511

Merged
zibet27 merged 2 commits intoktorio:release/3.xfrom
fru1tworld:fix-5033-scoped-session-cookie
Apr 8, 2026
Merged

#5033 Fix scoped Sessions plugin not sending Set-Cookie without call.respond()#5511
zibet27 merged 2 commits intoktorio:release/3.xfrom
fru1tworld:fix-5033-scoped-session-cookie

Conversation

@fru1tworld
Copy link
Copy Markdown
Contributor

@fru1tworld fru1tworld commented Apr 7, 2026

Subsystem
Server, ktor-server-core (routing)

Motivation
Fixes #5033. Scoped Sessions plugin doesn't send Set-Cookie when handler sets status without call.respond().

Solution
Call respond() through RoutingPipelineCall when handler set status but didn't respond, so scoped sendPipeline interceptors fire.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 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: e6c8065d-3b64-4c57-a240-6d551929b72a

📥 Commits

Reviewing files that changed from the base of the PR and between 049538f and 1b74906.

📒 Files selected for processing (2)
  • ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt
  • ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/sessions/SessionTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt
  • ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/sessions/SessionTest.kt

📝 Walkthrough

Walkthrough

After pipeline execution, routing now responds with the current response status if the call remains unhandled; added a test verifying that setting a session and only a response status emits a Set-Cookie header.

Changes

Cohort / File(s) Summary
Routing execution logic
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt
After routingCallPipeline.execute(...) completes successfully, if the call is still unhandled, check response.status() and invoke respond(status) to send that status.
Session tests
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/sessions/SessionTest.kt
Added test scoped session sends Set-Cookie header without explicit respond that sets a session and only response.status(...), asserting Set-Cookie is emitted and status is OK.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PR #4938: Also modifies RoutingRoot.executeResult (changes around response/exception handling), touching the same function and closely related.
🚥 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 references issue #5033 and describes the fix for scoped Sessions plugin not sending Set-Cookie without call.respond().
Description check ✅ Passed The description follows the template with all required sections (Subsystem, Motivation, Solution) completed with relevant details about the issue and fix.
Linked Issues check ✅ Passed The code changes directly address issue #5033 by ensuring respond() is called when a handler sets status without responding, enabling scoped sendPipeline interceptors to fire.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the scoped Sessions plugin issue: routing logic modification and a test validating the fix with no unrelated changes.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix-5033-scoped-session-cookie

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.

@fru1tworld fru1tworld marked this pull request as ready for review April 7, 2026 05:37
@bjhham bjhham requested a review from zibet27 April 7, 2026 07:31
Copy link
Copy Markdown
Collaborator

@zibet27 zibet27 left a comment

Choose a reason for hiding this comment

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

Nice!

application.monitor.raise(RoutingCallStarted, routingCall)
try {
routingCallPipeline.execute(routingApplicationCall)
if (!routingApplicationCall.isHandled && routingApplicationCall.response.status() != null) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a nitpick
I would use let instead of !! operator

if (!routingApplicationCall.isHandled) {
   routingApplicationCall.response.status()?.let { status ->
      routingApplicationCall.respond(status)
   }
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, updated. Thanks for the review!

@fru1tworld fru1tworld requested a review from zibet27 April 7, 2026 12:13
Copy link
Copy Markdown
Collaborator

@zibet27 zibet27 left a comment

Choose a reason for hiding this comment

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

LGTM
@fru1tworld Thank you for the fix!

PS: I'll rebase the branch to release/3.x, so it can be included in the next patch release. Or you can do it by running the switch-base-branch.sh script in the project root

@fru1tworld fru1tworld force-pushed the fix-5033-scoped-session-cookie branch from 049538f to 1b74906 Compare April 7, 2026 23:52
@fru1tworld fru1tworld changed the base branch from main to release/3.x April 7, 2026 23:52
@fru1tworld
Copy link
Copy Markdown
Contributor Author

Rebased onto release/3.x using cherry-pick. Thanks for the review and the suggestion!

@zibet27 zibet27 merged commit c9fcd0f into ktorio:release/3.x Apr 8, 2026
15 of 16 checks passed
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.

Unable to update/remove session data if no response content

2 participants