Skip to content

Commit

Permalink
Strict markdown link pruning for user-provided content. (#7901)
Browse files Browse the repository at this point in the history
  • Loading branch information
@isoos
isoos authored Aug 13, 2024
1 parent 818845d commit 67db7e2
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 33 deletions.
25 changes: 20 additions & 5 deletions app/lib/frontend/dom/dom.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,27 @@ Node xAgoTimestamp(DateTime timestamp, {String? datePrefix}) {
/// Creates a DOM node with markdown content using the default [DomContext].
Node markdown(
String text, {
bool disableHashIds = false,
/// When set, the markdown output is pruned more heavily than usual, like:
/// - generated hash IDs are removed
/// - relative URLs of links and images are removed (and replaced with their text content)
///
/// Should be used for markdown content that is not entirely in our control.
bool strictPruning = false,
}) {
return dom.unsafeRawHtml(markdownToHtml(
text,
disableHashIds: disableHashIds,
));
return dom.unsafeRawHtml(
markdownToHtml(
text,
disableHashIds: strictPruning,
urlResolverFn: strictPruning
? null
: (
url, {
String? relativeFrom,
bool? isEmbeddedObject,
}) =>
url,
),
);
}

/// Creates DOM elements with <pre> and <code> for HLJS and pub.dev's copy-to-clipboard.
Expand Down
2 changes: 1 addition & 1 deletion app/lib/frontend/templates/views/pkg/score_tab.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ d.Node _section(ReportSection section) {
classes: ['pkg-report-content-summary', 'markdown-body'],
child: d.markdown(
_updatedSummary(section.summary),
disableHashIds: true,
strictPruning: true,
),
),
),
Expand Down
36 changes: 27 additions & 9 deletions app/lib/shared/markdown.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ class _RelativeUrlRewriter implements m.NodeVisitor {

if (r.children != null && r.children!.isNotEmpty) {
element.children!.insertAll(index, r.children!);
} else if (r.tag == 'img' && r.attributes.containsKey('alt')) {
element.children!.insert(index, m.Text('[${r.attributes['alt']}]'));
}
element.children!.remove(r);
_elementsToRemove.remove(r);
Expand All @@ -237,25 +239,41 @@ class _RelativeUrlRewriter implements m.NodeVisitor {
}
}

/// Returns a new URL based on [url] and [urlResolverFn].
///
/// When the returned value is `null`, the containing DOM node will be pruned
/// from the output, otherwise this method may return the same or an updated URL.
String? _rewriteUrl(String? url, {bool raw = false}) {
// pass-through for simple cases
if (url == null || url.startsWith('#')) {
return url;
}
if (Uri.tryParse(url) == null) {
// reject unparseable URLs
final uri = Uri.tryParse(url);
if (uri == null) {
return null;
}
try {
if (urlResolverFn != null) {
return urlResolverFn!(
url,
relativeFrom: relativeFrom,
isEmbeddedObject: raw,
);
if (urlResolverFn == null) {
// When we have no URL resolver, we may allow absolute URLs (with scheme) or
// in-page URLs (only fragments without any path reference).
if (uri.hasScheme || (uri.path.isEmpty && uri.hasFragment)) {
return url;
}
// Otherwise we should not display the URL and remove the DOM node.
return null;
}
try {
// Trying to resolve the URL using the given resolver function.
return urlResolverFn!(
url,
relativeFrom: relativeFrom,
isEmbeddedObject: raw,
);
} catch (e, st) {
_logger.warning('Link rewrite failed: $url', e, st);
}
return url;
// Safe URL fallback: removing the node containing the URL that we were not able to resolve.
return null;
}
}

Expand Down
29 changes: 11 additions & 18 deletions app/test/shared/markdown_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,13 @@ void main() {
});

test('sibling link within site', () {
expect(markdownToHtml('[text](README.md)'),
'<p><a href="README.md">text</a></p>\n');
expect(markdownToHtml('[text](README.md)'), '<p>text</p>\n');
expect(markdownToHtml('[text](README.md)', urlResolverFn: urlResolverFn),
'<p><a href="https://github.com/example/project/blob/master/README.md" rel="ugc">text</a></p>\n');
});

test('sibling image within site', () {
expect(markdownToHtml('![text](image.png)'),
'<p><img src="image.png" alt="text"></p>\n');
expect(markdownToHtml('![text](image.png)'), '<p>[text]</p>\n');
expect(markdownToHtml('![text](image.png)', urlResolverFn: urlResolverFn),
'<p><img src="https://github.com/example/project/raw/master/image.png" alt="text"></p>\n');
});
Expand All @@ -64,52 +62,47 @@ void main() {
expect(
markdownToHtml('![text](image.png)',
relativeFrom: 'example/README.md'),
'<p><img src="image.png" alt="text"></p>\n');
'<p>[text]</p>\n');
expect(
markdownToHtml('![text](image.png)',
urlResolverFn: urlResolverFn, relativeFrom: 'example/README.md'),
'<p><img src="https://github.com/example/project/raw/master/example/image.png" alt="text"></p>\n');
});

test('sibling link plus relative link', () {
expect(markdownToHtml('[text](README.md#section)'),
'<p><a href="README.md#section">text</a></p>\n');
expect(markdownToHtml('[text](README.md#section)'), '<p>text</p>\n');
expect(
markdownToHtml('[text](README.md#section)',
urlResolverFn: urlResolverFn),
'<p><a href="https://github.com/example/project/blob/master/README.md#section" rel="ugc">text</a></p>\n');
});

test('child link within site', () {
expect(markdownToHtml('[text](example/README.md)'),
'<p><a href="example/README.md">text</a></p>\n');
expect(markdownToHtml('[text](example/README.md)'), '<p>text</p>\n');
expect(
markdownToHtml('[text](example/README.md)',
urlResolverFn: urlResolverFn),
'<p><a href="https://github.com/example/project/blob/master/example/README.md" rel="ugc">text</a></p>\n');
});

test('child image within site', () {
expect(markdownToHtml('![text](example/image.png)'),
'<p><img src="example/image.png" alt="text"></p>\n');
expect(markdownToHtml('![text](example/image.png)'), '<p>[text]</p>\n');
expect(
markdownToHtml('![text](example/image.png)',
urlResolverFn: urlResolverFn),
'<p><img src="https://github.com/example/project/raw/master/example/image.png" alt="text"></p>\n');
});

test('root link within site', () {
expect(markdownToHtml('[text](/README.md)'),
'<p><a href="/README.md">text</a></p>\n');
expect(markdownToHtml('[text](/README.md)'), '<p>text</p>\n');
expect(
markdownToHtml('[text](/example/README.md)',
urlResolverFn: urlResolverFn),
'<p><a href="https://github.com/example/README.md" rel="ugc">text</a></p>\n');
});

test('root image within site', () {
expect(markdownToHtml('![text](/image.png)'),
'<p><img src="/image.png" alt="text"></p>\n');
expect(markdownToHtml('![text](/image.png)'), '<p>[text]</p>\n');
expect(
markdownToHtml('![text](/example/image.png)',
urlResolverFn: urlResolverFn),
Expand All @@ -131,14 +124,14 @@ void main() {
expect(
markdownToHtml('[text](README.md)',
urlResolverFn: fallbackUrlResolverFn('ftp://example.com/blah')),
'<p><a href="README.md">text</a></p>\n');
'<p>text</p>\n');
});

test('not valid host', () {
expect(
markdownToHtml('[text](README.md)',
urlResolverFn: fallbackUrlResolverFn('http://com/blah')),
'<p><a href="README.md">text</a></p>\n');
'<p>text</p>\n');
});
});

Expand Down Expand Up @@ -170,7 +163,7 @@ void main() {

test('bad image link with attribute', () {
expect(markdownToHtml('![demo](src="https://github.com/a/b/c.gif")'),
'<p></p>\n');
'<p>[demo]</p>\n');
});
});

Expand Down

0 comments on commit 67db7e2

Please sign in to comment.