diff --git a/app/lib/package/models.dart b/app/lib/package/models.dart index c764e5afc..0e715dfb9 100644 --- a/app/lib/package/models.dart +++ b/app/lib/package/models.dart @@ -11,7 +11,6 @@ import 'package:json_annotation/json_annotation.dart'; import 'package:pana/models.dart'; import 'package:pub_dev/service/download_counts/backend.dart'; import 'package:pub_dev/service/download_counts/download_counts.dart'; -import 'package:pub_dev/shared/markdown.dart'; import 'package:pub_semver/pub_semver.dart'; import '../package/model_properties.dart'; @@ -1071,11 +1070,7 @@ class PackageLinks { /// The link to `CONTRIBUTING.md` in the git repository (when the repository is verified). final String? contributingUrl; - /// The inferred base URL that can be used to link files from. - final String? _baseUrl; - - PackageLinks._( - this._baseUrl, { + PackageLinks._({ this.homepageUrl, String? documentationUrl, this.repositoryUrl, @@ -1093,12 +1088,7 @@ class PackageLinks { }) { repositoryUrl ??= urls.inferRepositoryUrl(homepageUrl); issueTrackerUrl ??= urls.inferIssueTrackerUrl(repositoryUrl); - final baseUrl = urls.inferBaseUrl( - homepageUrl: homepageUrl, - repositoryUrl: repositoryUrl, - ); return PackageLinks._( - baseUrl, homepageUrl: homepageUrl, documentationUrl: documentationUrl, repositoryUrl: repositoryUrl, @@ -1156,12 +1146,7 @@ class PackagePageData { return inferred; } - final baseUrl = urls.inferBaseUrl( - homepageUrl: result.homepageUrl ?? inferred.homepageUrl, - repositoryUrl: result.repositoryUrl ?? inferred.repositoryUrl, - ); return PackageLinks._( - baseUrl, homepageUrl: result.homepageUrl ?? inferred.homepageUrl, repositoryUrl: result.repositoryUrl ?? inferred.repositoryUrl, issueTrackerUrl: result.issueTrackerUrl ?? inferred.issueTrackerUrl, @@ -1170,10 +1155,16 @@ class PackagePageData { ); }(); - /// The verified repository (or homepage). - late final urlResolverFn = - scoreCard.panaReport?.result?.repository?.resolveUrl ?? - fallbackUrlResolverFn(packageLinks._baseUrl); + /// The URL resolver using a verified repository + /// (unless the verification explicitly failed). + late final urlResolverFn = () { + final result = scoreCard.panaReport?.result; + final status = result?.repositoryStatus; + if (status == RepositoryStatus.failed) { + return null; + } + return result?.repository?.resolveUrl; + }(); PackageView toPackageView() { return _view ??= PackageView.fromModel( diff --git a/app/lib/shared/markdown.dart b/app/lib/shared/markdown.dart index bcc21a599..54307ee66 100644 --- a/app/lib/shared/markdown.dart +++ b/app/lib/shared/markdown.dart @@ -4,11 +4,10 @@ import 'package:logging/logging.dart'; import 'package:markdown/markdown.dart' as m; -import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:sanitize_html/sanitize_html.dart'; -import 'urls.dart' show getRepositoryUrl, UriExt; +import 'urls.dart' show UriExt; /// Resolves [reference] relative to a repository URL. typedef UrlResolverFn = String Function( @@ -313,67 +312,6 @@ class _TaskListRewriteNodeVisitor implements m.NodeVisitor { void visitText(m.Text text) {} } -bool _isAbsolute(String url) => url.contains(':'); - -String _rewriteAbsoluteUrl(String url) { - final uri = Uri.parse(url); - if (uri.host == 'github.com') { - final segments = uri.pathSegments; - if (segments.length > 3 && segments[2] == 'blob') { - final newSegments = List.of(segments); - newSegments[2] = 'raw'; - return uri.replace(pathSegments: newSegments).toString(); - } - } - return url; -} - -String _rewriteRelativeUrl({ - required String baseUrl, - required String url, - required String? baseDir, -}) { - final uri = Uri.parse(url); - final linkPath = uri.path; - final linkFragment = uri.fragment; - if (linkPath.isEmpty) { - return url; - } - String newUrl; - if (linkPath.startsWith('/')) { - newUrl = Uri.parse(baseUrl).replace(path: linkPath).toString(); - } else { - final adjustedLinkPath = p.normalize(p.join(baseDir ?? '.', linkPath)); - final repoUrl = getRepositoryUrl(baseUrl, adjustedLinkPath); - if (repoUrl == null) { - return url; - } - newUrl = repoUrl; - } - if (linkFragment.isNotEmpty) { - newUrl = '$newUrl#$linkFragment'; - } - return newUrl; -} - -/// Returns null if the [url] looks invalid. -String? _pruneBaseUrl(String? url) { - if (url == null) return null; - try { - final Uri uri = Uri.parse(url); - if (uri.scheme != 'http' && uri.scheme != 'https') { - return null; - } - if (uri.host.isEmpty || !uri.host.contains('.')) { - return null; - } - return uri.toString(); - } catch (e) { - // url is user-provided, may be malicious, ignoring errors. - } - return null; -} - /// Group corresponding changelog nodes together, if it matches the following /// pattern: /// - version identifiers are the only content in a single line @@ -447,29 +385,3 @@ Version? _extractVersion(String? text) { return null; } } - -// TODO: remove after repository verification is launched -UrlResolverFn? fallbackUrlResolverFn(String? providedBaseUrl) { - final baseUrl = _pruneBaseUrl(providedBaseUrl); - if (baseUrl == null) { - return null; - } - return ( - String url, { - bool? isEmbeddedObject, - String? relativeFrom, - }) { - String newUrl = url; - if (!_isAbsolute(newUrl)) { - newUrl = _rewriteRelativeUrl( - url: newUrl, - baseUrl: baseUrl, - baseDir: relativeFrom == null ? null : p.dirname(relativeFrom), - ); - } - if ((isEmbeddedObject ?? false) && _isAbsolute(newUrl)) { - newUrl = _rewriteAbsoluteUrl(newUrl); - } - return newUrl; - }; -} diff --git a/app/lib/shared/urls.dart b/app/lib/shared/urls.dart index 231fb2ec5..00a263060 100644 --- a/app/lib/shared/urls.dart +++ b/app/lib/shared/urls.dart @@ -17,25 +17,6 @@ const httpsApiDartDev = 'https://api.dart.dev/'; /// rejected and the URL mustn't be displayed. const trustedUrlSchemes = ['http', 'https', 'mailto']; -/// Extensions that are considered to be images. -final _imageExtensions = { - '.apng', - '.avif', - '.gif', - '.jpg', - '.jpeg', - '.png', - '.svg', - '.webp', -}; - -/// Common repository URL replacement patterns. -const _repositoryReplacePrefixes = { - 'http://github.com': 'https://github.com', - 'https://www.github.com': 'https://github.com', - 'https://www.gitlab.com': 'https://gitlab.com', -}; - /// Hostnames that are trusted in user-generated content (and don't get rel="ugc"). const trustedTargetHost = [ 'api.dart.dev', @@ -294,42 +275,6 @@ String? inferIssueTrackerUrl(String? baseUrl) { return null; } -/// Infer base URL that can be used to link files from. -String? inferBaseUrl({String? homepageUrl, String? repositoryUrl}) { - var baseUrl = repositoryUrl ?? homepageUrl; - if (baseUrl == null) return null; - - // In a few cases people specify only a deep repository URL for their - // package (e.g. a monorepo for multiple packages). While we default the - // base URL to be the repository URL, this check allows us to use the deep - // URL for linking. - if (homepageUrl != null && homepageUrl.startsWith(baseUrl)) { - baseUrl = homepageUrl; - } - - // URL cleanup - final prefixReplacements = const { - 'http://github.com/': 'https://github.com/', - 'http://www.github.com/': 'https://github.com/', - 'https://www.github.com/': 'https://github.com/', - 'http://gitlab.com/': 'https://gitlab.com/', - 'http://www.gitlab.com/': 'https://gitlab.com/', - 'https://www.gitlab.com/': 'https://gitlab.com/', - }; - for (final prefix in prefixReplacements.keys) { - if (baseUrl!.startsWith(prefix)) { - baseUrl = baseUrl.replaceFirst(prefix, prefixReplacements[prefix]!); - } - } - if ((baseUrl!.startsWith('https://github.com/') || - baseUrl.startsWith('https://gitlab.com/')) && - baseUrl.endsWith('.git')) { - baseUrl = baseUrl.substring(0, baseUrl.length - 4); - } - - return baseUrl; -} - /// Infer the hosting/service provider for a given URL. String? inferServiceProviderName(String? url) { if (url == null) { @@ -383,57 +328,6 @@ String myPublishersUrl() => '/my-publishers'; String myActivityLogUrl() => '/my-activity-log'; String createPublisherUrl() => '/create-publisher'; -/// Returns an URL that is likely the downloadable URL of the given path. -String? getRepositoryUrl( - String? repository, - String relativePath, { - String branch = 'master', -}) { - if (repository == null || repository.isEmpty) return null; - for (final key in _repositoryReplacePrefixes.keys) { - if (repository!.startsWith(key)) { - repository = - repository.replaceFirst(key, _repositoryReplacePrefixes[key]!); - } - } - try { - final uri = Uri.parse(repository!); - final segments = List.of(uri.pathSegments); - while (segments.isNotEmpty && segments.last.isEmpty) { - segments.removeLast(); - } - - if (repository.startsWith('https://github.com/') || - repository.startsWith('https://gitlab.com/')) { - if (segments.length >= 2 && - segments[1].endsWith('.git') && - segments[1].length > 4) { - segments[1] = segments[1].substring(0, segments[1].length - 4); - } - - final extension = p.extension(relativePath).toLowerCase(); - final isRaw = _imageExtensions.contains(extension); - final typeSegment = isRaw ? 'raw' : 'blob'; - - if (segments.length < 2) { - return null; - } else if (segments.length == 2) { - final newUrl = uri.replace(pathSegments: segments).toString(); - return p.url.join(newUrl, typeSegment, branch, relativePath); - } else if (segments[2] == 'tree' || segments[2] == 'blob') { - segments[2] = typeSegment; - final newUrl = uri.replace(pathSegments: segments).toString(); - return p.url.join(newUrl, relativePath); - } else { - return null; - } - } - } catch (_) { - return null; - } - return null; -} - /// Parses [url] and returns the [Uri] object only if the result Uri is valid /// (e.g. is relative or has recognized scheme). Uri? parseValidUrl(String? url) { diff --git a/app/test/shared/markdown_test.dart b/app/test/shared/markdown_test.dart index 38f0f23b5..98b6426ce 100644 --- a/app/test/shared/markdown_test.dart +++ b/app/test/shared/markdown_test.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'package:pana/pana.dart'; import 'package:pub_dev/shared/markdown.dart'; import 'package:test/test.dart'; @@ -30,7 +31,7 @@ void main() { group('Valid custom base URL', () { final baseUrl = 'https://github.com/example/project'; - final urlResolverFn = fallbackUrlResolverFn(baseUrl); + final urlResolverFn = Repository.parseUrl(baseUrl).resolveUrl; test('relative link within page', () { expect(markdownToHtml('[text](#relative)'), @@ -109,7 +110,7 @@ void main() { expect( markdownToHtml('[text](/example/README.md)', urlResolverFn: urlResolverFn), - '

text

\n'); + '

text

\n'); }); test('root image within site', () { @@ -117,7 +118,7 @@ void main() { expect( markdownToHtml('![text](/example/image.png)', urlResolverFn: urlResolverFn), - '

text

\n'); + '

text

\n'); }); test('email', () { @@ -130,22 +131,6 @@ void main() { }); }); - group('Bad custom base URL', () { - test('not http(s)', () { - expect( - markdownToHtml('[text](README.md)', - urlResolverFn: fallbackUrlResolverFn('ftp://example.com/blah')), - '

text

\n'); - }); - - test('not valid host', () { - expect( - markdownToHtml('[text](README.md)', - urlResolverFn: fallbackUrlResolverFn('http://com/blah')), - '

text

\n'); - }); - }); - group('Unsafe markdown', () { test('javascript link', () { expect(markdownToHtml('[a](javascript:alert("x"))'), '

a

\n'); @@ -153,13 +138,6 @@ void main() { }); group('Bad markdown', () { - test('bad link', () { - expect( - markdownToHtml('[a][b]', - urlResolverFn: fallbackUrlResolverFn('http://www.example.com/')), - '

[a][b]

\n'); - }); - test('bad link, keeping link text', () { expect(markdownToHtml('[my illegal url](http://illegal@@thing)'), '

my illegal url

\n'); @@ -259,17 +237,22 @@ void main() { test('root path: /[..]/blob/master/[path].gif', () { expect( markdownToHtml( - '![text](/rcpassos/progress_hud/blob/master/progress_hud.gif)', - urlResolverFn: fallbackUrlResolverFn( - 'https://github.com/rcpassos/progress_hud')), - '

text

\n'); + '![text](/rcpassos/progress_hud/blob/master/progress_hud.gif)', + urlResolverFn: + Repository.parseUrl('https://github.com/rcpassos/progress_hud') + .resolveUrl, + ), + '

text

\n'); }); test('relative path: [path].gif', () { expect( - markdownToHtml('![text](progress_hud.gif)', - urlResolverFn: fallbackUrlResolverFn( - 'https://github.com/rcpassos/progress_hud')), + markdownToHtml( + '![text](progress_hud.gif)', + urlResolverFn: + Repository.parseUrl('https://github.com/rcpassos/progress_hud') + .resolveUrl, + ), '

text

\n'); }); }); diff --git a/app/test/shared/urls_test.dart b/app/test/shared/urls_test.dart index 05af48336..f8e7c1bce 100644 --- a/app/test/shared/urls_test.dart +++ b/app/test/shared/urls_test.dart @@ -117,66 +117,6 @@ void main() { }); }); - group('Infer base URL', () { - final repo = 'https://gitlab.com/user/repo'; - - test('only homepage is set', () { - expect(inferBaseUrl(homepageUrl: repo), repo); - expect( - inferBaseUrl(homepageUrl: '$repo/tree/master/dir'), - '$repo/tree/master/dir', - ); - }); - - test('only repository is set', () { - expect(inferBaseUrl(repositoryUrl: repo), repo); - expect( - inferBaseUrl(repositoryUrl: '$repo/tree/master/dir'), - '$repo/tree/master/dir', - ); - }); - - test('deep homepage, inferred repository', () { - final homepage = '$repo/tree/master/dir'; - expect( - inferBaseUrl( - homepageUrl: homepage, - repositoryUrl: inferRepositoryUrl(homepage), - ), - '$repo/tree/master/dir', - ); - }); - - test('unrelated homepage, simple repository', () { - final homepage = 'https://example.com/'; - expect( - inferBaseUrl( - homepageUrl: homepage, - repositoryUrl: repo, - ), - repo, - ); - }); - - test('URL cleanup', () { - expect( - inferBaseUrl( - homepageUrl: 'http://gitlab.com/user/repo.git', - ), - repo); - expect( - inferBaseUrl( - homepageUrl: 'http://www.gitlab.com/user/repo.git', - ), - repo); - expect( - inferBaseUrl( - homepageUrl: 'https://www.gitlab.com/user/repo.git', - ), - repo); - }); - }); - group('search urls', () { test('non-identifier characters', () { expect(searchUrl(q: 'a b'), '/packages?q=a+b'); diff --git a/app/test/task/testdata/goldens/packages/oxygen/example.html b/app/test/task/testdata/goldens/packages/oxygen/example.html index 4b1c09c67..806bb5bc7 100644 --- a/app/test/task/testdata/goldens/packages/oxygen/example.html +++ b/app/test/task/testdata/goldens/packages/oxygen/example.html @@ -202,9 +202,7 @@

Metadata

- - example/example.dart - + example/example.dart

                     main() { print('example'); }
diff --git a/app/test/task/testdata/goldens/packages/oxygen/versions/1.0.0/example.html b/app/test/task/testdata/goldens/packages/oxygen/versions/1.0.0/example.html
index d1b23951c..202895a35 100644
--- a/app/test/task/testdata/goldens/packages/oxygen/versions/1.0.0/example.html
+++ b/app/test/task/testdata/goldens/packages/oxygen/versions/1.0.0/example.html
@@ -206,9 +206,7 @@ 

Metadata

- - example/example.dart - + example/example.dart

                     main() { print('example'); }