fix: apply review feedback

This commit is contained in:
Nicolas Werner 2021-09-29 16:36:49 +02:00
parent 08bbb3f6f5
commit 7dd176c278
6 changed files with 57 additions and 59 deletions

View File

@ -990,16 +990,12 @@ class RoomKeyRequest extends ToDeviceEvent {
final room = this.room; final room = this.room;
final session = await keyManager.loadInboundGroupSession( final session = await keyManager.loadInboundGroupSession(
room.id, request.sessionId, request.senderKey); room.id, request.sessionId, request.senderKey);
if (session == null) { if (session?.inboundGroupSession == null) {
Logs().v("[KeyManager] Not forwarding key we don't have");
return;
}
if (session.inboundGroupSession == null) {
Logs().v("[KeyManager] Not forwarding key we don't have"); Logs().v("[KeyManager] Not forwarding key we don't have");
return; return;
} }
final message = session.content.copy(); final message = session!.content.copy();
message['forwarding_curve25519_key_chain'] = message['forwarding_curve25519_key_chain'] =
List<String>.from(session.forwardingCurve25519KeyChain); List<String>.from(session.forwardingCurve25519KeyChain);

View File

@ -50,7 +50,7 @@ class OlmManager {
Map<String, List<OlmSession>> get olmSessions => _olmSessions; Map<String, List<OlmSession>> get olmSessions => _olmSessions;
final Map<String, List<OlmSession>> _olmSessions = {}; final Map<String, List<OlmSession>> _olmSessions = {};
// NOTE(Nico): Do we really want to create a new account on passing null instead of signing the user out? // NOTE(Nico): On initial login we pass null to create a new account
Future<void> init(String? olmAccount) async { Future<void> init(String? olmAccount) async {
if (olmAccount == null) { if (olmAccount == null) {
try { try {

View File

@ -810,7 +810,7 @@ class _KeyVerificationMethodSas extends _KeyVerificationMethod {
} }
Future<void> _sendAccept() async { Future<void> _sendAccept() async {
final sas = this.sas ??= olm.SAS(); final sas = this.sas = olm.SAS();
commitment = _makeCommitment(sas.get_pubkey(), startCanonicalJson); commitment = _makeCommitment(sas.get_pubkey(), startCanonicalJson);
await request.send(EventTypes.KeyVerificationAccept, { await request.send(EventTypes.KeyVerificationAccept, {
'method': type, 'method': type,

View File

@ -17,6 +17,7 @@
*/ */
import 'package:matrix/encryption/utils/stored_inbound_group_session.dart'; import 'package:matrix/encryption/utils/stored_inbound_group_session.dart';
import 'package:matrix_api_lite/src/utils/filter_map_extension.dart';
import 'package:olm/olm.dart' as olm; import 'package:olm/olm.dart' as olm;
import '../../matrix.dart'; import '../../matrix.dart';
@ -76,22 +77,23 @@ class SessionKey {
SessionKey.fromDb(StoredInboundGroupSession dbEntry, String key) SessionKey.fromDb(StoredInboundGroupSession dbEntry, String key)
: key = key, : key = key,
content = Event.getMapFromPayload(dbEntry.content), content = Event.getMapFromPayload(dbEntry.content),
indexes = indexes = Event.getMapFromPayload(dbEntry.indexes)
Map<String, String>.from(Event.getMapFromPayload(dbEntry.indexes)), .catchMap((k, v) => MapEntry(k, v as String)),
allowedAtIndex = Map<String, Map<String, int>>.from( allowedAtIndex = Event.getMapFromPayload(dbEntry.allowedAtIndex)
Event.getMapFromPayload(dbEntry.allowedAtIndex) .catchMap((k, v) => MapEntry(k, Map<String, int>.from(v))),
.map((k, v) => MapEntry(k, Map<String, int>.from(v)))),
roomId = dbEntry.roomId, roomId = dbEntry.roomId,
sessionId = dbEntry.sessionId, sessionId = dbEntry.sessionId,
senderKey = dbEntry.senderKey, senderKey = dbEntry.senderKey,
inboundGroupSession = olm.InboundGroupSession() { inboundGroupSession = olm.InboundGroupSession() {
final parsedSenderClaimedKeys = Map<String, String>.from( final parsedSenderClaimedKeys =
Event.getMapFromPayload(dbEntry.senderClaimedKeys)); Event.getMapFromPayload(dbEntry.senderClaimedKeys)
.catchMap((k, v) => MapEntry(k, v as String));
// we need to try...catch as the map used to be <String, int> and that will throw an error. // we need to try...catch as the map used to be <String, int> and that will throw an error.
senderClaimedKeys = (parsedSenderClaimedKeys.isNotEmpty) senderClaimedKeys = (parsedSenderClaimedKeys.isNotEmpty)
? parsedSenderClaimedKeys ? parsedSenderClaimedKeys
: (content['sender_claimed_keys'] is Map : (content['sender_claimed_keys'] is Map
? Map<String, String>.from(content['sender_claimed_keys']) ? content['sender_claimed_keys']
.catchMap((k, v) => MapEntry(k, v as String))
: (content['sender_claimed_ed25519_key'] is String : (content['sender_claimed_ed25519_key'] is String
? <String, String>{ ? <String, String>{
'ed25519': content['sender_claimed_ed25519_key'] 'ed25519': content['sender_claimed_ed25519_key']

View File

@ -98,10 +98,10 @@ class Event extends MatrixEvent {
this.roomId = roomId ?? room?.id; this.roomId = roomId ?? room?.id;
this.senderId = senderId; this.senderId = senderId;
this.unsigned = unsigned; this.unsigned = unsigned;
// synapse unfortunatley isn't following the spec and tosses the prev_content // synapse unfortunatley isn't following the spec and tosses the prev_content
// into the unsigned block. // into the unsigned block.
// Currently we are facing a very strange bug in web which is impossible to debug. // Currently we are facing a very strange bug in web which is impossible to debug.
// It may be because of this line so we put this in try-catch until we can fix it. // It may be because of this line so we put this in try-catch until we can fix it.
try { try {
this.prevContent = (prevContent != null && prevContent.isNotEmpty) this.prevContent = (prevContent != null && prevContent.isNotEmpty)
? prevContent ? prevContent
@ -111,21 +111,21 @@ class Event extends MatrixEvent {
? unsigned['prev_content'] ? unsigned['prev_content']
: null; : null;
} catch (_) { } catch (_) {
// A strange bug in dart web makes this crash // A strange bug in dart web makes this crash
} }
this.stateKey = stateKey; this.stateKey = stateKey;
this.originServerTs = originServerTs; this.originServerTs = originServerTs;
// Mark event as failed to send if status is `sending` and event is older // Mark event as failed to send if status is `sending` and event is older
// than the timeout. This should not happen with the deprecated Moor // than the timeout. This should not happen with the deprecated Moor
// database! // database!
if (status == 0 && room?.client?.database != null) { if (status == 0 && room?.client?.database != null) {
// Age of this event in milliseconds // Age of this event in milliseconds
final age = DateTime.now().millisecondsSinceEpoch - final age = DateTime.now().millisecondsSinceEpoch -
originServerTs.millisecondsSinceEpoch; originServerTs.millisecondsSinceEpoch;
if (age > room.client.sendMessageTimeoutSeconds * 1000) { if (age > room.client.sendMessageTimeoutSeconds * 1000) {
// Update this event in database and open timelines // Update this event in database and open timelines
final json = toJson(); final json = toJson();
json['unsigned'] ??= <String, dynamic>{}; json['unsigned'] ??= <String, dynamic>{};
json['unsigned'][messageSendingStatusKey] = -1; json['unsigned'][messageSendingStatusKey] = -1;
@ -324,8 +324,8 @@ class Event extends MatrixEvent {
/// Try to send this event again. Only works with events of status -1. /// Try to send this event again. Only works with events of status -1.
Future<String> sendAgain({String txid}) async { Future<String> sendAgain({String txid}) async {
if (status != -1) return null; if (status != -1) return null;
// we do not remove the event here. It will automatically be updated // we do not remove the event here. It will automatically be updated
// in the `sendEvent` method to transition -1 -> 0 -> 1 -> 2 // in the `sendEvent` method to transition -1 -> 0 -> 1 -> 2
final newEventId = await room.sendEvent( final newEventId = await room.sendEvent(
content, content,
txid: txid ?? unsigned['transaction_id'] ?? eventId, txid: txid ?? unsigned['transaction_id'] ?? eventId,
@ -420,7 +420,7 @@ class Event extends MatrixEvent {
return getThumbnail ? thumbnailMxcUrl : attachmentMxcUrl; return getThumbnail ? thumbnailMxcUrl : attachmentMxcUrl;
} }
// size determined from an approximate 800x800 jpeg thumbnail with method=scale // size determined from an approximate 800x800 jpeg thumbnail with method=scale
static const _minNoThumbSize = 80 * 1024; static const _minNoThumbSize = 80 * 1024;
/// Gets the attachment https URL to display in the timeline, taking into account if the original image is tiny. /// Gets the attachment https URL to display in the timeline, taking into account if the original image is tiny.
@ -449,14 +449,14 @@ class Event extends MatrixEvent {
final thisInfoMap = useThumbnailMxcUrl ? thumbnailInfoMap : infoMap; final thisInfoMap = useThumbnailMxcUrl ? thumbnailInfoMap : infoMap;
final thisMxcUrl = final thisMxcUrl =
useThumbnailMxcUrl ? infoMap['thumbnail_url'] : content['url']; useThumbnailMxcUrl ? infoMap['thumbnail_url'] : content['url'];
// if we have as method scale, we can return safely the original image, should it be small enough // if we have as method scale, we can return safely the original image, should it be small enough
if (getThumbnail && if (getThumbnail &&
method == ThumbnailMethod.scale && method == ThumbnailMethod.scale &&
thisInfoMap['size'] is int && thisInfoMap['size'] is int &&
thisInfoMap['size'] < minNoThumbSize) { thisInfoMap['size'] < minNoThumbSize) {
getThumbnail = false; getThumbnail = false;
} }
// now generate the actual URLs // now generate the actual URLs
if (getThumbnail) { if (getThumbnail) {
return Uri.parse(thisMxcUrl).getThumbnail( return Uri.parse(thisMxcUrl).getThumbnail(
room.client, room.client,
@ -480,7 +480,7 @@ class Event extends MatrixEvent {
throw "This event hasn't any attachment or thumbnail."; throw "This event hasn't any attachment or thumbnail.";
} }
getThumbnail = mxcUrl != attachmentMxcUrl; getThumbnail = mxcUrl != attachmentMxcUrl;
// Is this file storeable? // Is this file storeable?
final thisInfoMap = getThumbnail ? thumbnailInfoMap : infoMap; final thisInfoMap = getThumbnail ? thumbnailInfoMap : infoMap;
final storeable = room.client.database != null && final storeable = room.client.database != null &&
thisInfoMap['size'] is int && thisInfoMap['size'] is int &&
@ -517,7 +517,7 @@ class Event extends MatrixEvent {
Uint8List uint8list; Uint8List uint8list;
// Is this file storeable? // Is this file storeable?
final thisInfoMap = getThumbnail ? thumbnailInfoMap : infoMap; final thisInfoMap = getThumbnail ? thumbnailInfoMap : infoMap;
var storeable = room.client.database != null && var storeable = room.client.database != null &&
thisInfoMap['size'] is int && thisInfoMap['size'] is int &&
@ -527,7 +527,7 @@ class Event extends MatrixEvent {
uint8list = await room.client.database.getFile(mxcUrl); uint8list = await room.client.database.getFile(mxcUrl);
} }
// Download the file // Download the file
if (uint8list == null) { if (uint8list == null) {
downloadCallback ??= (Uri url) async { downloadCallback ??= (Uri url) async {
return (await http.get(url)).bodyBytes; return (await http.get(url)).bodyBytes;
@ -541,7 +541,7 @@ class Event extends MatrixEvent {
} }
} }
// Decrypt the file // Decrypt the file
if (isEncrypted) { if (isEncrypted) {
final fileMap = final fileMap =
getThumbnail ? infoMap['thumbnail_file'] : content['file']; getThumbnail ? infoMap['thumbnail_file'] : content['file'];
@ -579,10 +579,10 @@ class Event extends MatrixEvent {
} }
var body = plaintextBody ? this.plaintextBody : this.body; var body = plaintextBody ? this.plaintextBody : this.body;
// we need to know if the message is an html message to be able to determine // we need to know if the message is an html message to be able to determine
// if we need to strip the reply fallback. // if we need to strip the reply fallback.
var htmlMessage = content['format'] != 'org.matrix.custom.html'; var htmlMessage = content['format'] != 'org.matrix.custom.html';
// If we have an edit, we want to operate on the new content // If we have an edit, we want to operate on the new content
if (hideEdit && if (hideEdit &&
relationshipType == RelationshipTypes.edit && relationshipType == RelationshipTypes.edit &&
content.tryGet<Map<String, dynamic>>('m.new_content') != null) { content.tryGet<Map<String, dynamic>>('m.new_content') != null) {
@ -600,9 +600,9 @@ class Event extends MatrixEvent {
body; body;
} }
} }
// Hide reply fallback // Hide reply fallback
// Be sure that the plaintextBody already stripped teh reply fallback, // Be sure that the plaintextBody already stripped teh reply fallback,
// if the message is formatted // if the message is formatted
if (hideReply && (!plaintextBody || htmlMessage)) { if (hideReply && (!plaintextBody || htmlMessage)) {
body = body.replaceFirst( body = body.replaceFirst(
RegExp(r'^>( \*)? <[^>]+>[^\n\r]+\r?\n(> [^\n]*\r?\n)*\r?\n'), ''); RegExp(r'^>( \*)? <[^>]+>[^\n\r]+\r?\n(> [^\n]*\r?\n)*\r?\n'), '');
@ -613,7 +613,7 @@ class Event extends MatrixEvent {
localizedBody = callback(this, i18n, body); localizedBody = callback(this, i18n, body);
} }
// Add the sender name prefix // Add the sender name prefix
if (withSenderNamePrefix && if (withSenderNamePrefix &&
type == EventTypes.Message && type == EventTypes.Message &&
textOnlyMessageTypes.contains(messageType)) { textOnlyMessageTypes.contains(messageType)) {
@ -691,13 +691,13 @@ class Event extends MatrixEvent {
return this; return this;
} }
if (hasAggregatedEvents(timeline, RelationshipTypes.edit)) { if (hasAggregatedEvents(timeline, RelationshipTypes.edit)) {
// alright, we have an edit // alright, we have an edit
final allEditEvents = aggregatedEvents(timeline, RelationshipTypes.edit) final allEditEvents = aggregatedEvents(timeline, RelationshipTypes.edit)
// we only allow edits made by the original author themself // we only allow edits made by the original author themself
.where((e) => e.senderId == senderId && e.type == EventTypes.Message) .where((e) => e.senderId == senderId && e.type == EventTypes.Message)
.toList(); .toList();
// we need to check again if it isn't empty, as we potentially removed all // we need to check again if it isn't empty, as we potentially removed all
// aggregated edits // aggregated edits
if (allEditEvents.isNotEmpty) { if (allEditEvents.isNotEmpty) {
allEditEvents.sort((a, b) => a.originServerTs.millisecondsSinceEpoch - allEditEvents.sort((a, b) => a.originServerTs.millisecondsSinceEpoch -
b.originServerTs.millisecondsSinceEpoch > b.originServerTs.millisecondsSinceEpoch >
@ -705,7 +705,7 @@ class Event extends MatrixEvent {
? 1 ? 1
: -1); : -1);
final rawEvent = allEditEvents.last.toJson(); final rawEvent = allEditEvents.last.toJson();
// update the content of the new event to render // update the content of the new event to render
if (rawEvent['content']['m.new_content'] is Map) { if (rawEvent['content']['m.new_content'] is Map) {
rawEvent['content'] = rawEvent['content']['m.new_content']; rawEvent['content'] = rawEvent['content']['m.new_content'];
} }
@ -720,16 +720,16 @@ class Event extends MatrixEvent {
content['format'] == 'org.matrix.custom.html' && content['format'] == 'org.matrix.custom.html' &&
content['formatted_body'] is String; content['formatted_body'] is String;
// regexes to fetch the number of emotes, including emoji, and if the message consists of only those // regexes to fetch the number of emotes, including emoji, and if the message consists of only those
// to match an emoji we can use the following regex: // to match an emoji we can use the following regex:
// (?:\x{00a9}|\x{00ae}|[\x{2600}-\x{27bf}]|[\x{2b00}-\x{2bff}]|\x{d83c}[\x{d000}-\x{dfff}]|\x{d83d}[\x{d000}-\x{dfff}]|\x{d83e}[\x{d000}-\x{dfff}])[\x{fe00}-\x{fe0f}]? // (?:\x{00a9}|\x{00ae}|[\x{2600}-\x{27bf}]|[\x{2b00}-\x{2bff}]|\x{d83c}[\x{d000}-\x{dfff}]|\x{d83d}[\x{d000}-\x{dfff}]|\x{d83e}[\x{d000}-\x{dfff}])[\x{fe00}-\x{fe0f}]?
// we need to replace \x{0000} with \u0000, the comment is left in the other format to be able to paste into regex101.com // we need to replace \x{0000} with \u0000, the comment is left in the other format to be able to paste into regex101.com
// to see if there is a custom emote, we use the following regex: <img[^>]+data-mx-(?:emote|emoticon)(?==|>|\s)[^>]*> // to see if there is a custom emote, we use the following regex: <img[^>]+data-mx-(?:emote|emoticon)(?==|>|\s)[^>]*>
// now we combind the two to have four regexes: // now we combind the two to have four regexes:
// 1. are there only emoji, or whitespace // 1. are there only emoji, or whitespace
// 2. are there only emoji, emotes, or whitespace // 2. are there only emoji, emotes, or whitespace
// 3. count number of emoji // 3. count number of emoji
// 4- count number of emoji or emotes // 4- count number of emoji or emotes
static final RegExp _onlyEmojiRegex = RegExp( static final RegExp _onlyEmojiRegex = RegExp(
r'^((?:\u00a9|\u00ae|[\u2600-\u27bf]|[\u2b00-\u2bff]|\ud83c[\ud000-\udfff]|\ud83d[\ud000-\udfff]|\ud83e[\ud000-\udfff])[\ufe00-\ufe0f]?|\s)*$', r'^((?:\u00a9|\u00ae|[\u2600-\u27bf]|[\u2b00-\u2bff]|\ud83c[\ud000-\udfff]|\ud83d[\ud000-\udfff]|\ud83e[\ud000-\udfff])[\ufe00-\ufe0f]?|\s)*$',
caseSensitive: false, caseSensitive: false,

View File

@ -183,7 +183,7 @@ abstract class SignableKey extends MatrixSignableKey {
return String.fromCharCodes(canonicalJson.encode(data)); return String.fromCharCodes(canonicalJson.encode(data));
} }
bool _verifySignature(String /*!*/ pubKey, String /*!*/ signature, bool _verifySignature(String pubKey, String signature,
{bool isSignatureWithoutLibolmValid = false}) { {bool isSignatureWithoutLibolmValid = false}) {
olm.Utility olmutil; olm.Utility olmutil;
try { try {
@ -321,7 +321,7 @@ abstract class SignableKey extends MatrixSignableKey {
} }
} }
Future<void> /*!*/ setBlocked(bool newBlocked); Future<void> setBlocked(bool newBlocked);
@override @override
Map<String, dynamic> toJson() { Map<String, dynamic> toJson() {
@ -416,7 +416,7 @@ class DeviceKeys extends SignableKey {
_validSelfSignature ?? _validSelfSignature ??
(_validSelfSignature = (deviceId != null && (_validSelfSignature = (deviceId != null &&
signatures signatures
?.tryGet<Map<String, dynamic>>(userId) ?.tryGetMap<String, dynamic>(userId)
?.tryGet<String>('ed25519:$deviceId') == ?.tryGet<String>('ed25519:$deviceId') ==
null null
? false ? false