Skip to content

Commit

Permalink
url: fix url spec compliance issues
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Lemire <[email protected]>
PR-URL: nodejs#46547
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
anonrig and lemire committed Apr 5, 2023
1 parent 11fceac commit 1645bf6
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 2 deletions.
13 changes: 13 additions & 0 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,19 @@ function update(url, params) {
ctx.search = '?' + serializedParams;
} else {
ctx.search = '';

// Potentially strip trailing spaces from an opaque path
if (ctx.hasOpaquePath && ctx.hash.length === 0) {
let length = ctx.pathname.length;
while (length > 0 && ctx.pathname.charCodeAt(length - 1) === 32) {
length--;
}

// No need to copy the whole string if there is no space at the end
if (length !== ctx.pathname.length) {
ctx.pathname = ctx.pathname.slice(0, length);
}
}
}
ctx.href = constructHref(ctx);
}
Expand Down
8 changes: 7 additions & 1 deletion test/wpt/status/url.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
"skip": "TODO: port from .window.js"
},
"historical.any.js": {
"requires": ["small-icu"]
"requires": ["small-icu"],
"fail": {
"expected": [
"URL: no structured serialize/deserialize support",
"URLSearchParams: no structured serialize/deserialize support"
]
}
},
"urlencoded-parser.any.js": {
"requires": ["small-icu"]
Expand Down
2 changes: 1 addition & 1 deletion test/wpt/test-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ runner.setScriptModifier((obj) => {
});
runner.pretendGlobalThisAs('Window');
runner.setInitScript(`
globalThis.location = {};
globalThis.location ||= {};
`);
runner.runJsTests();

0 comments on commit 1645bf6

Please sign in to comment.