Skip to content

fix: preserve booleans when assigning query parameters - #2013

Merged
yowainwright merged 4 commits into
koajs:masterfrom
tianrking:codex/query-boolean-serialization
Oct 21, 2026
Merged

yowainwright merged 4 commits into
koajs:masterfrom
tianrking:codex/query-boolean-serialization

Conversation

@tianrking

@tianrking tianrking commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Assigning ctx.query = { enabled: true, disabled: false } currently produces enabled=&disabled=. Preserve boolean values through the existing primitive-value guard, including repeated and mixed arrays. Parsed values remain strings, escaping is unchanged, and unsupported values still serialize as empty strings.

The regression tests now live in __tests__/lib/search-params.test.js, as requested. They cover scalar and array serialization, escaping, unsupported values, query cache rewrites, and middleware behavior.

Validation on the updated branch: all 469 tests pass; Standard lint and git diff --check pass. GitHub reports all eight checks passing.

Summary by Sourcery

Preserve boolean query parameter values during serialization while retaining existing handling for other value types.

Bug Fixes:

  • Preserve boolean values when serializing scalar and repeated query parameters instead of converting them to empty strings.

Tests:

  • Add regression coverage for boolean serialization in scalar and mixed array query parameters.

Summary by CodeRabbit

  • Bug Fixes

    • Boolean search parameters are now serialized correctly instead of being converted to empty values.
    • Mixed-value arrays preserve boolean values alongside numbers and empty strings.
  • Tests

    • Added coverage for boolean scalar parameters and mixed arrays.

@sourcery-ai

sourcery-ai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Reviewer's Guide

Preserves boolean values when query objects are stringified by allowing booleans through the existing primitive guard, while retaining string parsing, escaping, and unsupported-value behavior. Comprehensive regressions validate arrays, rewrites, and end-to-end GET/POST middleware behavior.

Sequence diagram for boolean query parameter serialization

sequenceDiagram
    participant Client
    participant Middleware
    participant SearchParams as search_params
    participant URLSearchParams
    Client->>Middleware: GET/POST request
    Middleware->>SearchParams: stringify(query)
    SearchParams->>URLSearchParams: append(key, boolean)
    URLSearchParams-->>SearchParams: escaped query string
    SearchParams-->>Middleware: enabled=true&disabled=false
    Middleware-->>Client: response with preserved query values
Loading

File-Level Changes

Change Details Files
Allow boolean primitives to serialize as their literal string values in query parameters.
  • Extend the existing primitive-value guard to include booleans.
  • Continue serializing unsupported values as empty strings and preserve URLSearchParams escaping behavior.
  • Leave parsed query values as strings.
lib/search-params.js
Add regression coverage for boolean serialization, query rewriting, and HTTP middleware behavior.
  • Cover scalar, repeated, mixed, escaped-key, and unsupported-value cases.
  • Verify parsed-query cache invalidation and URL/query property updates after rewrites.
  • Exercise GET and POST requests through real sockets and composed middleware.
__tests__/request/query-boolean.test.js

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 22fd769a-1720-434d-afd6-4cda21128712
📥 Commits

Reviewing files that changed from the base of the PR and between 090f4e1 and 486b842.

📒 Files selected for processing: 1
  • __tests__/lib/search-params.test.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

addKey now preserves boolean values during query serialization. New tests cover scalar and array values, unsupported types, escaped keys, successive query rewrites, and rewritten queries in GET and POST middleware. The tests also check parsed query values, URL fields, query-cache behavior, and preservation of originalUrl.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 486b8

The query serialization change has no identified material merge risk.

Security architecture risk: ⚪ Minimal · up to b7afc

The change restores boolean serialization without changing query escaping, parsed-value types, or request ownership. No material security risk was identified in the affected assignment and middleware path.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly affected exposure is the query representation of requests whose middleware assigns boolean values. Downstream middleware can observe the preserved strings; request wrappers and their query caches remain request-local.

Trust Boundaries and Controls

  • inferred — The inspected path changes value preservation, not query-assignment authority. Assigned values still cross into an encoded query string and return as parsed strings; no new authentication or authorization bypass was established by this transition.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving boolean values when assigning query parameters.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files.

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@codecov

codecov Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.90%. Comparing base (2fecd02) to head (486b842).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2013   +/-   ##
=======================================
  Coverage   99.90%   99.90%           
=======================================
  Files           9        9           
  Lines        2144     2144           
=======================================
  Hits         2142     2142           
  Misses          2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yowainwright yowainwright left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this catch!!!

Please write tests here:

💪

@yowainwright yowainwright left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@tianrking great work! Sorry I missed your update.
I reduced the test footprint.
Thank you 🥇

@yowainwright
yowainwright merged commit a82feec into koajs:master Oct 21, 2026
8 checks passed
@yowainwright

Copy link
Copy Markdown
Member

@jonathanong @fengmk2 , I think this merged pr is a decent place to publish 3.3.0.
Can you assist with this if you agree?
I can begin on 4.0.0 after. 🚀

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