fix(web): stop extractUrls treating email addresses as URLs#1187
fix(web): stop extractUrls treating email addresses as URLs#1187abhay-codes07 wants to merge 1 commit into
Conversation
URL_TOKEN_REGEX has no left boundary, so extractUrls matched domain-shaped tokens inside email addresses: "john.doe@example.com" produced two bogus URLs, https://john.doe and https://example.com. Since the add-document note view offers a bulk "N links detected - save each as its own memory?" banner at two or more detected URLs, a note containing a single email address triggered an import offer of garbage links. Switch to matchAll and inspect the characters adjacent to each match: tokens ending at "@" (email local part), starting after "@" (mail domain), or beginning mid-token are skipped. Also fix the dedupe key, which lowercased the whole URL and dropped distinct case-sensitive paths (https://example.com/Page vs /page). The key is now built from the parsed URL, so scheme and host compare case-insensitively while path, query, and hash stay case-sensitive. Add regression tests; four of the nine fail against the previous implementation.
![vorflux[bot]](https://avatars.githubusercontent.com/in/2483280?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
Reviewed — found 1 issue(s). I reviewed the extractUrls change in apps/web/lib/url-helpers.ts and the added regression coverage in apps/web/lib/extract-urls.test.ts. The new email guard still misses common email local-part formats, so some email addresses can still produce bogus extracted URLs.
Findings
apps/web/lib/url-helpers.ts
extractUrlsonly checks characters immediately adjacent to a matched domain token, which misses email local-parts containing separators such as+,_,-, or%before@.
Verdict
| // local part, one starting right after "@" is the mail domain. Also | ||
| // skip matches that begin mid-token (e.g. after "_", which the | ||
| // hostname charset can't include) — those aren't standalone URLs. | ||
| if (after === "@" || before === "@" || /[\w.-]/.test(before)) continue |
There was a problem hiding this comment.
The filter still misses common valid email local-parts that contain separators before the @. For example, first.last+tag@example.com can match first.last because the candidate ends before +, so neither adjacent character is @. Consider skipping the surrounding non-whitespace token whenever it contains @, or otherwise detecting email tokens before URL matching.
TestingThe testing subagent classified the change as internal URL-extraction logic only, with no UI, mobile, API, schema, or integration surface. It ran the focused Commands run: bun test apps/web/lib/extract-urls.test.ts
bunx biome ci apps/web/lib/url-helpers.ts apps/web/lib/extract-urls.test.tsResult: Verdict✅ Passed. Focused unit coverage and Biome checks passed for the changed URL helper and regression tests. |
What
extractUrlsinapps/web/lib/url-helpers.tstreats email addresses as URLs.URL_TOKEN_REGEXhas no left boundary, so forit extracts two bogus URLs:
https://john.doeandhttps://example.com.This is user-visible: the add-document note view (
components/add-document/note.tsx) shows a "N links detected — save each as its own memory?" banner when 2+ URLs are detected, so a note containing a single email address triggers a bulk-import offer of garbage links.There's a second defect in the same function: the dedupe key lowercases the entire URL, so
https://example.com/Pageandhttps://example.com/page— distinct, case-sensitive resources — are collapsed and one is silently dropped.How
matchtomatchAlland inspect the characters adjacent to each candidate: a token ending at@is an email local part, a token starting right after@is the mail domain, and a token beginning mid-word (e.g. after_, which the hostname charset can't include) isn't a standalone URL. All are skipped.URL(origin + pathname + search + hash), so scheme and host compare case-insensitively — which the URL parser normalizes anyway — while path, query, and hash stay case-sensitive. Trailing-slash-only variants still count as duplicates, as before.No API change: same signature, same
{urls, duplicates}result shape.Testing
apps/web/lib/extract-urls.test.ts(bun:test, same setup as the otherlib/*.test.tsfiles): emails alone, emails mixed with real URLs, multiple emails, markdown/angle-bracket links, scheme-less URLs, trailing punctuation, case/slash dedupe, and case-sensitive paths kept distinct.biome checkclean.Note: the test lives in its own file rather than
url-helpers.test.tsbecause #1182 (open) adds that file for the YouTube-detection fix — this way the two PRs merge cleanly in either order.cc @MaheshtheDev
Session Details
(aside)to your comment to have me ignore it.