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
2021-08-27 18:00:54 +02:00 · 2021-08-27 18:00:54 +02:00 · f3f9b219e1
parent b849c828e3
commit f3f9b219e1
5 changed files with 165 additions and 50 deletions
--- 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
--- 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,
  };
 }
--- 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>['●', '○', '■', '‣'];
--- 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': {
--- 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);