diff --git a/build.yaml b/build.yaml index 3aa354f58d..c68166ae08 100644 --- a/build.yaml +++ b/build.yaml @@ -4,5 +4,6 @@ targets: source_gen:combining_builder: options: ignore_for_file: + - constant_identifier_names # https://github.com/google/json_serializable.dart/issues/625 - unnecessary_cast diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 8424ac3315..b5c20aafd9 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'events.dart'; diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index 66af6d685c..a7b51a0181 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'initial_snapshot.dart'; diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index b4434a4a43..4a9fcb7640 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'model.dart'; diff --git a/lib/api/model/reaction.g.dart b/lib/api/model/reaction.g.dart index 4fdee8e00a..97fc0023a2 100644 --- a/lib/api/model/reaction.g.dart +++ b/lib/api/model/reaction.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'reaction.dart'; diff --git a/lib/api/route/account.g.dart b/lib/api/route/account.g.dart index 074c8aae74..d246d5a0f1 100644 --- a/lib/api/route/account.g.dart +++ b/lib/api/route/account.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'account.dart'; diff --git a/lib/api/route/events.g.dart b/lib/api/route/events.g.dart index b35cebabfa..e452021510 100644 --- a/lib/api/route/events.g.dart +++ b/lib/api/route/events.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'events.dart'; diff --git a/lib/api/route/messages.g.dart b/lib/api/route/messages.g.dart index c0f32708a6..6c34be86d7 100644 --- a/lib/api/route/messages.g.dart +++ b/lib/api/route/messages.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'messages.dart'; diff --git a/lib/api/route/realm.g.dart b/lib/api/route/realm.g.dart index a7bea0c493..506ffa83ca 100644 --- a/lib/api/route/realm.g.dart +++ b/lib/api/route/realm.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'realm.dart'; diff --git a/lib/api/route/users.g.dart b/lib/api/route/users.g.dart index a85125a50d..8d9b94ca5f 100644 --- a/lib/api/route/users.g.dart +++ b/lib/api/route/users.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'users.dart'; diff --git a/lib/model/compose.dart b/lib/model/compose.dart index cec844e162..ec8fdcb3dd 100644 --- a/lib/model/compose.dart +++ b/lib/model/compose.dart @@ -1,7 +1,7 @@ import 'dart:math'; import '../api/model/model.dart'; -import '../api/model/narrow.dart'; +import 'internal_link.dart'; import 'narrow.dart'; import 'store.dart'; @@ -101,82 +101,6 @@ String wrapWithBacktickFence({required String content, String? infoString}) { return resultBuffer.toString(); } -const _hashReplacements = { - "%": ".", - "(": ".28", - ")": ".29", - ".": ".2E", -}; - -final _encodeHashComponentRegex = RegExp(r'[%().]'); - -// Corresponds to encodeHashComponent in Zulip web; -// see web/shared/src/internal_url.ts. -String _encodeHashComponent(String str) { - return Uri.encodeComponent(str) - .replaceAllMapped(_encodeHashComponentRegex, (Match m) => _hashReplacements[m[0]!]!); -} - -/// A URL to the given [Narrow], on `store`'s realm. -/// -/// To include /near/{messageId} in the link, pass a non-null [nearMessageId]. -// Why take [nearMessageId] in a param, instead of looking for it in [narrow]? -// -// A reasonable question: after all, the "near" part of a near link (e.g., for -// quote-and-reply) does take the same form as other operator/operand pairs -// that we represent with [ApiNarrowElement]s, like "/stream/48-mobile". -// -// But unlike those other elements, we choose not to give the "near" element -// an [ApiNarrowElement] representation, because it doesn't have quite that role: -// it says where to look in a list of messages, but it doesn't filter the list down. -// In fact, from a brief look at server code, it seems to be *ignored* -// if you include it in the `narrow` param in get-messages requests. -// When you want to point the server to a location in a message list, you -// you do so by passing the `anchor` param. -Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { - final apiNarrow = narrow.apiEncode(); - final fragment = StringBuffer('narrow'); - for (ApiNarrowElement element in apiNarrow) { - fragment.write('/'); - if (element.negated) { - fragment.write('-'); - } - - if (element is ApiNarrowDm) { - final supportsOperatorDm = store.connection.zulipFeatureLevel! >= 177; // TODO(server-7) - element = element.resolve(legacy: !supportsOperatorDm); - } - - fragment.write('${element.operator}/'); - - switch (element) { - case ApiNarrowStream(): - final streamId = element.operand; - final name = store.streams[streamId]?.name ?? 'unknown'; - final slugifiedName = _encodeHashComponent(name.replaceAll(' ', '-')); - fragment.write('$streamId-$slugifiedName'); - case ApiNarrowTopic(): - fragment.write(_encodeHashComponent(element.operand)); - case ApiNarrowDmModern(): - final suffix = element.operand.length >= 3 ? 'group' : 'dm'; - fragment.write('${element.operand.join(',')}-$suffix'); - case ApiNarrowPmWith(): - final suffix = element.operand.length >= 3 ? 'group' : 'pm'; - fragment.write('${element.operand.join(',')}-$suffix'); - case ApiNarrowDm(): - assert(false, 'ApiNarrowDm should have been resolved'); - case ApiNarrowMessageId(): - fragment.write(element.operand.toString()); - } - } - - if (nearMessageId != null) { - fragment.write('/near/$nearMessageId'); - } - - return store.account.realmUrl.replace(fragment: fragment.toString()); -} - /// An @-mention, like @**Chris Bobbe|13313**. /// /// To omit the user ID part ("|13313") whenever the name part is unambiguous, diff --git a/lib/model/database.g.dart b/lib/model/database.g.dart index b7f0086756..130a0f9c45 100644 --- a/lib/model/database.g.dart +++ b/lib/model/database.g.dart @@ -1,6 +1,6 @@ // GENERATED CODE - DO NOT MODIFY BY HAND -// ignore_for_file: unnecessary_cast +// ignore_for_file: constant_identifier_names, unnecessary_cast part of 'database.dart'; diff --git a/lib/model/internal_link.dart b/lib/model/internal_link.dart new file mode 100644 index 0000000000..26ac1c8cb5 --- /dev/null +++ b/lib/model/internal_link.dart @@ -0,0 +1,285 @@ +import 'package:flutter/foundation.dart'; +import 'package:json_annotation/json_annotation.dart'; + +import '../api/model/narrow.dart'; +import 'narrow.dart'; +import 'store.dart'; + +part 'internal_link.g.dart'; + +const _hashReplacements = { + "%": ".", + "(": ".28", + ")": ".29", + ".": ".2E", +}; + +final _encodeHashComponentRegex = RegExp(r'[%().]'); + +// Corresponds to encodeHashComponent in Zulip web; +// see web/shared/src/internal_url.ts. +String _encodeHashComponent(String str) { + return Uri.encodeComponent(str) + .replaceAllMapped(_encodeHashComponentRegex, (Match m) => _hashReplacements[m[0]!]!); +} + +/// Decode a dot-encoded string; return null if malformed. +// The Zulip webapp uses this encoding in narrow-links: +// https://github.com/zulip/zulip/blob/1577662a6/static/js/hash_util.js#L18-L25 +@visibleForTesting +String? decodeHashComponent(String str) { + try { + return Uri.decodeComponent(str.replaceAll('.', '%')); + } on ArgumentError { + // as with '%1': Invalid argument(s): Truncated URI + return null; + } on FormatException { + // as with '%FF': FormatException: Invalid UTF-8 byte (at offset 0) + return null; + } +} + +/// A URL to the given [Narrow], on `store`'s realm. +/// +/// To include /near/{messageId} in the link, pass a non-null [nearMessageId]. +// Why take [nearMessageId] in a param, instead of looking for it in [narrow]? +// +// A reasonable question: after all, the "near" part of a near link (e.g., for +// quote-and-reply) does take the same form as other operator/operand pairs +// that we represent with [ApiNarrowElement]s, like "/stream/48-mobile". +// +// But unlike those other elements, we choose not to give the "near" element +// an [ApiNarrowElement] representation, because it doesn't have quite that role: +// it says where to look in a list of messages, but it doesn't filter the list down. +// In fact, from a brief look at server code, it seems to be *ignored* +// if you include it in the `narrow` param in get-messages requests. +// When you want to point the server to a location in a message list, you +// you do so by passing the `anchor` param. +Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { + final apiNarrow = narrow.apiEncode(); + final fragment = StringBuffer('narrow'); + for (ApiNarrowElement element in apiNarrow) { + fragment.write('/'); + if (element.negated) { + fragment.write('-'); + } + + if (element is ApiNarrowDm) { + final supportsOperatorDm = store.connection.zulipFeatureLevel! >= 177; // TODO(server-7) + element = element.resolve(legacy: !supportsOperatorDm); + } + + fragment.write('${element.operator}/'); + + switch (element) { + case ApiNarrowStream(): + final streamId = element.operand; + final name = store.streams[streamId]?.name ?? 'unknown'; + final slugifiedName = _encodeHashComponent(name.replaceAll(' ', '-')); + fragment.write('$streamId-$slugifiedName'); + case ApiNarrowTopic(): + fragment.write(_encodeHashComponent(element.operand)); + case ApiNarrowDmModern(): + final suffix = element.operand.length >= 3 ? 'group' : 'dm'; + fragment.write('${element.operand.join(',')}-$suffix'); + case ApiNarrowPmWith(): + final suffix = element.operand.length >= 3 ? 'group' : 'pm'; + fragment.write('${element.operand.join(',')}-$suffix'); + case ApiNarrowDm(): + assert(false, 'ApiNarrowDm should have been resolved'); + case ApiNarrowMessageId(): + fragment.write(element.operand.toString()); + } + } + + if (nearMessageId != null) { + fragment.write('/near/$nearMessageId'); + } + + return store.account.realmUrl.replace(fragment: fragment.toString()); +} + +/// Create a new `Uri` object in relation to a given realmUrl. +/// +/// Returns `null` if `urlString` could not be parsed as a `Uri`. +Uri? tryResolveOnRealmUrl(String urlString, Uri realmUrl) { + try { + return realmUrl.resolve(urlString); + } on FormatException { + return null; + } +} + +/// A [Narrow] from a given URL, on `store`'s realm. +/// +/// `url` must already be passed through [tryResolveOnRealmUrl]. +/// +/// Returns `null` if any of the operator/operand pairs are invalid. +/// +/// Since narrow links can combine operators in ways our [Narrow] type can't +/// represent, this can also return null for valid narrow links. +/// +/// This can also return null for some valid narrow links that our Narrow +/// type *could* accurately represent. We should try to understand these +/// better, but some kinds will be rare, even unheard-of: +/// #narrow/stream/1-announce/stream/1-announce (duplicated operator) +// TODO(#252): handle all valid narrow links, returning a search narrow +Narrow? parseInternalLink(Uri url, PerAccountStore store) { + if (!_isInternalLink(url, store.account.realmUrl)) return null; + + final (category, segments) = _getCategoryAndSegmentsFromFragment(url.fragment); + switch (category) { + case 'narrow': + if (segments.isEmpty || !segments.length.isEven) return null; + return _interpretNarrowSegments(segments, store); + } + return null; +} + +/// Check if `url` is an internal link on the given `realmUrl`. +bool _isInternalLink(Uri url, Uri realmUrl) { + try { + if (url.origin != realmUrl.origin) return false; + } on StateError { + return false; + } + return (url.hasEmptyPath || url.path == '/') + && !url.hasQuery + && url.hasFragment; +} + +/// Split `fragment` of arbitrary segments and handle trailing slashes +(String, List) _getCategoryAndSegmentsFromFragment(String fragment) { + final [category, ...segments] = fragment.split('/'); + if (segments.length > 1 && segments.last == '') segments.removeLast(); + return (category, segments); +} + +Narrow? _interpretNarrowSegments(List segments, PerAccountStore store) { + assert(segments.isNotEmpty); + assert(segments.length.isEven); + + ApiNarrowStream? streamElement; + ApiNarrowTopic? topicElement; + ApiNarrowDm? dmElement; + + for (var i = 0; i < segments.length; i += 2) { + final (operator, negated) = _parseOperator(segments[i]); + if (negated) return null; + final operand = segments[i + 1]; + switch (operator) { + case _NarrowOperator.stream: + if (streamElement != null) return null; + final streamId = _parseStreamOperand(operand, store); + if (streamId == null) return null; + streamElement = ApiNarrowStream(streamId, negated: negated); + + case _NarrowOperator.topic: + case _NarrowOperator.subject: + if (topicElement != null) return null; + final String? topic = decodeHashComponent(operand); + if (topic == null) return null; + topicElement = ApiNarrowTopic(topic, negated: negated); + + case _NarrowOperator.dm: + case _NarrowOperator.pmWith: + if (dmElement != null) return null; + final dmIds = _parseDmOperand(operand); + if (dmIds == null) return null; + dmElement = ApiNarrowDm(dmIds, negated: negated); + + case _NarrowOperator.near: + continue; // TODO(#82): support for near + + case _NarrowOperator.unknown: + return null; + } + } + + if (dmElement != null) { + if (streamElement != null || topicElement != null) return null; + return DmNarrow.withUsers(dmElement.operand, selfUserId: store.account.userId); + } else if (streamElement != null) { + final streamId = streamElement.operand; + if (topicElement != null) { + return TopicNarrow(streamId, topicElement.operand); + } else { + return StreamNarrow(streamId); + } + } + return null; +} + +@JsonEnum(fieldRename: FieldRename.kebab, alwaysCreate: true) +enum _NarrowOperator { + // 'dm' is new in server-7.0; means the same as 'pm-with' + dm, + near, + pmWith, + stream, + subject, + topic, + unknown; + + static _NarrowOperator fromRawString(String raw) => _byRawString[raw] ?? unknown; + + static final _byRawString = _$_NarrowOperatorEnumMap.map((key, value) => MapEntry(value, key)); +} + +(_NarrowOperator, bool) _parseOperator(String input) { + final String operator; + final bool negated; + if (input.startsWith('-')) { + operator = input.substring(1); + negated = true; + } else { + operator = input; + negated = false; + } + return (_NarrowOperator.fromRawString(operator), negated); +} + +/// Parse the operand of a `stream` operator, returning a stream ID. +/// +/// The ID might point to a stream that's hidden from our user (perhaps +/// doesn't exist). If so, most likely the user doesn't have permission to +/// see the stream's existence -- like with a guest user for any stream +/// they're not in, or any non-admin with a private stream they're not in. +/// Could be that whoever wrote the link just made something up. +/// +/// Returns null if the operand has an unexpected shape, or has the old shape +/// (stream name but no ID) and we don't know of a stream by the given name. +int? _parseStreamOperand(String operand, PerAccountStore store) { + // "New" (2018) format: ${stream_id}-${stream_name} . + final match = RegExp(r'^(\d+)(?:-.*)?$').firstMatch(operand); + final newFormatStreamId = (match != null) ? int.parse(match.group(1)!, radix: 10) : null; + if (newFormatStreamId != null && store.streams.containsKey(newFormatStreamId)) { + return newFormatStreamId; + } + + // Old format: just stream name. This case is relevant indefinitely, + // so that links in old conversations continue to work. + final String? streamName = decodeHashComponent(operand); + if (streamName == null) return null; + final stream = store.streamsByName[streamName]; + if (stream != null) return stream.streamId; + + if (newFormatStreamId != null) { + // Neither format found a stream, so it's hidden or doesn't exist. But + // at least we have a stream ID; give that to the caller. + return newFormatStreamId; + } + + // Unexpected shape, or the old shape and we don't know of a stream with + // the given name. + return null; +} + +List? _parseDmOperand(String operand) { + final rawIds = operand.split('-')[0].split(','); + try { + return rawIds.map((rawId) => int.parse(rawId, radix: 10)).toList(); + } on FormatException { + return null; + } +} diff --git a/lib/model/internal_link.g.dart b/lib/model/internal_link.g.dart new file mode 100644 index 0000000000..7a6952c5d5 --- /dev/null +++ b/lib/model/internal_link.g.dart @@ -0,0 +1,19 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: constant_identifier_names, unnecessary_cast + +part of 'internal_link.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +const _$_NarrowOperatorEnumMap = { + _NarrowOperator.dm: 'dm', + _NarrowOperator.near: 'near', + _NarrowOperator.pmWith: 'pm-with', + _NarrowOperator.stream: 'stream', + _NarrowOperator.subject: 'subject', + _NarrowOperator.topic: 'topic', + _NarrowOperator.unknown: 'unknown', +}; diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index c71cfe5407..fb29a5f151 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -168,6 +168,13 @@ class DmNarrow extends Narrow implements SendableNarrow { ); } + factory DmNarrow.withUsers(List userIds, {required int selfUserId}) { + return DmNarrow( + allRecipientIds: {...userIds, selfUserId}.toList()..sort(), + selfUserId: selfUserId, + ); + } + /// The user IDs of everyone in the conversation, sorted. /// /// Each message in the conversation is sent by one of these users diff --git a/lib/model/store.dart b/lib/model/store.dart index 8860d815f3..0e45555615 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -161,6 +161,8 @@ class PerAccountStore extends ChangeNotifier { .map((user) => MapEntry(user.userId, user))), streams = Map.fromEntries(initialSnapshot.streams.map( (stream) => MapEntry(stream.streamId, stream))), + streamsByName = Map.fromEntries(initialSnapshot.streams.map( + (stream) => MapEntry(stream.name, stream))), subscriptions = Map.fromEntries(initialSnapshot.subscriptions.map( (subscription) => MapEntry(subscription.streamId, subscription))), recentDmConversationsView = RecentDmConversationsView( @@ -185,6 +187,7 @@ class PerAccountStore extends ChangeNotifier { // Streams, topics, and stuff about them. final Map streams; + final Map streamsByName; final Map subscriptions; // TODO lots more data. When adding, be sure to update handleEvent too. @@ -285,6 +288,7 @@ class PerAccountStore extends ChangeNotifier { } else if (event is StreamCreateEvent) { assert(debugLog("server event: stream/create")); streams.addEntries(event.streams.map((stream) => MapEntry(stream.streamId, stream))); + streamsByName.addEntries(event.streams.map((stream) => MapEntry(stream.name, stream))); // (Don't touch `subscriptions`. If the user is subscribed to the stream, // details will come in a later `subscription` event.) notifyListeners(); @@ -292,6 +296,7 @@ class PerAccountStore extends ChangeNotifier { assert(debugLog("server event: stream/delete")); for (final stream in event.streams) { streams.remove(stream.streamId); + streamsByName.remove(stream.name); subscriptions.remove(stream.streamId); } notifyListeners(); diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 92bc193cb1..9dd59afafc 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -7,6 +7,7 @@ import '../api/core.dart'; import '../api/model/model.dart'; import '../model/binding.dart'; import '../model/content.dart'; +import '../model/internal_link.dart'; import '../model/store.dart'; import 'code_block.dart'; import 'dialog.dart'; @@ -661,12 +662,9 @@ void _launchUrl(BuildContext context, String urlString) async { } final store = PerAccountStoreWidget.of(context); - final Uri url; - try { - url = store.account.realmUrl.resolve(urlString); - } on FormatException { // TODO(log) + final url = tryResolveOnRealmUrl(urlString, store.account.realmUrl); + if (url == null) { // TODO(log) await showError(context, null); - if (!context.mounted) return; // TODO(dart): redundant for sake of lint return; } diff --git a/test/example_data.dart b/test/example_data.dart index 7f8206c735..776605b1da 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -273,11 +273,12 @@ InitialSnapshot initialSnapshot({ crossRealmBots: crossRealmBots ?? [], ); } +const _initialSnapshot = initialSnapshot; -PerAccountStore store() { +PerAccountStore store({Account? account, InitialSnapshot? initialSnapshot}) { return PerAccountStore.fromInitialSnapshot( - account: selfAccount, - connection: FakeApiConnection.fromAccount(selfAccount), - initialSnapshot: initialSnapshot(), + account: account ?? selfAccount, + connection: FakeApiConnection.fromAccount(account ?? selfAccount), + initialSnapshot: initialSnapshot ?? _initialSnapshot(), ); } diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index 4f22334778..ad68a70d10 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -2,6 +2,7 @@ import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/model/compose.dart'; import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/internal_link.dart'; import '../example_data.dart' as eg; import 'test_store.dart'; diff --git a/test/model/internal_link_test.dart b/test/model/internal_link_test.dart new file mode 100644 index 0000000000..c80c41ca76 --- /dev/null +++ b/test/model/internal_link_test.dart @@ -0,0 +1,388 @@ + +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/internal_link.dart'; +import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/store.dart'; + +import '../example_data.dart' as eg; +import 'test_store.dart'; + +PerAccountStore setupStore({ + required Uri realmUrl, + List? streams, + List? users, +}) { + final account = eg.selfAccount.copyWith(realmUrl: realmUrl); + final store = eg.store(account: account); + if (streams != null) { + store.addStreams(streams); + } + store.addUser(eg.selfUser); + if (users != null) { + store.addUsers(users); + } + return store; +} + +void main() { + final realmUrl = Uri.parse('https://example.com/'); + + void testExpectedNarrows(List<(String, Narrow?)> testCases, { + List? streams, + PerAccountStore? store, + List? users, + }) { + assert((streams != null || users != null) ^ (store != null)); + store ??= setupStore(realmUrl: realmUrl, streams: streams, users: users); + for (final testCase in testCases) { + final String urlString = testCase.$1; + final Uri url = tryResolveOnRealmUrl(urlString, realmUrl)!; + final Narrow? expected = testCase.$2; + test(urlString, () { + check(parseInternalLink(url, store!)).equals(expected); + }); + } + } + + group('parseInternalLink', () { + final streams = [ + eg.stream(streamId: 1, name: 'check'), + ]; + final testCases = [ + (true, 'legacy: stream name, no ID', + '#narrow/stream/check', realmUrl), + (true, 'legacy: stream name, no ID, topic', + '#narrow/stream/check/topic/topic1', realmUrl), + + (true, 'with numeric stream ID', + '#narrow/stream/123-check', realmUrl), + (true, 'with numeric stream ID and topic', + '#narrow/stream/123-a/topic/topic1', realmUrl), + + (true, 'with numeric pm user IDs (new operator)', + '#narrow/dm/123-mark', realmUrl), + (true, 'with numeric pm user IDs (old operator)', + '#narrow/pm-with/123-mark', realmUrl), + + (false, 'wrong fragment', + '#nope', realmUrl), + (false, 'wrong path', + 'user_uploads/#narrow/stream/check', realmUrl), + (false, 'wrong domain', + 'https://another.com/#narrow/stream/check', realmUrl), + + (false, '#narrowly', + '#narrowly/stream/check', realmUrl), + + (false, 'double slash', + 'https://example.com//#narrow/stream/check', realmUrl), + (false, 'triple slash', + 'https://example.com///#narrow/stream/check', realmUrl), + + (true, 'with port', + 'https://example.com:444/#narrow/stream/check', + Uri.parse('https://example.com:444/')), + + // Dart's [Uri] currently lacks IDNA or Punycode support: + // https://github.com/dart-lang/sdk/issues/26284 + // https://github.com/dart-lang/sdk/issues/29420 + + // (true, 'same domain, punycoded host', + // 'https://example.xn--h2brj9c/#narrow/stream/check', + // Uri.parse('https://example.भारत/')), // FAILS + + (true, 'punycodable host', + 'https://example.भारत/#narrow/stream/check', + Uri.parse('https://example.भारत/')), + + // (true, 'same domain, IDNA-mappable', + // 'https://ℯⅩªm🄿ₗℰ.ℭᴼⓂ/#narrow/stream/check', + // Uri.parse('https://example.com')), // FAILS + + (true, 'ipv4 address', + 'http://192.168.0.1/#narrow/stream/check', + Uri.parse('http://192.168.0.1/')), + + // (true, 'same IPv4 address, IDNA-mappable', + // 'http://1𝟗𝟚。①⁶🯸.₀。𝟭/#narrow/stream/check', + // Uri.parse('http://192.168.0.1/')), // FAILS + + // TODO: Add tests for IPv6. + + // These examples may seem weird, but a previous version of + // the zulip-mobile code accepted most of them. + + // This one, except possibly the fragment, is a 100% realistic link + // for innocent normal use. The buggy old version narrowly avoided + // accepting it... but would accept all the variations below. + (false, 'wrong domain, realm-like path, narrow-like fragment', + 'https://web.archive.org/web/*/${realmUrl.resolve('#narrow/stream/check')}', + realmUrl), + (false, 'odd scheme, wrong domain, realm-like path, narrow-like fragment', + 'ftp://web.archive.org/web/*/${realmUrl.resolve('#narrow/stream/check')}', + realmUrl), + (false, 'same domain, realm-like path, narrow-like fragment', + 'web/*/${realmUrl.resolve('#narrow/stream/check')}', + realmUrl), + ]; + for (final testCase in testCases) { + final bool expected = testCase.$1; + final String description = testCase.$2; + final String urlString = testCase.$3; + final Uri realmUrl = testCase.$4; + test('${expected ? 'accepts': 'rejects'} $description: $urlString', () { + final store = setupStore(realmUrl: realmUrl, streams: streams); + final url = tryResolveOnRealmUrl(urlString, realmUrl)!; + final result = parseInternalLink(url, store); + check(result != null).equals(expected); + }); + } + }); + + group('parseInternalLink', () { + final streams = [ + eg.stream(streamId: 1, name: 'check'), + eg.stream(streamId: 3, name: 'mobile'), + eg.stream(streamId: 5, name: 'stream'), + eg.stream(streamId: 123, name: 'topic'), + ]; + + group('"/#narrow/stream/<...>" returns expected StreamNarrow', () { + const testCases = [ + ('/#narrow/stream/check', StreamNarrow(1)), + ('/#narrow/stream/stream/', StreamNarrow(5)), + ('/#narrow/stream/topic/', StreamNarrow(123)), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + + group('"/#narrow/stream/<...>/topic/<...>" returns expected TopicNarrow', () { + const testCases = [ + ('/#narrow/stream/check/topic/test', TopicNarrow(1, 'test')), + ('/#narrow/stream/mobile/subject/topic/near/378333', TopicNarrow(3, 'topic')), + ('/#narrow/stream/mobile/topic/topic/', TopicNarrow(3, 'topic')), + ('/#narrow/stream/stream/topic/topic/near/1', TopicNarrow(5, 'topic')), + ('/#narrow/stream/stream/subject/topic/near/1', TopicNarrow(5, 'topic')), + ('/#narrow/stream/stream/subject/topic', TopicNarrow(5, 'topic')), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + + group('"/#narrow/dm/<...>" returns expected DmNarrow', () { + final expectedNarrow = DmNarrow.withUsers([1, 2], + selfUserId: eg.selfUser.userId); + final testCases = [ + ('/#narrow/dm/1,2-group', expectedNarrow), + ('/#narrow/dm/1,2-group/near/1', expectedNarrow), + ('/#narrow/dm/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', null), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + + group('"/#narrow/pm-with/<...>" returns expected DmNarrow', () { + final expectedNarrow = DmNarrow.withUsers([1, 2], + selfUserId: eg.selfUser.userId); + final testCases = [ + ('/#narrow/pm-with/1,2-group', expectedNarrow), + ('/#narrow/pm-with/1,2-group/near/1', expectedNarrow), + ('/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', null), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + + group('unexpected link shapes are rejected', () { + final testCases = [ + ('/#narrow/stream/name/topic/', null), // missing operand + ('/#narrow/stream/name/unknown/operand/', null), // unknown operator + ]; + testExpectedNarrows(testCases, streams: streams); + }); + }); + + group('decodeHashComponent', () { + group('correctly decodes MediaWiki-style dot-encoded strings', () { + final testCases = [ + ['some_text', 'some_text'], + ['some.20text', 'some text'], + ['some.2Etext', 'some.text'], + + ['na.C3.AFvet.C3.A9', 'naïveté'], + ['.C2.AF.5C_(.E3.83.84)_.2F.C2.AF', r'¯\_(ツ)_/¯'], + ]; + for (final [testCase, expected] in testCases) { + test('"$testCase"', () => + check(decodeHashComponent(testCase)).equals(expected)); + } + + final malformedTestCases = [ + // malformed dot-encoding + 'some.text', + 'some.2gtext', + 'some.arbitrary_text', + + // malformed UTF-8 + '.88.99.AA.BB', + ]; + for (final testCase in malformedTestCases) { + test('"$testCase"', () => + check(decodeHashComponent(testCase)).isNull()); + } + }); + + group('parses correctly in stream and topic operands', () { + final streams = [ + eg.stream(streamId: 1, name: 'some_stream'), + eg.stream(streamId: 2, name: 'some stream'), + eg.stream(streamId: 3, name: 'some.stream'), + ]; + const testCases = [ + ('/#narrow/stream/some_stream', StreamNarrow(1)), + ('/#narrow/stream/some.20stream', StreamNarrow(2)), + ('/#narrow/stream/some.2Estream', StreamNarrow(3)), + ('/#narrow/stream/some_stream/topic/some_topic', TopicNarrow(1, 'some_topic')), + ('/#narrow/stream/some_stream/topic/some.20topic', TopicNarrow(1, 'some topic')), + ('/#narrow/stream/some_stream/topic/some.2Etopic', TopicNarrow(1, 'some.topic')), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + }); + + group('parseInternalLink edge cases', () { + void testExpectedStreamNarrow(String testCase, int? streamId) { + final streamNarrow = (streamId != null) ? StreamNarrow(streamId) : null; + testExpectedNarrows([(testCase, streamNarrow)], streams: [ + eg.stream(streamId: 1, name: "general"), + ]); + } + + group('basic', () { + testExpectedStreamNarrow('#narrow/stream/1-general', 1); + }); + + group('if stream not found, use stream ID anyway', () { + testExpectedStreamNarrow('#narrow/stream/123-topic', 123); + }); + + group('on stream link with wrong name, ID wins', () { + testExpectedStreamNarrow('#narrow/stream/1-nonsense', 1); + testExpectedStreamNarrow('#narrow/stream/1-', 1); + }); + + group('on malformed stream link: reject', () { + testExpectedStreamNarrow('#narrow/stream/-1', null); + testExpectedStreamNarrow('#narrow/stream/1nonsense-general', null); + testExpectedStreamNarrow('#narrow/stream/-general', null); + }); + }); + + group('parseInternalLink with historic links', () { + group('for stream with hyphens or even looking like new-style', () { + final streams = [ + eg.stream(streamId: 1, name: 'test-team'), + eg.stream(streamId: 2, name: '311'), + eg.stream(streamId: 3, name: '311-'), + eg.stream(streamId: 4, name: '311-help'), + eg.stream(streamId: 5, name: '--help'), + ]; + const testCases = [ + ('#narrow/stream/test-team/', StreamNarrow(1)), + ('#narrow/stream/311/', StreamNarrow(2)), + ('#narrow/stream/311-/', StreamNarrow(3)), + ('#narrow/stream/311-help/', StreamNarrow(4)), + ('#narrow/stream/--help/', StreamNarrow(5)), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + + group('on ambiguous new- or old-style: new wins', () { + final streams = [ + eg.stream(streamId: 1, name: '311'), + eg.stream(streamId: 2, name: '311-'), + eg.stream(streamId: 3, name: '311-help'), + eg.stream(streamId: 311, name: 'collider'), + ]; + const testCases = [ + ('#narrow/stream/311/', StreamNarrow(311)), + ('#narrow/stream/311-/', StreamNarrow(311)), + ('#narrow/stream/311-help/', StreamNarrow(311)), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + + group('on old stream link', () { + final streams = [ + eg.stream(streamId: 1, name: 'check'), + eg.stream(streamId: 2, name: 'bot testing'), + eg.stream(streamId: 3, name: 'check.API'), + eg.stream(streamId: 4, name: 'stream'), + eg.stream(streamId: 5, name: 'topic'), + ]; + const testCases = [ + ('#narrow/stream/check/', StreamNarrow(1)), + ('#narrow/stream/bot.20testing/', StreamNarrow(2)), + ('#narrow/stream/check.2EAPI/', StreamNarrow(3)), + ('#narrow/stream/stream/', StreamNarrow(4)), + ('#narrow/stream/topic/', StreamNarrow(5)), + + ('#narrow/stream/check.API/', null), + ]; + testExpectedNarrows(testCases, streams: streams); + }); + }); + + group('parseInternalLink', () { + group('topic link parsing', () { + final stream = eg.stream(name: "general"); + + group('basic', () { + String mkUrlString(operand) { + return '#narrow/stream/${stream.streamId}-${stream.name}/topic/$operand'; + } + final testCases = [ + (mkUrlString('(no.20topic)'), TopicNarrow(stream.streamId, '(no topic)')), + (mkUrlString('lunch'), TopicNarrow(stream.streamId, 'lunch')), + ]; + testExpectedNarrows(testCases, streams: [stream]); + }); + + group('on old topic link, with dot-encoding', () { + String mkUrlString(operand) { + return '#narrow/stream/${stream.name}/topic/$operand'; + } + final testCases = [ + (mkUrlString('(no.20topic)'), TopicNarrow(stream.streamId, '(no topic)')), + (mkUrlString('google.2Ecom'), TopicNarrow(stream.streamId, 'google.com')), + (mkUrlString('google.com'), null), + (mkUrlString('topic.20name'), TopicNarrow(stream.streamId, 'topic name')), + (mkUrlString('stream'), TopicNarrow(stream.streamId, 'stream')), + (mkUrlString('topic'), TopicNarrow(stream.streamId, 'topic')), + ]; + testExpectedNarrows(testCases, streams: [stream]); + }); + }); + + group('DM link parsing', () { + void testExpectedDmNarrow(String testCase) { + final expectedNarrow = DmNarrow.withUsers([1, 2], + selfUserId: eg.selfUser.userId); + testExpectedNarrows([(testCase, expectedNarrow)], users: [ + eg.user(userId: 1), + eg.user(userId: 2), + ]); + } + + group('on group PM link', () { + testExpectedDmNarrow('#narrow/dm/1,2-group'); + testExpectedDmNarrow('#narrow/pm-with/1,2-group'); + }); + + group('on group PM link including self', () { + // The webapp doesn't generate these, but best to handle them anyway. + testExpectedDmNarrow('#narrow/dm/1,2,${eg.selfUser.userId}-group'); + testExpectedDmNarrow('#narrow/pm-with/1,2,${eg.selfUser.userId}-group'); + }); + }); + }); +} diff --git a/test/model/narrow_test.dart b/test/model/narrow_test.dart index c7825c6766..10fb8e8efa 100644 --- a/test/model/narrow_test.dart +++ b/test/model/narrow_test.dart @@ -90,6 +90,20 @@ void main() { selfUserId: 5)); }); + test('withUsers: without selfUserId', () { + final actual = DmNarrow.withUsers([1, 2], selfUserId: 3); + check(actual).equals(DmNarrow( + allRecipientIds: [1, 2, 3], + selfUserId: 3)); + }); + + test('withUsers: with selfUserId', () { + final actual = DmNarrow.withUsers([1, 2, 3], selfUserId: 3); + check(actual).equals(DmNarrow( + allRecipientIds: [1, 2, 3], + selfUserId: 3)); + }); + test('otherRecipientIds', () { check(DmNarrow(allRecipientIds: [1, 2, 3], selfUserId: 2)) .otherRecipientIds.deepEquals([1, 3]);