Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize HTML sanitizing when preview email #3223

Merged
merged 9 commits into from
Oct 24, 2024
9 changes: 9 additions & 0 deletions contact/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,15 @@ packages:
url: "https://pub.dev"
source: hosted
version: "3.2.1"
sanitize_html:
dab246 marked this conversation as resolved.
Show resolved Hide resolved
dependency: transitive
description:
path: sanitize_html
ref: support_mail
resolved-ref: fda32cde4d4baadaa988477f498ab6622ee79987
url: "https://github.com/linagora/dart-neats.git"
source: git
version: "2.1.0"
shelf:
dependency: transitive
description:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ class MessageContentTransformer {
Map<String, String>? mapUrlDownloadCID
}) async {
await Future.wait([
if (_configuration.domTransformers.isNotEmpty)
..._configuration.domTransformers.map((domTransformer) async =>
domTransformer.process(
document: document,
dioClient: _dioClient,
mapUrlDownloadCID: mapUrlDownloadCID,
)
..._configuration.domTransformers.map((domTransformer) async =>
domTransformer.process(
document: document,
dioClient: _dioClient,
mapUrlDownloadCID: mapUrlDownloadCID,
)
)
]);
}
Expand All @@ -38,24 +37,32 @@ class MessageContentTransformer {
required String message,
Map<String, String>? mapUrlDownloadCID
}) async {
final document = parse(message);
await _transformDocument(
document: document,
mapUrlDownloadCID: mapUrlDownloadCID,
);
final newMessage = _configuration.textTransformers.isNotEmpty
? _transformMessage(message)
: message;

final document = parse(newMessage);

if (_configuration.domTransformers.isNotEmpty) {
await _transformDocument(
document: document,
mapUrlDownloadCID: mapUrlDownloadCID,
);
}

return document;
}

String _transformMessage(String message) {
if (_configuration.textTransformers.isNotEmpty) {
for (var transformer in _configuration.textTransformers) {
message = transformer.process(message, _htmlEscape);
}
for (var transformer in _configuration.textTransformers) {
message = transformer.process(message, _htmlEscape);
}
return message;
}

String toMessage(String message) {
return _transformMessage(message);
return _configuration.textTransformers.isNotEmpty
? _transformMessage(message)
: message;
}
}
16 changes: 16 additions & 0 deletions core/lib/presentation/utils/html_transformer/sanitize_html.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import 'package:sanitize_html/sanitize_html.dart';

class SanitizeHtml {
String process({
required String inputHtml,
List<String>? allowAttributes,
List<String>? allowTags,
}) {
final outputHtml = sanitizeHtml(
inputHtml,
allowAttributes: allowAttributes,
allowTags: allowTags,
);
return outputHtml;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'package:core/utils/app_logger.dart';
import 'package:get/get.dart';

class SanitizeUrl {
Expand All @@ -24,7 +23,6 @@ class SanitizeUrl {
} else {
originalUrl = '';
}
log('SanitizeUrl::process:originalUrl = $originalUrl');
return originalUrl;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import 'dart:convert';
import 'package:core/presentation/utils/html_transformer/base/text_transformer.dart';
import 'package:core/presentation/utils/html_transformer/sanitize_html.dart';

class StandardizeHtmlSanitizingTransformers extends TextTransformer {

static const List<String> mailAllowedHtmlAttributes = [
'style',
'public-asset-id',
'data-filename',
'bgcolor',
'id',
'class',
];

static const List<String> mailAllowedHtmlTags = [
'font',
'u',
'center',
'style',
'body',
];

const StandardizeHtmlSanitizingTransformers();

@override
String process(String text, HtmlEscape htmlEscape) =>
SanitizeHtml().process(
hoangdat marked this conversation as resolved.
Show resolved Hide resolved
inputHtml: text,
allowAttributes: mailAllowedHtmlAttributes,
allowTags: mailAllowedHtmlTags,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import 'package:core/presentation/utils/html_transformer/dom/remove_tooltip_link
import 'package:core/presentation/utils/html_transformer/dom/sanitize_hyper_link_tag_in_html_transformers.dart';
import 'package:core/presentation/utils/html_transformer/dom/script_transformers.dart';
import 'package:core/presentation/utils/html_transformer/dom/signature_transformers.dart';
import 'package:core/presentation/utils/html_transformer/text/sanitize_autolink_html_transformers.dart';
import 'package:core/presentation/utils/html_transformer/text/standardize_html_sanitizing_transformers.dart';
import 'package:core/utils/platform_info.dart';

/// Contains the configuration for all transformations.
Expand All @@ -37,7 +37,9 @@ class TransformConfiguration {

factory TransformConfiguration.fromDomTransformers(List<DomTransformer> domTransformers) => TransformConfiguration(domTransformers, []);

factory TransformConfiguration.empty() => const TransformConfiguration([], []);
factory TransformConfiguration.fromTextTransformers(
List<TextTransformer> textTransformers
) => TransformConfiguration([], textTransformers);

factory TransformConfiguration.forReplyForwardEmail() => TransformConfiguration.fromDomTransformers([
if (PlatformInfo.isWeb)
Expand All @@ -46,10 +48,15 @@ class TransformConfiguration {
const RemoveCollapsedSignatureButtonTransformer(),
]);

factory TransformConfiguration.forDraftsEmail() => TransformConfiguration.fromDomTransformers([const ImageTransformer()]);
factory TransformConfiguration.forEditDraftsEmail() => TransformConfiguration.fromDomTransformers([
...TransformConfiguration.forDraftsEmail().domTransformers,
const HideDraftSignatureTransformer()]);
factory TransformConfiguration.forDraftsEmail() => TransformConfiguration.create(
customDomTransformers: [const ImageTransformer()]
);
factory TransformConfiguration.forEditDraftsEmail() => TransformConfiguration.create(
customDomTransformers: [
...TransformConfiguration.forDraftsEmail().domTransformers,
const HideDraftSignatureTransformer()
]
);

factory TransformConfiguration.forPreviewEmailOnWeb() => TransformConfiguration.create(
customDomTransformers: [
Expand All @@ -65,7 +72,9 @@ class TransformConfiguration {

factory TransformConfiguration.forPreviewEmail() => TransformConfiguration.standardConfiguration;

factory TransformConfiguration.forRestoreEmail() => TransformConfiguration.fromDomTransformers([const ImageTransformer()]);
factory TransformConfiguration.forRestoreEmail() => TransformConfiguration.create(
customDomTransformers: [const ImageTransformer()]
);

factory TransformConfiguration.forPrintEmail() => TransformConfiguration.fromDomTransformers([
if (PlatformInfo.isWeb)
Expand Down Expand Up @@ -115,6 +124,6 @@ class TransformConfiguration {
];

static const List<TextTransformer> standardTextTransformers = [
SanitizeAutolinkHtmlTransformers()
StandardizeHtmlSanitizingTransformers(),
];
}
9 changes: 9 additions & 0 deletions core/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,15 @@ packages:
url: "https://pub.dev"
source: hosted
version: "3.0.1"
sanitize_html:
dependency: "direct main"
description:
path: sanitize_html
ref: support_mail
resolved-ref: fda32cde4d4baadaa988477f498ab6622ee79987
url: "https://github.com/linagora/dart-neats.git"
source: git
version: "2.1.0"
shelf:
dependency: transitive
description:
Expand Down
9 changes: 9 additions & 0 deletions core/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ dependencies:
url: https://github.com/dab246/languagetool_textfield.git
ref: twake-supported

# Sanitize_html is restricting Tags and Attributes. So some of our own tags and attributes (signature, public asset,...) will be lost when sanitizing html.
# TODO: We will change it when the PR in upstream repository will be merged
# https://github.com/google/dart-neats/pull/259
dab246 marked this conversation as resolved.
Show resolved Hide resolved
sanitize_html:
git:
url: https://github.com/linagora/dart-neats.git
ref: support_mail
path: sanitize_html

### Dependencies from pub.dev ###
cupertino_icons: 1.0.6

Expand Down
52 changes: 52 additions & 0 deletions core/test/utils/standardize_html_sanitizing_transformers_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import 'package:core/presentation/utils/html_transformer/text/standardize_html_sanitizing_transformers.dart';
import 'package:flutter_test/flutter_test.dart';
import 'dart:convert';

void main() {
group('StandardizeHtmlSanitizingTransformers::test', () {
const transformer = StandardizeHtmlSanitizingTransformers();
const htmlEscape = HtmlEscape();

test('SHOULD remove attributes of IMG tag WHEN they are invalid', () {
const inputHtml = '<img src="1" href="1" onerror="javascript:alert(1)">';
hoangdat marked this conversation as resolved.
Show resolved Hide resolved
final result = transformer.process(inputHtml, htmlEscape);

expect(result, equals('<img src="1">'));
});

test('SHOULD remove all SCRIPTS tags', () {
const inputHtml = '<script>alert("This is an alert message!");</script>';
final result = transformer.process(inputHtml, htmlEscape).trim();

expect(result, equals(''));
});

test('SHOULD remove all IFRAME tags', () {
const inputHtml = '<iframe style="xg-p:absolute;top:0;left:0;width:100%;height:100%" onmouseover="prompt(1)">';
final result = transformer.process(inputHtml, htmlEscape);

expect(result, equals(''));
});

test('SHOULD remove href attribute of A tag WHEN it is invalid', () {
const inputHtml = '<a href="javascript:alert(1)" id="id1">test</a>';
final result = transformer.process(inputHtml, htmlEscape);

expect(result, equals('<a id="id1">test</a>'));
});

test('SHOULD persist value src attribute of IMG tag WHEN it is base64 string', () {
const inputHtml = '<img src="data:image/jpeg;base64,iVBORw0KGgoAAAANSUhEUgAAAAUA">';
final result = transformer.process(inputHtml, htmlEscape);

expect(result, equals('<img src="data:image/jpeg;base64,iVBORw0KGgoAAAANSUhEUgAAAAUA">'));
});

test('SHOULD persist value src attribute of IMG tag WHEN it is CID string', () {
const inputHtml = '<img src="cid:email123">';
final result = transformer.process(inputHtml, htmlEscape);

expect(result, equals('<img src="cid:email123">'));
});
});
}
21 changes: 21 additions & 0 deletions docs/adr/0053-standardize-html-sanitizing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# 53. Standardize HTML sanitizing

Date: 2024-10-24

## Status

Accepted

## Context

- The email content contains many unnecessary html attribute tags. We want to display the email content best on multiple platforms.

## Decision

- We sanitize the html before displaying the email.
- Customize the `sanitize html` library to allow `attributes`, `tags` and `className` to be used.
- The processing order is from `tags` -> `attributes` -> `className`

## Consequences

- Email content is cleaner. Displays well on all platforms. Any changes to sanitize html are updated here
17 changes: 17 additions & 0 deletions lib/features/base/upgradeable/upgrade_hive_database_steps_v12.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@

import 'package:tmail_ui_user/features/base/upgradeable/upgrade_database_steps.dart';
import 'package:tmail_ui_user/features/caching/caching_manager.dart';

class UpgradeHiveDatabaseStepsV12 extends UpgradeDatabaseSteps {

final CachingManager _cachingManager;

UpgradeHiveDatabaseStepsV12(this._cachingManager);

@override
Future<void> onUpgrade(int oldVersion, int newVersion) async {
if (oldVersion > 0 && oldVersion < newVersion && newVersion == 12) {
await _cachingManager.clearEmailCacheAndAllStateCache();
}
}
}
4 changes: 3 additions & 1 deletion lib/features/caching/caching_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,20 @@ class CachingManager {
return Future.wait([
_stateCacheClient.deleteItem(StateType.email.getTupleKeyStored(accountId, session.username)),
_emailCacheClient.clearAllData(),
if (PlatformInfo.isMobile) clearAllFileInStorage(),
], eagerError: true);
}

Future<void> clearEmailCacheAndAllStateCache() {
return Future.wait([
_stateCacheClient.clearAllData(),
_emailCacheClient.clearAllData(),
if (PlatformInfo.isMobile) clearAllFileInStorage(),
], eagerError: true);
}

Future<bool> storeCacheVersion(int newVersion) async {
log('CachingManager::storeCacheVersion()');
log('CachingManager::storeCacheVersion():newVersion = $newVersion');
return _hiveCacheVersionClient.storeVersion(newVersion);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/features/caching/config/cache_version.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

class CacheVersion {
static const int hiveDBVersion = 11;
static const int hiveDBVersion = 12;
}
2 changes: 2 additions & 0 deletions lib/features/caching/config/hive_cache_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:hive/hive.dart';
import 'package:path_provider/path_provider.dart' as path_provider;
import 'package:tmail_ui_user/features/base/upgradeable/upgrade_hive_database_steps_v10.dart';
import 'package:tmail_ui_user/features/base/upgradeable/upgrade_hive_database_steps_v11.dart';
import 'package:tmail_ui_user/features/base/upgradeable/upgrade_hive_database_steps_v12.dart';
import 'package:tmail_ui_user/features/base/upgradeable/upgrade_hive_database_steps_v7.dart';
import 'package:tmail_ui_user/features/caching/caching_manager.dart';
import 'package:tmail_ui_user/features/caching/config/cache_version.dart';
Expand Down Expand Up @@ -67,6 +68,7 @@ class HiveCacheConfig {
await UpgradeHiveDatabaseStepsV7(cachingManager).onUpgrade(oldVersion, newVersion);
await UpgradeHiveDatabaseStepsV10(cachingManager).onUpgrade(oldVersion, newVersion);
await UpgradeHiveDatabaseStepsV11(cachingManager).onUpgrade(oldVersion, newVersion);
await UpgradeHiveDatabaseStepsV12(cachingManager).onUpgrade(oldVersion, newVersion);

if (oldVersion != newVersion) {
await cachingManager.storeCacheVersion(newVersion);
Expand Down
5 changes: 4 additions & 1 deletion lib/features/email/data/local/html_analyzer.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:collection/collection.dart';
import 'package:core/data/constants/constant.dart';
import 'package:core/presentation/utils/html_transformer/html_transform.dart';
import 'package:core/presentation/utils/html_transformer/text/sanitize_autolink_html_transformers.dart';
import 'package:core/presentation/utils/html_transformer/transform_configuration.dart';
import 'package:core/utils/app_logger.dart';
import 'package:dartz/dartz.dart';
Expand Down Expand Up @@ -35,7 +36,9 @@ class HtmlAnalyzer {
case EmailContentType.textPlain:
final message = _htmlTransform.transformToTextPlain(
content: emailContent.content,
transformConfiguration: transformConfiguration
transformConfiguration: TransformConfiguration.fromTextTransformers([
const SanitizeAutolinkHtmlTransformers()
]),
);
return EmailContent(emailContent.type, message);
default:
Expand Down
Loading
Loading