Skip to content

fix: handle undefined pathname in URL builder#2021

Open
UnschooledGamer wants to merge 4 commits intoAcode-Foundation:mainfrom
UnschooledGamer:fix/url-no-doc-id
Open

fix: handle undefined pathname in URL builder#2021
UnschooledGamer wants to merge 4 commits intoAcode-Foundation:mainfrom
UnschooledGamer:fix/url-no-doc-id

Conversation

@UnschooledGamer
Copy link
Copy Markdown
Member

Return early when pathname is undefined to avoid inserting an extra ':' and strip a leading '/' from newDocId before concatenation

Return early when pathname is undefined to avoid inserting an extra ':'
and strip a leading '/' from newDocId before concatenation
@UnschooledGamer
Copy link
Copy Markdown
Member Author

UnschooledGamer commented Apr 9, 2026

Oh no, biome didn't complain locally.

We'll be fixed before merging

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 9, 2026

Greptile Summary

This PR adds an early-return path in Url.join for Termux content:// URLs when docId has no : separator (i.e., pathname is undefined), avoiding further processing that could corrupt the path. The separator logic inside the new block is dead code in practice — path.join(undefined, X) always produces \"/X\", so by the time if(!pathname) is evaluated, newDocId always starts with \"/\" and the two conditional branches never fire — but the returned URL is correct.

Confidence Score: 4/5

Safe to merge; the fix produces correct URLs for the targeted edge case.

The only remaining finding (P2) is that the two separator branches inside if(!pathname) are unreachable dead code — the logic is redundant but harmless. No correctness issues remain from the prior review thread.

src/utils/Url.js — the if(!pathname) block can be simplified by removing the dead separator branches.

Vulnerabilities

No security concerns identified.

Important Files Changed

Filename Overview
src/utils/Url.js Adds early return for Termux URLs when pathname is undefined; separator branches inside if(!pathname) are dead code due to prior rootCondition/newDocIdCondition processing, but the overall path result is correct.

Reviews (3): Last reviewed commit: "Update Url.js" | Re-trigger Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@UnschooledGamer
Copy link
Copy Markdown
Member Author

@greptileai review again

@UnschooledGamer
Copy link
Copy Markdown
Member Author

@greptileai

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant