Skip to content

Commit ea59606

Browse files
authored
(HTML-safe) inline markdown for publisher description. (dart-lang#3608)
* (HTML-safe) inline markdown for publisher description. * Test \r\n * line breaks * Fix XML/HTML check * Replace newlines before sanitize
1 parent e1bdbb4 commit ea59606

File tree

9 files changed

+179
-81
lines changed

9 files changed

+179
-81
lines changed

app/lib/frontend/templates/_utils.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ String renderFile(
2121
final content = file.text;
2222
if (content != null) {
2323
if (_isMarkdownFile(filename)) {
24-
return markdownToHtml(content, baseUrl,
25-
baseDir: p.dirname(filename), isChangelog: isChangelog);
24+
return markdownToHtml(content,
25+
baseUrl: baseUrl,
26+
baseDir: p.dirname(filename),
27+
isChangelog: isChangelog);
2628
} else if (_isDartFile(filename)) {
2729
return _renderDartCode(content);
2830
} else {
@@ -45,7 +47,7 @@ bool _isMarkdownFile(String filename) {
4547
bool _isDartFile(String filename) => filename.toLowerCase().endsWith('.dart');
4648

4749
String _renderDartCode(String text) =>
48-
markdownToHtml('````dart\n${text.trim()}\n````\n', null);
50+
markdownToHtml('````dart\n${text.trim()}\n````\n');
4951

5052
String _renderPlainText(String text) =>
5153
'<div class="highlight"><pre>${_escapeAngleBrackets(text)}</pre></div>';

app/lib/frontend/templates/misc.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import 'layout.dart';
2626
String renderUnauthenticatedPage({String messageMarkdown}) {
2727
messageMarkdown ??= 'You need to be logged in to view this page.';
2828
final content = templateCache.renderTemplate('account/unauthenticated', {
29-
'message_html': markdownToHtml(messageMarkdown, null),
29+
'message_html': markdownToHtml(messageMarkdown),
3030
});
3131
return renderLayoutPage(
3232
PageType.standalone,
@@ -41,7 +41,7 @@ String renderUnauthenticatedPage({String messageMarkdown}) {
4141
String renderUnauthorizedPage({String messageMarkdown}) {
4242
messageMarkdown ??= 'You have insufficient permissions to view this page.';
4343
final content = templateCache.renderTemplate('account/unauthorized', {
44-
'message_html': markdownToHtml(messageMarkdown, null),
44+
'message_html': markdownToHtml(messageMarkdown),
4545
});
4646
return renderLayoutPage(
4747
PageType.standalone,
@@ -106,7 +106,7 @@ String _renderStandalonePageContent({
106106
'Only one of `contentHtml` and `contentMarkdown` must be specified.');
107107
}
108108

109-
contentHtml ??= markdownToHtml(contentMarkdown, null);
109+
contentHtml ??= markdownToHtml(contentMarkdown);
110110
return templateCache.renderTemplate('page/standalone', {
111111
'content_html': contentHtml,
112112
'has_side_image': sideImageUrl != null,
@@ -118,7 +118,7 @@ String _renderStandalonePageContent({
118118
String renderErrorPage(String title, String message) {
119119
final values = {
120120
'title': title,
121-
'message_html': markdownToHtml(message, null),
121+
'message_html': markdownToHtml(message),
122122
};
123123
final String content = templateCache.renderTemplate('page/error', values);
124124
return renderLayoutPage(

app/lib/frontend/templates/package_analysis.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ String _renderSuggestionBlockHtml(String header, List<Suggestion> suggestions) {
9191
return {
9292
'icon_class': _suggestionIconClass(suggestion.level),
9393
'title_html': _renderSuggestionTitle(suggestion.title, suggestion.score),
94-
'description_html': markdownToHtml(suggestion.description, null),
94+
'description_html': markdownToHtml(suggestion.description),
9595
'suggestion_help_html': getSuggestionHelpMessage(suggestion.code),
9696
};
9797
}).toList();
@@ -120,7 +120,7 @@ String _renderSuggestionTitle(String title, double score) {
120120
if (formattedScore != null) {
121121
title = '$title ($formattedScore)';
122122
}
123-
return markdownToHtml(title, null);
123+
return markdownToHtml(title);
124124
}
125125

126126
String _formatSuggestionScore(double score) {

app/lib/frontend/templates/publisher.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:convert';
77
import 'package:client_data/publisher_api.dart' as api;
88
import 'package:client_data/page_data.dart';
99
import 'package:meta/meta.dart';
10+
import 'package:pub_dev/shared/markdown.dart';
1011

1112
import '../../package/models.dart' show PackageView;
1213
import '../../publisher/models.dart' show Publisher;
@@ -74,7 +75,7 @@ String _shortDescriptionHtml(Publisher publisher) {
7475
if (description != null && description.length > 1010) {
7576
description = description.substring(0, 1000) + '[...]';
7677
}
77-
return htmlEscape.convert(description);
78+
return markdownToHtml(description, inlineOnly: true);
7879
}
7980

8081
/// Renders the `views/publisher/info_box.mustache` template.

app/lib/search/text_utils.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ String compactDescription(String text) => _compactText(text, maxLength: 500);
4141

4242
String compactReadme(String text) {
4343
if (text == null || text.isEmpty) return '';
44-
final html = markdownToHtml(text, null)
44+
final html = markdownToHtml(text)
4545
.replaceAll('</li>', '\n</li>')
4646
.replaceAll('</p>', '\n</p>')
4747
.replaceAll('</ul>', '\n</ul>')

app/lib/shared/handler_helpers.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ shelf.Handler _logRequestWrapper(Logger logger, shelf.Handler handler) {
145145
if (shouldLog) {
146146
logger.info('Caught response exception: $e');
147147
}
148-
final content =
149-
markdownToHtml('# Error `${e.code}`\n\n${e.message}\n', null);
148+
final content = markdownToHtml('# Error `${e.code}`\n\n${e.message}\n');
150149
return htmlResponse(
151150
renderLayoutPage(PageType.package, content, title: 'Error ${e.code}'),
152151
status: e.status,
@@ -173,7 +172,7 @@ Request ID: ${context.traceId}
173172
''';
174173

175174
final content = renderLayoutPage(
176-
PageType.package, markdownToHtml(markdownText, null),
175+
PageType.package, markdownToHtml(markdownText),
177176
title: title);
178177
return htmlResponse(content, status: 500, headers: debugHeaders);
179178
} finally {

app/lib/shared/markdown.dart

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ final Logger _logger = Logger('pub.markdown');
1515
/// h4, h5 and h6 are considered formatting-only headers
1616
const _structuralHeaderTags = <String>{'h1', 'h2', 'h3'};
1717

18+
/// Matches more than 1 line breaks.
19+
final _multiLineBreakRegExp = RegExp(r'\n{2,}');
20+
21+
/// The element tags that are accepted when inline-only parsing is used.
22+
final _safeInlineTags = <String>{'em', 'strong'};
23+
1824
const _whitelistedClassNames = <String>[
1925
'changelog-entry',
2026
'changelog-version',
@@ -24,27 +30,36 @@ const _whitelistedClassNames = <String>[
2430
];
2531

2632
/// Renders markdown [text] to HTML.
33+
///
34+
/// Setting [inlineOnly] not only restricts the processed rules to inline syntax,
35+
/// but it also restricts the accepted output elements to bold and italics
36+
/// formatting.
2737
String markdownToHtml(
28-
String text,
29-
String baseUrl, {
38+
String text, {
39+
String baseUrl,
3040
String baseDir,
3141
bool isChangelog = false,
42+
bool inlineOnly = false,
3243
}) {
3344
if (text == null) return null;
34-
var nodes = _parseMarkdownSource(text);
45+
text = text.replaceAll('\r\n', '\n');
46+
var nodes = _parseMarkdownSource(text, inlineOnly);
3547
nodes = _rewriteRelativeUrls(nodes, baseUrl: baseUrl, baseDir: baseDir);
3648
if (isChangelog) {
3749
nodes = _groupChangelogNodes(nodes).toList();
3850
}
39-
return _renderSafeHtml(nodes);
51+
return _renderSafeHtml(nodes, inlineOnly);
4052
}
4153

4254
/// Parses markdown [source].
43-
List<m.Node> _parseMarkdownSource(String source) {
55+
List<m.Node> _parseMarkdownSource(String source, bool inlineOnly) {
4456
if (source == null) return null;
4557
final document = m.Document(
4658
extensionSet: m.ExtensionSet.gitHubWeb,
4759
blockSyntaxes: m.ExtensionSet.gitHubWeb.blockSyntaxes);
60+
if (inlineOnly) {
61+
return document.parseInline(source);
62+
}
4863
final lines = source.replaceAll('\r\n', '\n').split('\n');
4964
return document.parseLines(lines);
5065
}
@@ -63,25 +78,48 @@ List<m.Node> _rewriteRelativeUrls(
6378

6479
/// Renders sanitized, safe HTML from markdown nodes.
6580
/// Adds hash link HTML to header blocks.
66-
String _renderSafeHtml(List<m.Node> nodes) {
81+
String _renderSafeHtml(List<m.Node> nodes, bool inlineOnly) {
6782
// Filter unsafe urls on some of the elements.
68-
final unsafeUrlFilter = _UnsafeUrlFilter();
69-
nodes.forEach((node) => node.accept(unsafeUrlFilter));
83+
nodes.forEach((node) => node.accept(_UnsafeUrlFilter()));
84+
85+
if (inlineOnly) {
86+
_keepOnlyInlineElements(nodes);
87+
}
7088

7189
// add hash link HTML to header blocks
7290
final hashLink = _HashLink();
7391
nodes.forEach((node) => node.accept(hashLink));
7492

93+
var rawHtml = m.renderToHtml(nodes);
94+
if (inlineOnly) {
95+
rawHtml = rawHtml.replaceAll(_multiLineBreakRegExp, '<br />\n');
96+
}
97+
7598
// Renders the sanitized HTML.
7699
final html = sanitizeHtml(
77-
m.renderToHtml(nodes),
100+
rawHtml,
78101
allowElementId: (String id) => true, // TODO: blacklist ids used by pub site
79102
allowClassName: (String cn) {
80103
if (cn.startsWith('language-')) return true;
81104
return _whitelistedClassNames.contains(cn);
82105
},
83106
);
84-
return html + '\n';
107+
return inlineOnly ? html : '$html\n';
108+
}
109+
110+
void _keepOnlyInlineElements(List<m.Node> nodes) {
111+
if (nodes == null) return;
112+
for (var i = nodes.length - 1; i >= 0; i--) {
113+
final node = nodes[i];
114+
if (node is! m.Element) continue;
115+
116+
final elem = node as m.Element;
117+
_keepOnlyInlineElements(elem.children);
118+
if (!_safeInlineTags.contains(elem.tag)) {
119+
nodes.replaceRange(i, i + 1, [m.Text(elem.textContent)]);
120+
continue;
121+
}
122+
}
85123
}
86124

87125
/// Adds an extra <a href="#hash">#</a> element to h1, h2 and h3 elements.

app/test/frontend/golden/publisher_packages_page.html

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,9 @@ <h3 class="title">
200200
<aside class="detail-info-box">
201201
<h3 class="title">Description</h3>
202202
<p>This is our little software developer shop.
203-
204-
We develop full-stack in Dart, and happy about it.</p>
203+
<br/>
204+
We develop full-stack in Dart, and happy about it.
205+
</p>
205206
<h3 class="title">Publisher</h3>
206207
<p>
207208
<img class="-pub-publisher-shield" height="22" width="22" title="Domain ownership verified by pub.dev" src="/static/img/verified-publisher-blue.svg?hash=mocked_hash_919645162"/>example.com

0 commit comments

Comments
 (0)