Skip to content

chore(http): no extended url encoding - #824

Merged
basmasking merged 2 commits into
mainfrom
823-disable-extended-url-encoding
Sep 29, 2026
Merged

basmasking merged 2 commits into
mainfrom
823-disable-extended-url-encoding

Conversation

@petermasking

Copy link
Copy Markdown
Member

Fixes #823

@MaskingTechnology/jitar

@petermasking petermasking linked an issue Sep 29, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: e94ff854-89e4-4107-bfba-cfbe34074b8e

📥 Commits

Reviewing files that changed from the base of the PR and between e2a21d7 and a901764.

📒 Files selected for processing (2)
  • packages/http/src/HttpServer.ts
  • packages/runtime/src/server/Server.ts

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


Summary by CodeRabbit

  • Updates
    • URL-encoded request bodies are now parsed using non-extended parsing, which may affect how nested form fields are handled.
    • The server’s file-provisioning success message no longer includes an extra quotation mark.

Walkthrough

The URL-encoded body parser now uses extended: false. The successful file-provisioning log message no longer includes a stray quote after the colon.

Changes

URL-encoded Body Parsing

Layer / File(s) Summary
Parser option
packages/http/src/HttpServer.ts
The URL-encoded body parser now uses extended: false instead of true.

File-Provisioning Log

Layer / File(s) Summary
Success log text
packages/runtime/src/server/Server.ts
The successful file-provisioning log message no longer includes a stray quote after the colon. File retrieval, response construction, and error handling are unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a9017

The parser change reflects the requested simple-form behavior, and the inspected worker caller uses JSON. The provisioning edit changes only log wording; no concrete merge-blocking behavior risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a9017

The change affects how form submissions are interpreted, but the routes and downstream checks remain in place. Compatibility for external clients that send nested form fields is not established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The changed parser is global to this HTTP server and precedes its RPC and worker routes; JSON parsing remains a separate preceding middleware.

Trust Boundaries and Controls

  • observed — The route table is unchanged, worker handlers retain input validation, and Server.run still passes external RPC requests through MiddlewareManager. The behavior of any argument-dependent middleware policy was not established.

Hardening Proposals

  • proposed — Confirm the supported URL-encoded field grammar for external RPC and worker clients, and explicitly reject or migrate nested form encodings if they were previously supported.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description links issue #823 and includes the repository mention, but it omits the required “Changes proposed in this pull request” section and its change summary. Add the required “Changes proposed in this pull request” section. State that Express URL-encoded parsing now uses extended: false and that the unrelated log message was corrected.
Out of Scope Changes check ⚠️ Warning The change in packages/runtime/src/server/Server.ts only changes the success log text from Provided file: ' to Provided file:. It does not support disabling extended URL encoding and is unrelate… Remove the unrelated log message change from this PR, or move it to a separate PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: disabling extended URL encoding in the HTTP server.
Linked Issues check ✅ Passed Issue #823 requires extended URL encoding to be disabled. The PR changes packages/http/src/HttpServer.ts from Express extended: true to extended: false. This implements the linked coding objecti…
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 2…
Full details: Out of Scope Changes check

Explanation

The change in packages/runtime/src/server/Server.ts only changes the success log text from Provided file: ' to Provided file:. It does not support disabling extended URL encoding and is unrelated to issue #823.

  • Fix all pre-merge checks with AI

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

A rabbit checks the parser's setting,
Then spots a quote that needs resetting.
The log reads clean, the bodies parse,
I hop along through fields of grass.
A tidy patch, then home I bound!

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

@sonarqubecloud

Copy link
Copy Markdown

@basmasking
basmasking merged commit 918d31f into main Sep 29, 2026
24 checks passed
@basmasking
basmasking deleted the 823-disable-extended-url-encoding branch September 29, 2026 07:53
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.

Disable extended URL encoding

2 participants