Skip to content

fix: re-encode WebSocket proxy query params so spaces use %20 (#9041)#9064

Merged
mscolnick merged 2 commits into
mainfrom
ms/fix/lsp-spaces-in-filename
Apr 6, 2026
Merged

fix: re-encode WebSocket proxy query params so spaces use %20 (#9041)#9064
mscolnick merged 2 commits into
mainfrom
ms/fix/lsp-spaces-in-filename

Conversation

@mscolnick

Copy link
Copy Markdown
Contributor

Filenames with spaces failed LSP connections because Starlette decodes query-string spaces as '+' (form-urlencoded) while python-lsp-server expects percent-encoding (%20). Apply unquote_plus → quote on forwarded query params to normalize the encoding.

Filenames with spaces failed LSP connections because Starlette decodes
query-string spaces as '+' (form-urlencoded) while python-lsp-server
expects percent-encoding (%20). Apply unquote_plus → quote on forwarded
query params to normalize the encoding.
Copilot AI review requested due to automatic review settings April 6, 2026 16:22
@vercel

vercel Bot commented Apr 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Apr 6, 2026 4:35pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes WebSocket proxying for LSP connections by normalizing forwarded query parameter encoding so that spaces are percent-encoded (%20) instead of being represented in a form-encoding style that can break upstream servers like python-lsp-server.

Changes:

  • Re-encode proxied WebSocket query param values via quote(...) before connecting upstream.
  • Add a focused test to validate space normalization behavior for proxied WebSocket query params.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
marimo/_server/api/middleware.py Re-encodes WebSocket query param values when building the upstream ws_url.
tests/_server/api/test_middleware.py Adds a unit test covering +%20 normalization and related cases.

Comment thread marimo/_server/api/middleware.py Outdated
Comment thread tests/_server/api/test_middleware.py
…al plus signs

unquote_plus is redundant since Starlette already decodes query params,
and it corrupts values containing literal '+' (from %2B) by turning them
into spaces. Using just quote(v) correctly re-encodes already-decoded
values.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread marimo/_server/api/middleware.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread tests/_server/api/test_middleware.py
@mscolnick mscolnick requested a review from dmadisetti April 6, 2026 18:56
@mscolnick mscolnick enabled auto-merge (squash) April 6, 2026 19:54
@mscolnick mscolnick merged commit 0579862 into main Apr 6, 2026
47 checks passed
@mscolnick mscolnick deleted the ms/fix/lsp-spaces-in-filename branch April 6, 2026 21:07
@github-actions

github-actions Bot commented Apr 6, 2026

Copy link
Copy Markdown

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.22.5-dev15

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants