Skip to content

[fix] Percent-decode the userinfo of the URL - #2352

Merged
lpinca merged 1 commit into
websockets:masterfrom
simonkundrik:fix/decode-url-userinfo
Nov 5, 2026
Merged

lpinca merged 1 commit into
websockets:masterfrom
simonkundrik:fix/decode-url-userinfo

Conversation

@simonkundrik

Copy link
Copy Markdown
Contributor

The URL parser keeps the username and password percent-encoded, and ws used them as they are to build the Authorization header. A @, / or # in a password can only be written percent-encoded in a URL, so new WebSocket('ws://user:p%40ss@host') sends the password p%40ss and authentication fails. Writing ws://user:p@ss@host does the same, because the parser encodes the second @ itself. The same goes for :, spaces and non-ASCII characters, which the parser also encodes.

http.request() decodes the userinfo of a URL (url.urlToHttpOptions() does it too), and so did the old url.parse() code path, so this brings ws in line.

I used querystring.unescape() instead of decodeURIComponent() because it leaves malformed sequences as they are. A password such as 100% is sent as is today, and decodeURIComponent() would throw on it, which with a redirect would happen inside the 'response' handler.

I added tests for encoded characters, the unencoded @, multi-byte characters and malformed sequences. They fail on master. npm test passes and eslint and prettier are clean.

I also compared the header ws builds with the one http.request() builds for 50,000 random userinfos made of escapes, reserved characters and non-ASCII characters. Of the 26,992 that Node accepts, 25,703 differ on master and none differ now. For the rest Node throws on a malformed sequence and ws does not.

The URL parser keeps the username and password percent-encoded, and they
were used as they are to build the `Authorization` header, so
`ws://user:p%40ss@host` sent the password `p%40ss`.

Decode them with `querystring.unescape()`, like `http.request()` does.
Unlike `decodeURIComponent()` it leaves malformed sequences as they are,
so a password such as `100%` keeps working.
@lpinca

lpinca commented Nov 5, 2026

Copy link
Copy Markdown
Member

This is in the same vein as #2346, but aligning with Node.js behavior is a positive change.

@lpinca
lpinca merged commit e23d55e into websockets:master Nov 5, 2026
62 checks passed
@lpinca

lpinca commented Nov 5, 2026

Copy link
Copy Markdown
Member

Thank you.

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