Skip to content

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

Open
hyperz111 wants to merge 41 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 41 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 removes the third-party url-parse package and migrates all URL parsing in Url.js, remoteStorage.js, and fileBrowser.js to the native WHATWG URL API (new URL() + URLSearchParams).

  • Url.hidePassword and Url.decodeUrl now use new URL() directly, with no try/catch. Unlike url-parse, the native constructor throws TypeError for any invalid or malformed input, creating a regression path if a stored URL is ever malformed.
  • remoteStorage.edit() applies the same pattern without error handling; fileBrowser.js is guarded by an if (storage.url) null check but would still throw on a non-empty invalid string.
  • Dependency cleanup is clean: url-parse, querystringify, and requires-port are all removed from package.json and the lockfile.

Confidence Score: 3/5

The refactor is conceptually sound, but the switch from a lenient URL parser to a strict-throwing one introduces unhandled exception paths in Url.hidePassword, Url.decodeUrl, and remoteStorage.edit() that could crash the app when encountering any stored URL that does not satisfy the WHATWG parser.

Two call sites — Sftp.fromUrl and Ftp.fromUrl — call Url.decodeUrl with no surrounding error handling, and remoteStorage.edit() similarly has none. A single malformed URL in localStorage.storageList would now throw an unhandled TypeError instead of silently returning empty fields as before. Adding try/catch guards around the new URL() calls in those three locations would make this safe to merge.

src/utils/Url.js (hidePassword and decodeUrl) and src/lib/remoteStorage.js (edit) need error handling around the new URL() constructor calls.

Important Files Changed

Filename Overview
src/utils/Url.js Replaces url-parse with new URL() in hidePassword and decodeUrl; the new API throws on invalid input with no error handling, and a potential double-decode on query values.
src/lib/remoteStorage.js Replaces URLParse(url, true) with new URL(url) + searchParams.get(); no error handling around the URL constructor in edit().
src/pages/fileBrowser/fileBrowser.js Uses new URL(storage.url) guarded by if (storage.url) check; handles the keyFile param correctly via searchParams.get().
package.json Removes url-parse dependency; clean change.
package-lock.json Removes url-parse, querystringify, and requires-port from the lockfile; consistent with package.json.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Stored URL string] --> B{Valid URL?}
    B -- "Yes (url-parse & new URL)" --> C[Parse username / password / hostname / port / searchParams]
    B -- "No - url-parse" --> D[Return empty-field object\n– graceful degradation]
    B -- "No - new URL()" --> E[throws TypeError\n– unhandled exception ⚠️]
    C --> F[decodeURIComponent on each field]
    F --> G[Return parsed object to caller\nSftp.fromUrl / Ftp.fromUrl / edit / fileBrowser]
Loading

Comments Outside Diff (3)

  1. src/utils/Url.js, line 269-276 (link)

    new URL() throws on invalid URLs — no error guard

    new URL(url) throws a TypeError if url is empty, null, undefined, or any malformed string. url-parse was lenient and returned an object with empty defaults in the same cases. Neither hidePassword nor decodeUrl wraps the call in a try/catch, so any caller passing a bad URL (e.g., a legacy entry in localStorage.storageList) will now produce an unhandled exception instead of silently degrading. Sftp.fromUrl (fileSystem/sftp.js:591) and Ftp.fromUrl (fileSystem/ftp.js:360) call decodeUrl with no error handling of their own either.

  2. src/lib/remoteStorage.js, line 368-369 (link)

    new URL(url) in edit() throws for any invalid stored URL

    Same issue as in Url.js — if the url value retrieved from the storage list is not a well-formed URL string, new URL(url) will throw an unhandled TypeError. The old URLParse(url, true) call would have returned empty-string fields and continued. Consider wrapping in a try/catch or validating url before constructing the URL object.

  3. src/utils/Url.js, line 308-317 (link)

    Potential double-decode on keyFile / passPhrase

    URLSearchParams automatically percent-decodes values when iterating, so Object.fromEntries(searchParams) already returns decoded strings. Calling decodeURIComponent() on a pre-decoded value is normally idempotent, but if the decoded value happens to contain a bare % (e.g., a path like /50%done), the second decodeURIComponent will throw a URIError: URI malformed. The same pattern existed with querystringify before, so this is a pre-existing edge case — but it is worth confirming whether key-file paths or passphrases could ever contain literal % after the initial decode.

Reviews (1): Last reviewed commit: "chore: remove url-parse" | 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?

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.

1 participant