Skip to content

Commit f78de0a

Browse files
committed
discovery: reject malformed URLs, keep brackets in markdown destinations
Addresses further Copilot feedback on #994: - A candidate was only checked against the "https://" text prefix, so malformed values like bare "https://" or "https://?query" (no host) were still emitted as tappable links. Validate every candidate with `new URL(...)` and require the "https:" protocol before treating it as a link; rejected candidates are left as plain text (the original substring, not `url.href`, is still used for display/navigation, since the WHATWG URL parser normalizes some inputs like adding a trailing slash to a bare origin). - The URL boundary excluded square brackets universally, which broke markdown-link destinations containing the common `?tag[]=value` query syntax: the parse failed and the same target was then emitted as a truncated bare link pointing at the wrong address. Brackets are now only excluded from a *bare* URL's boundary (to avoid ambiguity with an immediately following markdown link) - a markdown destination's balanced closing paren is enough to bound it on its own. - Added the same eslint-disable-next-line react/no-array-index-key suppression to the linked-segment branch in LinkedText.tsx that the plain-text branch already had, since the AirBnB rule also flags indexes used inside template literals. Added regression tests for both parser fixes, matching stellar/freighter#2985.
1 parent e95fdbc commit f78de0a

3 files changed

Lines changed: 79 additions & 17 deletions

File tree

__tests__/helpers/linkedText.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,31 @@ describe("parseLinkedText", () => {
119119
{ text: '" now.' },
120120
]);
121121
});
122+
123+
it("does not linkify a malformed bare URL with no host", () => {
124+
expect(parseLinkedText("See https:// for syntax")).toEqual([
125+
{ text: "See https:// for syntax" },
126+
]);
127+
expect(parseLinkedText("See https://?query for syntax")).toEqual([
128+
{ text: "See https://?query for syntax" },
129+
]);
130+
});
131+
132+
it("does not linkify a malformed markdown URL with no host", () => {
133+
expect(parseLinkedText("[bad](https://)")).toEqual([
134+
{ text: "[bad](https://)" },
135+
]);
136+
});
137+
138+
it("keeps square brackets inside a markdown destination", () => {
139+
expect(
140+
parseLinkedText(
141+
"Search [tags](https://example.com/search?tag[]=security) here.",
142+
),
143+
).toEqual([
144+
{ text: "Search " },
145+
{ text: "tags", url: "https://example.com/search?tag[]=security" },
146+
{ text: " here." },
147+
]);
148+
});
122149
});

src/components/LinkedText.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export const LinkedText: React.FC<LinkedTextProps> = ({
2424
<Text {...textProps}>
2525
{parseLinkedText(children).map((segment, index) =>
2626
segment.url ? (
27+
// eslint-disable-next-line react/no-array-index-key
2728
<Text
2829
key={`${index}-${segment.url}`}
2930
{...textProps}

src/helpers/linkedText.ts

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@ export interface LinkedTextSegment {
1515
// used elsewhere for protocol URLs (see helpers/protocols.ts).
1616
const HTTPS_PREFIX = "https://";
1717

18-
// Characters that always end a URL, whether or not it's inside a markdown
19-
// link: whitespace, angle brackets, square brackets (markdown syntax), and
20-
// quotes (prose delimiters - a quoted URL shouldn't swallow the closing
21-
// quote). Parentheses are handled separately in findUrlEnd, since a URL may
18+
// Characters that always end a URL: whitespace, angle brackets, and quotes
19+
// (prose delimiters - a quoted URL shouldn't swallow the closing quote).
20+
// Parentheses are handled separately in findUrlEnd, since a URL may
2221
// legitimately contain balanced ones (e.g. a Wikipedia article title).
23-
const URL_BOUNDARY_CHAR = /[\s<>[\]"']/;
22+
const BARE_URL_BOUNDARY_CHAR = /[\s<>[\]"']/;
23+
24+
// Inside a markdown link's `(https://...)` destination, square brackets are
25+
// valid URL characters (e.g. the common `?tag[]=value` query syntax) - only
26+
// the balanced closing paren, whitespace, angle brackets, or a quote can end
27+
// it. Brackets are excluded from a *bare* URL's boundary set above only to
28+
// avoid ambiguity with an immediately following markdown link.
29+
const MARKDOWN_URL_BOUNDARY_CHAR = /[\s<>"']/;
2430

2531
const TRAILING_PUNCTUATION = /[.,!?;:]+$/;
2632

@@ -37,19 +43,34 @@ const splitTrailingPunctuation = (
3743
};
3844
};
3945

40-
// Extends a URL starting at `start` as far as possible, allowing balanced
41-
// parentheses inside it (e.g. https://en.wikipedia.org/wiki/Function_(maths))
42-
// so a legitimate paren in the URL path isn't mistaken for markdown/prose
43-
// punctuation. An unmatched closing paren ends the URL instead of being
44-
// consumed by it, since it's almost always the boundary of a markdown link
45-
// or the prose wrapping a link in parens.
46-
const findUrlEnd = (text: string, start: number): number => {
46+
// A candidate is only treated as a link if it's an actual parseable https
47+
// URL with a host - e.g. bare "https://" or "https://?query" (no host) are
48+
// rejected and left as plain text, matching the HTTPS-only convention used
49+
// elsewhere for protocol URLs. The original text is still used for
50+
// display/navigation (never `url.href`), since the WHATWG URL parser
51+
// normalizes some inputs (e.g. adding a trailing slash to a bare origin).
52+
const isValidHttpsUrl = (candidate: string): boolean => {
53+
try {
54+
return new URL(candidate).protocol === "https:";
55+
} catch {
56+
return false;
57+
}
58+
};
59+
60+
// Extends a URL starting at `start` as far as possible per `boundary`,
61+
// allowing balanced parentheses inside it (e.g.
62+
// https://en.wikipedia.org/wiki/Function_(maths)) so a legitimate paren in
63+
// the URL path isn't mistaken for markdown/prose punctuation. An unmatched
64+
// closing paren ends the URL instead of being consumed by it, since it's
65+
// almost always the boundary of a markdown link or the prose wrapping a
66+
// link in parens.
67+
const findUrlEnd = (text: string, start: number, boundary: RegExp): number => {
4768
let depth = 0;
4869
let index = start;
4970

5071
while (index < text.length) {
5172
const char = text[index];
52-
if (URL_BOUNDARY_CHAR.test(char)) {
73+
if (boundary.test(char)) {
5374
break;
5475
}
5576
if (char === "(") {
@@ -76,7 +97,8 @@ interface MarkdownLink {
7697
// at the `[` found at `openIndex`. Returns null if it isn't one - e.g. no
7798
// matching `]`, the label itself contains an unescaped `[` (meaning
7899
// `openIndex` isn't the real opening bracket - see the "nested brackets"
79-
// test case), or there's no `(https://...)` immediately after the `]`.
100+
// test case), there's no `(https://...)` immediately after the `]`, or the
101+
// URL itself isn't a valid https URL with a host.
80102
// Parsing forward like this (rather than scanning backward from a `https://`
81103
// occurrence) means a label that itself contains a URL - e.g.
82104
// `[https://a.example](https://b.example)` - is handled as a single link
@@ -104,12 +126,17 @@ const tryParseMarkdownLink = (
104126
return null;
105127
}
106128

107-
const urlEnd = findUrlEnd(text, urlStart);
129+
const urlEnd = findUrlEnd(text, urlStart, MARKDOWN_URL_BOUNDARY_CHAR);
108130
if (text[urlEnd] !== ")") {
109131
return null;
110132
}
111133

112-
return { label, url: text.slice(urlStart, urlEnd), end: urlEnd + 1 };
134+
const url = text.slice(urlStart, urlEnd);
135+
if (!isValidHttpsUrl(url)) {
136+
return null;
137+
}
138+
139+
return { label, url, end: urlEnd + 1 };
113140
};
114141

115142
/**
@@ -158,11 +185,18 @@ export const parseLinkedText = (text: string): LinkedTextSegment[] => {
158185
break;
159186
}
160187

161-
const urlEnd = findUrlEnd(text, httpsIndex);
188+
const urlEnd = findUrlEnd(text, httpsIndex, BARE_URL_BOUNDARY_CHAR);
162189
const { url, trailing } = splitTrailingPunctuation(
163190
text.slice(httpsIndex, urlEnd),
164191
);
165192

193+
if (!isValidHttpsUrl(url)) {
194+
// Not a real URL (e.g. bare "https://" with no host) - leave it as
195+
// plain text and keep scanning past the prefix.
196+
searchFrom = httpsIndex + HTTPS_PREFIX.length;
197+
continue;
198+
}
199+
166200
if (httpsIndex > emitted) {
167201
segments.push({ text: text.slice(emitted, httpsIndex) });
168202
}

0 commit comments

Comments
 (0)