From 4ef28ab5701ccd66425adcb5faa7251b83efa79c Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Thu, 14 Oct 2021 13:16:58 +0200 Subject: [PATCH] Sanitize dartdoc's HTML output. --- app/lib/dartdoc/customization.dart | 41 ++++++++++-- .../dartdoc/customizer_config_provider.dart | 2 + app/lib/dartdoc/dartdoc_runner.dart | 5 ++ app/lib/dartdoc/dartdoc_sanitizer.dart | 67 +++++++++++++++++++ app/test/dartdoc/customization_test.dart | 2 + app/test/dartdoc/dartdoc_sanitizer_test.dart | 63 +++++++++++++++++ 6 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 app/lib/dartdoc/dartdoc_sanitizer.dart create mode 100644 app/test/dartdoc/dartdoc_sanitizer_test.dart diff --git a/app/lib/dartdoc/customization.dart b/app/lib/dartdoc/customization.dart index 8d1ef1dbba..6993143baf 100644 --- a/app/lib/dartdoc/customization.dart +++ b/app/lib/dartdoc/customization.dart @@ -8,11 +8,18 @@ import 'dart:io'; import 'package:collection/collection.dart'; import 'package:html/dom.dart'; import 'package:html/parser.dart' as html_parser; +import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; + +import 'dartdoc_sanitizer.dart'; + +final _logger = Logger('dartdoc_customization'); class DartdocCustomizerConfig { final String packageName; final String packageVersion; final bool isLatestStable; + final bool isInternal; final String docRootUrl; final String latestStableDocumentationUrl; final String pubPackagePageUrl; @@ -26,6 +33,7 @@ class DartdocCustomizerConfig { required this.packageName, required this.packageVersion, required this.isLatestStable, + required this.isInternal, required this.docRootUrl, required this.latestStableDocumentationUrl, required this.pubPackagePageUrl, @@ -46,16 +54,41 @@ class DartdocCustomizer { final dir = Directory(path); await for (var fse in dir.list(recursive: true)) { if (fse is File && fse.path.endsWith('.html')) { - final c = await customizeFile(fse); + final relativeName = p.relative(fse.path, from: dir.path); + final level = p.split(relativeName).length - 1; + final c = await customizeFile(fse, level); changed = changed || c; } } return changed; } - Future customizeFile(File file) async { - final String oldContent = await file.readAsString(); - final newContent = customizeHtml(oldContent); + Future customizeFile(File file, int directoryLevel) async { + final oldContent = await file.readAsString(); + var newContent = oldContent; + try { + final sr = config.isInternal + ? SanitizerResult(true, oldContent, []) + : sanitizeDartdocHtml(oldContent, directoryLevel); + if (!sr.passed) { + _logger.info( + '[dartdoc-sanitized-html] Failed to sanitize ${config.packageName} ${config.packageVersion} ' + 'file ${file.path}: ${sr.removed.join('; ')}'); + } + newContent = customizeHtml(sr.contentHtml); + } catch (e, st) { + // Per-file catch-all for sanitization and customization, to make sure + // we will inspect all the files before uploading any of them. + _logger.shout( + '[dartdoc-sanitized-html] Failed to customize ${config.packageName} ${config.packageVersion} file ${file.path}', + e, + st); + + // Paranoid override, as we don't know what happened. + newContent = + 'Failed to customize dartdoc file.'; + } + // override file only if the content changed if (oldContent != newContent) { await file.writeAsString(newContent); return true; diff --git a/app/lib/dartdoc/customizer_config_provider.dart b/app/lib/dartdoc/customizer_config_provider.dart index 074acfdf8d..6037804cb2 100644 --- a/app/lib/dartdoc/customizer_config_provider.dart +++ b/app/lib/dartdoc/customizer_config_provider.dart @@ -12,11 +12,13 @@ DartdocCustomizerConfig customizerConfig({ required String packageName, required String packageVersion, required bool isLatestStable, + required bool isInternal, }) { return DartdocCustomizerConfig( packageName: packageName, packageVersion: packageVersion, isLatestStable: isLatestStable, + isInternal: isInternal, docRootUrl: isLatestStable ? pkgDocUrl(packageName, isLatest: true) : pkgDocUrl(packageName, version: packageVersion), diff --git a/app/lib/dartdoc/dartdoc_runner.dart b/app/lib/dartdoc/dartdoc_runner.dart index 2f747f6ceb..ad3734fe5d 100644 --- a/app/lib/dartdoc/dartdoc_runner.dart +++ b/app/lib/dartdoc/dartdoc_runner.dart @@ -19,6 +19,7 @@ import '../job/backend.dart'; import '../job/job.dart'; import '../package/backend.dart'; import '../package/models.dart'; +import '../package/overrides.dart'; import '../scorecard/backend.dart'; import '../scorecard/models.dart'; import '../shared/configuration.dart'; @@ -311,10 +312,14 @@ class DartdocJobProcessor extends JobProcessor { if (hasContent) { try { + final isInternal = + internalPackageNames.contains(job.packageName!) || + packageStatus.isPublishedByDartDev; await DartdocCustomizer(customizerConfig( packageName: job.packageName!, packageVersion: job.packageVersion!, isLatestStable: job.isLatestStable, + isInternal: isInternal, )).customizeDir(outputDir); logFileOutput.write('Content customization completed.\n\n'); } catch (e, st) { diff --git a/app/lib/dartdoc/dartdoc_sanitizer.dart b/app/lib/dartdoc/dartdoc_sanitizer.dart new file mode 100644 index 0000000000..20fcbeae43 --- /dev/null +++ b/app/lib/dartdoc/dartdoc_sanitizer.dart @@ -0,0 +1,67 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// 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:html/dom.dart'; +import 'package:html/parser.dart' as html_parser; + +class SanitizerResult { + final bool passed; + final String contentHtml; + final List removed; + + SanitizerResult(this.passed, this.contentHtml, this.removed); +} + +/// Removes unsafe elements and attributes from the output of dartdoc. +SanitizerResult sanitizeDartdocHtml(String input, int directoryLevel) { + final parsed = html_parser.parse(input); + final removed = []; + + void visit(Element elem) { + // remove elements based on tag and position + final tag = elem.localName?.toLowerCase() ?? ''; + if (tag == 'iframe') { + removed.add('iframe src="${elem.attributes['src']}"'); + elem.remove(); + return; + } + + // allow some

2

', + 0); + expect(rs.passed, false); + expect(rs.contentHtml, + '

1

2

'); + }); + + test('onclick="" is removed', () { + final rs = sanitizeDartdocHtml( + '

1

2

', + 0); + expect(rs.passed, false); + expect(rs.contentHtml, + '

1

2

'); + }); + + test('

2

', 0); + expect(rs.passed, false); + expect(rs.contentHtml, + '

1

2

'); + }); +}