Skip to content

refactor: replace url-parse to WHATWG URL API#2244

Open
hyperz111 wants to merge 44 commits into
Acode-Foundation:mainfrom
hyperz111:url-parse+to+whatwg-url
Open

refactor: replace url-parse to WHATWG URL API#2244
hyperz111 wants to merge 44 commits into
Acode-Foundation:mainfrom
hyperz111:url-parse+to+whatwg-url

Conversation

@hyperz111

Copy link
Copy Markdown
Contributor

In url-parse readme:

...The URL interface is available in all supported Node.js release lines and basically all browsers. Consider using it for better security and accuracy.

So I think if We can do this for now :)

hyperz111 and others added 30 commits August 18, 2025 10:33
@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the third-party url-parse library with a thin URLParse shim (src/utils/urlParse.js) that wraps the native WHATWG URL constructor and catches TypeError to return safe empty-string defaults, then updates all three call sites to use it.

  • The new shim correctly handles malformed/missing-scheme URLs that previously relied on url-parse's lenient parsing, resolving the unguarded new URL() risks flagged in prior reviews.
  • However, all three call sites (remoteStorage.js, Url.js, fileBrowser.js) still call decodeURIComponent() on values returned by searchParams.get() or Object.fromEntries(searchParams), both of which already return percent-decoded strings per the WHATWG spec — a double-decode that throws URIError: URI malformed whenever a stored path or passphrase contains a literal % after the first decode.

Confidence Score: 3/5

The migration to the WHATWG URL API is structurally sound, but the double-decode pattern left in all three call sites will crash edit/delete flows for any user whose key-file path or passphrase contains a % character after the initial decode.

The URLParse shim and the removal of url-parse are clean. The issue is that URLSearchParams.get() and Object.fromEntries(searchParams) both return already-decoded strings, so the surviving decodeURIComponent() calls on those values are a double-decode present in all three changed files. For most values this is a no-op, but for any key-file path or passphrase that was stored with a percent-encoded % (i.e. %25), the first decode produces a bare % and the second call throws URIError: URI malformed, breaking the edit flow in remoteStorage.js, the SFTP decode path in Url.js, and the key-file cleanup in fileBrowser.js.

src/lib/remoteStorage.js, src/utils/Url.js, and src/pages/fileBrowser/fileBrowser.js all retain redundant decodeURIComponent calls on searchParams-sourced values that should be removed.

Important Files Changed

Filename Overview
src/utils/urlParse.js New shim wrapping new URL() with a try/catch fallback that returns safe empty-string defaults, preventing unhandled TypeError for malformed or missing-scheme URLs throughout the codebase.
src/lib/remoteStorage.js Switched from url-parse to the new URLParse shim; searchParams.get() already returns decoded values but decodeURIComponent() is still called on them, causing double-decode that throws URIError for paths containing % characters.
src/utils/Url.js Updated decodeUrl to use the new shim; Object.fromEntries(searchParams) produces already-decoded values but decodeURIComponent is still applied to keyFile and passPhrase, introducing the same double-decode risk.
src/pages/fileBrowser/fileBrowser.js Switched to the new shim for key-file deletion path; still wraps searchParams.get("keyFile") with decodeURIComponent, causing a double-decode that can throw URIError and silently skip deleting the key file.
package.json Removes the url-parse dependency as expected for the migration to the WHATWG URL API.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Input URL string] --> B{URLParse shim}
    B -->|valid URL| C[new URL - WHATWG]
    B -->|throws TypeError| D[Fallback object\nempty strings + empty searchParams]
    C --> E[URL object\nusername/password: percent-encoded\nsearchParams.get: already decoded]
    D --> E
    E --> F{Field type}
    F -->|username / password| G[decodeURIComponent OK\nneeded - URL spec keeps these encoded]
    F -->|searchParams.get values\nsecurity, mode, keyFile, passPhrase| H[decodeURIComponent WRONG\nalready decoded - double-decode\nthrows URIError if value contains bare %]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Input URL string] --> B{URLParse shim}
    B -->|valid URL| C[new URL - WHATWG]
    B -->|throws TypeError| D[Fallback object\nempty strings + empty searchParams]
    C --> E[URL object\nusername/password: percent-encoded\nsearchParams.get: already decoded]
    D --> E
    E --> F{Field type}
    F -->|username / password| G[decodeURIComponent OK\nneeded - URL spec keeps these encoded]
    F -->|searchParams.get values\nsecurity, mode, keyFile, passPhrase| H[decodeURIComponent WRONG\nalready decoded - double-decode\nthrows URIError if value contains bare %]
Loading

Comments Outside Diff (4)

  1. src/lib/remoteStorage.js, line 380-389 (link)

    The WHATWG URLSearchParams.get() already returns percent-decoded strings. Calling decodeURIComponent() on top of it is a double-decode. If a stored value (e.g. a passphrase or keyfile path) was originally percent-encoded in the URL and resolves to a string containing a bare % (for example %done), the second decodeURIComponent call throws URIError: URI malformed, crashing the edit flow. Remove the redundant decodeURIComponent wraps on searchParams.get() results; the ones on username and password are correct because URL.username/URL.password remain percent-encoded.

  2. src/lib/remoteStorage.js, line 402-411 (link)

    searchParams.get("passPhrase") and searchParams.get("keyFile") already return decoded strings per the WHATWG spec. The subsequent decodeURIComponent calls are redundant and will throw URIError: URI malformed if a key-file path or passphrase (after initial decode) contains a literal % character.

  3. src/utils/Url.js, line 310-319 (link)

    Object.fromEntries(searchParams) iterates decoded key-value pairs (same as searchParams.get()), so keyFile and passPhrase are already decoded here. Passing them through decodeURIComponent again is a double-decode that throws URIError: URI malformed for any key-file path that contains a literal % after the initial decode (e.g. a path that was stored as %2Fhome%2Fuser%2F50%25done).

  4. src/pages/fileBrowser/fileBrowser.js, line 1159-1172 (link)

    parsedUrl.searchParams.get("keyFile") already returns a percent-decoded string. Wrapping it with decodeURIComponent is a double-decode; if the resolved path contains a literal % after decoding (e.g. a path encoded as %2Fhome%2Fuser%2F50%25done), this call throws URIError: URI malformed, preventing the key-file from being deleted when a storage entry is removed.

Reviews (3): Last reviewed commit: "chore: use URLParse" | Re-trigger Greptile

@hyperz111

Copy link
Copy Markdown
Contributor Author

WHATWG URL will throws on invalid input but url-parse not, maybe We should add exception handler?

@RohitKushvaha01

Copy link
Copy Markdown
Member

WHATWG URL will throws on invalid input but url-parse not, maybe We should add exception handler?

Yes

@hyperz111

Copy link
Copy Markdown
Contributor Author

@greptile review again

@hyperz111

Copy link
Copy Markdown
Contributor Author

@greptile review again

@hyperz111

Copy link
Copy Markdown
Contributor Author

@RohitKushvaha01 what about the double-decode problem? Because the query values on original url-parse return value are also decoded.

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

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants