From f3f9b219e166095325bd2e3e5fc0acfcb70ec72d Mon Sep 17 00:00:00 2001
From: Sorunome <mail@sorunome.de>
Date: Fri, 27 Aug 2021 18:00:54 +0200
Subject: [PATCH] feat: Cleanup Event.plaintextBody and add [plaintextBody] to
 Event.getLocalizedBody It appears that [hideEdit] in Event.getLocalizedBody
 was written in a way that it assumes a valid event body. This was also fixed,
 while also adding tests for the various parameters of Event.getLocalizedBody

---
 lib/src/event.dart                     | 47 +++++++++++-----
 lib/src/utils/event_localizations.dart | 64 ++++++++++++----------
 lib/src/utils/html_to_text.dart        | 27 +++++++---
 test/event_test.dart                   | 75 ++++++++++++++++++++++++++
 test/html_to_text_test.dart            |  2 +
 5 files changed, 165 insertions(+), 50 deletions(-)

diff --git a/lib/src/event.dart b/lib/src/event.dart
index 44396bc7..7e0b67d4 100644
--- a/lib/src/event.dart
+++ b/lib/src/event.dart
@@ -568,31 +568,52 @@ class Event extends MatrixEvent {
 
   /// Returns a localized String representation of this event. For a
   /// room list you may find [withSenderNamePrefix] useful. Set [hideReply] to
-  /// crop all lines starting with '>'.
+  /// crop all lines starting with '>'. With [plaintextBody] it'll use the
+  /// plaintextBody instead of the normal body.
   String getLocalizedBody(
     MatrixLocalizations i18n, {
     bool withSenderNamePrefix = false,
     bool hideReply = false,
     bool hideEdit = false,
+    bool plaintextBody = false,
   }) {
     if (redacted) {
       return i18n.removedBy(redactedBecause.sender.calcDisplayname());
     }
+    var body = plaintextBody ? this.plaintextBody : this.body;
+
+    // we need to know if the message is an html message to be able to determine
+    // if we need to strip the reply fallback.
+    var htmlMessage = content['format'] != 'org.matrix.custom.html';
+    // If we have an edit, we want to operate on the new content
+    if (hideEdit &&
+        relationshipType == RelationshipTypes.edit &&
+        content.tryGet<Map<String, dynamic>>('m.new_content') != null) {
+      if (plaintextBody &&
+          content['m.new_content']['format'] == 'org.matrix.custom.html') {
+        htmlMessage = true;
+        body = HtmlToText.convert(
+            (content['m.new_content'] as Map<String, dynamic>)
+                    .tryGet<String>('formatted_body') ??
+                formattedText);
+      } else {
+        htmlMessage = false;
+        body = (content['m.new_content'] as Map<String, dynamic>)
+                .tryGet<String>('body') ??
+            body;
+      }
+    }
+    // Hide reply fallback
+    // Be sure that the plaintextBody already stripped teh reply fallback,
+    // if the message is formatted
+    if (hideReply && (!plaintextBody || htmlMessage)) {
+      body = body.replaceFirst(
+          RegExp(r'^>( \*)? <[^>]+>[^\n\r]+\r?\n(> [^\n]*\r?\n)*\r?\n'), '');
+    }
     final callback = EventLocalizations.localizationsMap[type];
     var localizedBody = i18n.unknownEvent(type);
     if (callback != null) {
-      localizedBody = callback(this, i18n);
-    }
-    // Hide reply fallback
-    if (hideReply) {
-      localizedBody = localizedBody.replaceFirst(
-          RegExp(r'^>( \*)? <[^>]+>[^\n\r]+\r?\n(> [^\n]*\r?\n)*\r?\n'), '');
-    }
-    // Hide edit fallback
-    if (hideEdit &&
-        relationshipType == RelationshipTypes.edit &&
-        content.containsKey('m.new_content')) {
-      localizedBody = content['m.new_content']['body'] ?? localizedBody;
+      localizedBody = callback(this, i18n, body);
     }
 
     // Add the sender name prefix
diff --git a/lib/src/utils/event_localizations.dart b/lib/src/utils/event_localizations.dart
index b5cd7a1f..2a9ec34b 100644
--- a/lib/src/utils/event_localizations.dart
+++ b/lib/src/utils/event_localizations.dart
@@ -24,8 +24,13 @@ import '../room.dart';
 import 'matrix_localizations.dart';
 
 abstract class EventLocalizations {
+  // As we need to create the localized body off of a different set of parameters, we
+  // might create it with `event.plaintextBody`, maybe with `event.body`, maybe with the
+  // reply fallback stripped, and maybe with the new body in `event.content['m.new_content']`.
+  // Thus, it seems easier to offload that logic into `Event.getLocalizedBody()` and pass the
+  // `body` variable around here.
   static String _localizedBodyNormalMessage(
-      Event event, MatrixLocalizations i18n) {
+      Event event, MatrixLocalizations i18n, String body) {
     switch (event.messageType) {
       case MessageTypes.Image:
         return i18n.sentAPicture(event.sender.calcDisplayname());
@@ -40,7 +45,7 @@ abstract class EventLocalizations {
       case MessageTypes.Sticker:
         return i18n.sentASticker(event.sender.calcDisplayname());
       case MessageTypes.Emote:
-        return '* ${event.body}';
+        return '* $body';
       case MessageTypes.BadEncrypted:
         String errorText;
         switch (event.body) {
@@ -57,7 +62,7 @@ abstract class EventLocalizations {
             errorText = i18n.noPermission + '.';
             break;
           default:
-            errorText = event.body;
+            errorText = body;
             break;
         }
         return i18n.couldNotDecryptMessage(errorText);
@@ -65,27 +70,27 @@ abstract class EventLocalizations {
       case MessageTypes.Notice:
       case MessageTypes.None:
       default:
-        return event.body;
+        return body;
     }
   }
 
   // This map holds how to localize event types, and thus which event types exist.
   // If an event exists but it does not have a localized body, set its callback to null
   static final Map<String,
-          String Function(Event event, MatrixLocalizations i18n)>
+          String Function(Event event, MatrixLocalizations i18n, String body)>
       localizationsMap = {
-    EventTypes.Sticker: (event, i18n) =>
+    EventTypes.Sticker: (event, i18n, body) =>
         i18n.sentASticker(event.sender.calcDisplayname()),
-    EventTypes.Redaction: (event, i18n) =>
+    EventTypes.Redaction: (event, i18n, body) =>
         i18n.redactedAnEvent(event.sender.calcDisplayname()),
-    EventTypes.RoomAliases: (event, i18n) =>
+    EventTypes.RoomAliases: (event, i18n, body) =>
         i18n.changedTheRoomAliases(event.sender.calcDisplayname()),
-    EventTypes.RoomCanonicalAlias: (event, i18n) =>
+    EventTypes.RoomCanonicalAlias: (event, i18n, body) =>
         i18n.changedTheRoomInvitationLink(event.sender.calcDisplayname()),
-    EventTypes.RoomCreate: (event, i18n) =>
+    EventTypes.RoomCreate: (event, i18n, body) =>
         i18n.createdTheChat(event.sender.calcDisplayname()),
-    EventTypes.RoomTombstone: (event, i18n) => i18n.roomHasBeenUpgraded,
-    EventTypes.RoomJoinRules: (event, i18n) {
+    EventTypes.RoomTombstone: (event, i18n, body) => i18n.roomHasBeenUpgraded,
+    EventTypes.RoomJoinRules: (event, i18n, body) {
       final joinRules = JoinRules.values.firstWhere(
           (r) =>
               r.toString().replaceAll('JoinRules.', '') ==
@@ -98,7 +103,7 @@ abstract class EventLocalizations {
             event.sender.calcDisplayname(), joinRules.getLocalizedString(i18n));
       }
     },
-    EventTypes.RoomMember: (event, i18n) {
+    EventTypes.RoomMember: (event, i18n, body) {
       var text = 'Failed to parse member event';
       final targetName = event.stateKeyUser.calcDisplayname();
       // Has the membership changed?
@@ -162,15 +167,16 @@ abstract class EventLocalizations {
       }
       return text;
     },
-    EventTypes.RoomPowerLevels: (event, i18n) =>
+    EventTypes.RoomPowerLevels: (event, i18n, body) =>
         i18n.changedTheChatPermissions(event.sender.calcDisplayname()),
-    EventTypes.RoomName: (event, i18n) => i18n.changedTheChatNameTo(
+    EventTypes.RoomName: (event, i18n, body) => i18n.changedTheChatNameTo(
         event.sender.calcDisplayname(), event.content['name']),
-    EventTypes.RoomTopic: (event, i18n) => i18n.changedTheChatDescriptionTo(
-        event.sender.calcDisplayname(), event.content['topic']),
-    EventTypes.RoomAvatar: (event, i18n) =>
+    EventTypes.RoomTopic: (event, i18n, body) =>
+        i18n.changedTheChatDescriptionTo(
+            event.sender.calcDisplayname(), event.content['topic']),
+    EventTypes.RoomAvatar: (event, i18n, body) =>
         i18n.changedTheChatAvatar(event.sender.calcDisplayname()),
-    EventTypes.GuestAccess: (event, i18n) {
+    EventTypes.GuestAccess: (event, i18n, body) {
       final guestAccess = GuestAccess.values.firstWhere(
           (r) =>
               r.toString().replaceAll('GuestAccess.', '') ==
@@ -183,7 +189,7 @@ abstract class EventLocalizations {
             guestAccess.getLocalizedString(i18n));
       }
     },
-    EventTypes.HistoryVisibility: (event, i18n) {
+    EventTypes.HistoryVisibility: (event, i18n, body) {
       final historyVisibility = HistoryVisibility.values.firstWhere(
           (r) =>
               r.toString().replaceAll('HistoryVisibility.', '') ==
@@ -197,7 +203,7 @@ abstract class EventLocalizations {
             historyVisibility.getLocalizedString(i18n));
       }
     },
-    EventTypes.Encryption: (event, i18n) {
+    EventTypes.Encryption: (event, i18n, body) {
       var localizedBody =
           i18n.activatedEndToEndEncryption(event.sender.calcDisplayname());
       if (!event.room.client.encryptionEnabled) {
@@ -205,18 +211,18 @@ abstract class EventLocalizations {
       }
       return localizedBody;
     },
-    EventTypes.CallAnswer: (event, i18n) =>
+    EventTypes.CallAnswer: (event, i18n, body) =>
         i18n.answeredTheCall(event.sender.calcDisplayname()),
-    EventTypes.CallHangup: (event, i18n) =>
+    EventTypes.CallHangup: (event, i18n, body) =>
         i18n.endedTheCall(event.sender.calcDisplayname()),
-    EventTypes.CallInvite: (event, i18n) =>
+    EventTypes.CallInvite: (event, i18n, body) =>
         i18n.startedACall(event.sender.calcDisplayname()),
-    EventTypes.CallCandidates: (event, i18n) =>
+    EventTypes.CallCandidates: (event, i18n, body) =>
         i18n.sentCallInformations(event.sender.calcDisplayname()),
-    EventTypes.Encrypted: (event, i18n) =>
-        _localizedBodyNormalMessage(event, i18n),
-    EventTypes.Message: (event, i18n) =>
-        _localizedBodyNormalMessage(event, i18n),
+    EventTypes.Encrypted: (event, i18n, body) =>
+        _localizedBodyNormalMessage(event, i18n, body),
+    EventTypes.Message: (event, i18n, body) =>
+        _localizedBodyNormalMessage(event, i18n, body),
     EventTypes.Reaction: null,
   };
 }
diff --git a/lib/src/utils/html_to_text.dart b/lib/src/utils/html_to_text.dart
index 6bd0b1c6..dcdabdcc 100644
--- a/lib/src/utils/html_to_text.dart
+++ b/lib/src/utils/html_to_text.dart
@@ -25,8 +25,19 @@ class HtmlToText {
   /// Convert an HTML string to a pseudo-markdown plain text representation, with
   /// `data-mx-spoiler` spans redacted
   static String convert(String html) {
+    // riot-web is notorious for creating bad reply fallback events from invalid messages which, if
+    // not handled properly, can lead to impersonation. As such, we strip the entire `<mx-reply>` tags
+    // here already, to prevent that from happening.
+    // We do *not* do this in an AST and just with simple regex here, as riot-web tends to create
+    // miss-matching tags, and this way we actually correctly identify what we want to strip and, well,
+    // strip it.
+    final renderHtml = html.replaceAll(
+        RegExp('<mx-reply>.*<\/mx-reply>',
+            caseSensitive: false, multiLine: false, dotAll: true),
+        '');
+
     final opts = _ConvertOpts();
-    var reply = _walkNode(opts, parseFragment(html));
+    var reply = _walkNode(opts, parseFragment(renderHtml));
     reply = reply.replaceAll(RegExp(r'\s*$', multiLine: false), '');
     return reply;
   }
@@ -105,19 +116,19 @@ class HtmlToText {
     opts.listDepth++;
     final entries = _listChildNodes(opts, node, {'li'});
     opts.listDepth--;
-    var entry = 0;
+    var entry = 1;
     if (node.attributes['start'] is String &&
         RegExp(r'^[0-9]+$', multiLine: false)
             .hasMatch(node.attributes['start'])) {
       entry = int.parse(node.attributes['start']);
     }
 
-    return entries.map((s) {
-      entry++;
-      return ('    ' * opts.listDepth) +
-          '$entry. ' +
-          s.replaceAll('\n', '\n' + ('    ' * opts.listDepth) + '  ');
-    }).join('\n');
+    return entries
+        .map((s) =>
+            ('    ' * opts.listDepth) +
+            '${entry++}. ' +
+            s.replaceAll('\n', '\n' + ('    ' * opts.listDepth) + '  '))
+        .join('\n');
   }
 
   static const _listBulletPoints = <String>['●', '○', '■', '‣'];
diff --git a/test/event_test.dart b/test/event_test.dart
index 7349c60a..24040bdd 100644
--- a/test/event_test.dart
+++ b/test/event_test.dart
@@ -895,6 +895,81 @@ void main() {
       expect(event.isEventTypeKnown, false);
     });
 
+    test('getLocalizedBody, parameters', () {
+      final matrix = Client('testclient', httpClient: FakeMatrixApi());
+      final room = Room(id: '!1234:example.com', client: matrix);
+      var event = Event.fromJson({
+        'content': {
+          'body': 'This is an example text message',
+          'format': 'org.matrix.custom.html',
+          'formatted_body': '<b>This is an example text message</b>',
+          'msgtype': 'm.text'
+        },
+        'event_id': '\$143273582443PhrSn:example.org',
+        'origin_server_ts': 1432735824653,
+        'room_id': '!jEsUZKDJdhlrceRyVU:example.org',
+        'sender': '@example:example.org',
+        'type': 'm.room.message',
+        'unsigned': {'age': 1234}
+      }, room);
+      expect(
+          event.getLocalizedBody(FakeMatrixLocalizations(),
+              plaintextBody: true),
+          '**This is an example text message**');
+
+      event = Event.fromJson({
+        'content': {
+          'body': '* This is an example text message',
+          'format': 'org.matrix.custom.html',
+          'formatted_body': '* <b>This is an example text message</b>',
+          'msgtype': 'm.text',
+          'm.relates_to': <String, dynamic>{
+            'rel_type': 'm.replace',
+            'event_id': '\$some_event',
+          },
+          'm.new_content': <String, dynamic>{
+            'body': 'This is an example text message',
+            'format': 'org.matrix.custom.html',
+            'formatted_body': '<b>This is an example text message</b>',
+            'msgtype': 'm.text'
+          },
+        },
+        'event_id': '\$143273582443PhrSn:example.org',
+        'origin_server_ts': 1432735824653,
+        'room_id': '!jEsUZKDJdhlrceRyVU:example.org',
+        'sender': '@example:example.org',
+        'type': 'm.room.message',
+        'unsigned': {'age': 1234}
+      }, room);
+      expect(event.getLocalizedBody(FakeMatrixLocalizations(), hideEdit: true),
+          'This is an example text message');
+      expect(
+          event.getLocalizedBody(FakeMatrixLocalizations(),
+              hideEdit: true, plaintextBody: true),
+          '**This is an example text message**');
+
+      event = Event.fromJson({
+        'content': {
+          'body': '> <@user:example.org> beep\n\nhmm, fox',
+          'format': 'org.matrix.custom.html',
+          'formatted_body': '<mx-reply>beep</mx-reply>hmm, <em>fox</em>',
+          'msgtype': 'm.text'
+        },
+        'event_id': '\$143273582443PhrSn:example.org',
+        'origin_server_ts': 1432735824653,
+        'room_id': '!jEsUZKDJdhlrceRyVU:example.org',
+        'sender': '@example:example.org',
+        'type': 'm.room.message',
+        'unsigned': {'age': 1234}
+      }, room);
+      expect(event.getLocalizedBody(FakeMatrixLocalizations(), hideReply: true),
+          'hmm, fox');
+      expect(
+          event.getLocalizedBody(FakeMatrixLocalizations(),
+              hideReply: true, plaintextBody: true),
+          'hmm, *fox*');
+    });
+
     test('aggregations', () {
       final event = Event.fromJson({
         'content': {
diff --git a/test/html_to_text_test.dart b/test/html_to_text_test.dart
index 480368cf..2e142da7 100644
--- a/test/html_to_text_test.dart
+++ b/test/html_to_text_test.dart
@@ -70,6 +70,7 @@ void main() {
         '<ul><li>hey<ul><li>a</li><li>b</li></ul></li><li>foxies</li></ul>':
             '● hey\n      ○ a\n      ○ b\n● foxies',
         '<ol><li>a</li><li>b</li></ol>': '1. a\n2. b',
+        '<ol start="42"><li>a</li><li>b</li></ol>': '42. a\n43. b',
         '<ol><li>a<ol><li>aa</li><li>bb</li></ol></li><li>b</li></ol>':
             '1. a\n      1. aa\n      2. bb\n2. b',
         '<ol><li>a<ul><li>aa</li><li>bb</li></ul></li><li>b</li></ol>':
@@ -90,6 +91,7 @@ void main() {
         '<h6>fox</h6>': '###### fox',
         '<span>fox</span>': 'fox',
         '<p>fox</p>\n<p>floof</p>': 'fox\n\nfloof',
+        '<mx-reply>beep</mx-reply><p>fox</p>\n<p>floof</p>': 'fox\n\nfloof',
       };
       for (final entry in testMap.entries) {
         expect(HtmlToText.convert(entry.key), entry.value);