From dcc28e0db0cadc3982458f1387fa13154bb4d792 Mon Sep 17 00:00:00 2001 From: Shu Chen Date: Fri, 15 Sep 2023 14:52:56 +0100 Subject: [PATCH] internal_link: parse internal links into narrows --- lib/model/internal_link.dart | 174 +++++++++++++++++++- lib/model/internal_link.g.dart | 19 +++ test/model/internal_link_test.dart | 251 +++++++++++++++++++++++++++++ 3 files changed, 443 insertions(+), 1 deletion(-) create mode 100644 lib/model/internal_link.g.dart create mode 100644 test/model/internal_link_test.dart diff --git a/lib/model/internal_link.dart b/lib/model/internal_link.dart index 79c4ea2f4a..26903e6e7b 100644 --- a/lib/model/internal_link.dart +++ b/lib/model/internal_link.dart @@ -1,8 +1,11 @@ -import 'store.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 = { "%": ".", @@ -20,6 +23,13 @@ String _encodeHashComponent(String str) { .replaceAllMapped(_encodeHashComponentRegex, (Match m) => _hashReplacements[m[0]!]!); } +/// Decode a dot-encoded string. +// The Zulip webapp uses this encoding in narrow-links: +// https://github.com/zulip/zulip/blob/1577662a6/static/js/hash_util.js#L18-L25 +String _decodeHashComponent(String str) { + return Uri.decodeComponent(str.replaceAll('.', '%')); +} + /// A URL to the given [Narrow], on `store`'s realm. /// /// To include /near/{messageId} in the link, pass a non-null [nearMessageId]. @@ -79,3 +89,165 @@ Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { return store.account.realmUrl.replace(fragment: fragment.toString()); } + +/// A [Narrow] from a given URL, on `store`'s realm. +/// +/// 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) +/// +/// The passed `url` must appear to be a link to a Zulip narrow on the given +/// `realm`. +Narrow? parseInternalLink(Uri url, PerAccountStore store) { + if (!url.hasFragment) return null; + if (!url.hasScheme || url.host.isEmpty) return null; + if (!url.hasEmptyPath && (url.path != '/')) return null; + + if (url.origin != store.account.realmUrl.origin) 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; +} + +// Helper to split fragment that handles 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 _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)!) : 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 streamName = _decodeHashComponent(operand); + 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? _parsePmOperand(String operand) { + final rawIds = operand.split('-')[0].split(','); + try { + return rawIds.map(int.parse).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..bd6aefc06c --- /dev/null +++ b/lib/model/internal_link.g.dart @@ -0,0 +1,19 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: 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/test/model/internal_link_test.dart b/test/model/internal_link_test.dart new file mode 100644 index 0000000000..6326d5cd3f --- /dev/null +++ b/test/model/internal_link_test.dart @@ -0,0 +1,251 @@ + +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 'binding.dart'; + +Future setupStore(Uri realmUrl, {List? streams, User? selfUser}) async { + addTearDown(testBinding.reset); + selfUser ??= eg.selfUser; + streams ??= []; + final account = Account( + id: 1001, + realmUrl: realmUrl, + email: selfUser.email, + userId: selfUser.userId, + apiKey: 'abcd', + zulipFeatureLevel: eg.recentZulipFeatureLevel, + zulipVersion: eg.recentZulipVersion, + zulipMergeBase: eg.recentZulipVersion, + ); + final initialSnapshot = eg.initialSnapshot(streams: streams); + await testBinding.globalStore.add(account, initialSnapshot); + return await testBinding.globalStore.perAccount(account.id); +} + +void main() { + final realmUrl = Uri.parse('http://on_realm.example.com/'); + TestZulipBinding.ensureInitialized(); + + checkExpectedNarrows(testCases, {List? streams, PerAccountStore? store}) async { + assert((streams != null) ^ (store != null)); + if (store == null) { + streams ??= []; + store = await setupStore(realmUrl, streams: streams); + } + for (final testCase in testCases) { + final String path = testCase.$1; + final Uri url = realmUrl.resolve(path); + final Narrow? expected = testCase.$2; + check(because: path, parseInternalLink(url, store)).equals(expected); + } + } + + group('Validate narrow links', () { + test('is valid narrow link', () async { + final streams = [ + eg.stream(streamId: 1, name: 'check'), + ]; + final store = await setupStore(realmUrl, streams: streams); + final testCases = [ + (true, 'legacy: stream name, no ID', + realmUrl.resolve('/#narrow/stream/check')), + (true, 'legacy: stream name, no ID, topic', + realmUrl.resolve('/#narrow/stream/check/topic/topic1')), + + (true, 'with numeric stream ID', + realmUrl.resolve('/#narrow/stream/123-check')), + (true, 'with numeric stream ID and topic', + realmUrl.resolve('/#narrow/stream/123-check/topic/topic1')), + + (true, 'with numeric pm user IDs (new operator)', + realmUrl.resolve('/#narrow/dm/123-mark')), + (true, 'with numeric pm user IDs (old operator)', + realmUrl.resolve('/#narrow/pm-with/123-mark')), + + (false, 'wrong fragment', + realmUrl.resolve('/#nope')), + (false, 'wrong path', + realmUrl.resolve('/user_uploads/#narrow/stream/check')), + + (false, '#narrowly', + realmUrl.resolve('/#narrowly/stream/check')), + + (false, 'double slash', + Uri.parse('http://on_realm.example.com//#narrow/stream/check')), + (false, 'triple slash', + Uri.parse('http://on_realm.example.com///#narrow/stream/check')), + ]; + for (final testCase in testCases) { + final bool expected = testCase.$1; + final String because = testCase.$2; + final Uri url = testCase.$3; + final result = parseInternalLink(url, store); + check(because: because, result != null).equals(expected); + } + }); + + test('is valid narrow link with different realmUrls', () async { + final streams = [ + eg.stream(streamId: 1, name: 'check'), + ]; + final testCases = [ + (false, 'wrong domain', realmUrl, + Uri.parse('https://another.com/#narrow/stream/check')), + (true, 'with port', Uri.parse('http://on_realm.example.com:444/'), + Uri.parse('http://on_realm.example.com:444/#narrow/stream/check')), + + // TODO: Add tests for IPv6. + + // These examples may seem weird, but a previous version 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', realmUrl, + Uri.parse('https://web.archive.org/web/*/${realmUrl.resolve('#narrow/stream/check').toString()}')), + (false, 'odd scheme, wrong domain, realm-like path, narrow-like fragment', realmUrl, + Uri.parse('ftp://web.archive.org/web/*/${realmUrl.resolve('#narrow/stream/check').toString()}')), + (false, 'same domain, realm-like path, narrow-like fragment', realmUrl, + realmUrl.resolve('web/*/${realmUrl.resolve('#narrow/stream/check').toString()}')), + ]; + for (final testCase in testCases) { + final bool expected = testCase.$1; + final String because = testCase.$2; + final Uri realmUrl = testCase.$3; + final Uri url = testCase.$4; + final store = await setupStore(realmUrl, streams: streams); + final result = parseInternalLink(url, store); + check(because: because, result != null).equals(expected); + testBinding.reset(); + } + }); + }); + + group('Get Narrow', () { + test('returns expected narrow', () async { + 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'), + ]; + final selfUser = eg.user(userId: 1982); + final store = await setupStore(realmUrl, selfUser: selfUser, streams: streams); + final testCases = [ + // stream links + ('/#narrow/stream/check', const StreamNarrow(1)), + ('/#narrow/stream/stream/', const StreamNarrow(5)), + ('/#narrow/stream/topic/', const StreamNarrow(123)), + + ('/#narrow/stream/1-check', const StreamNarrow(1)), + ('/#narrow/stream/123-topic', const StreamNarrow(123)), + ('/#narrow/stream/3-mobile/near/378333', const StreamNarrow(3)), // TODO(#82): near + + // streams with wrong name use the streamId anyway + ('/#narrow/stream/1-incorrect-name', const StreamNarrow(1)), + ('/#narrow/stream/1-', const StreamNarrow(1)), + + // streams not in the store use the streamId anyway + ('/#narrow/stream/999-name', const StreamNarrow(999)), + + // malformed stream links + ('/#narrow/stream/123name', null), + ('/#narrow/stream/-123name', null), + ('/#narrow/stream/-name', null), + + // TODO(#252): return SearchNarrow + // negated links are not yet supported because they + // need to return a SearchNarrow + ('/#narrow/-stream/check', null), + ('/#narrow/-stream/check/topic/test', null), + ('/#narrow/stream/check/-topic/test', null), + ('/#narrow/-stream/check/-topic/test', null), + + // topic links + ('/#narrow/stream/check/topic/test', const TopicNarrow(1, 'test')), + ('/#narrow/stream/mobile/subject/topic/near/378333', const TopicNarrow(3, 'topic')), // TODO(#82): near + ('/#narrow/stream/mobile/topic/topic/', const TopicNarrow(3, 'topic')), + ('/#narrow/stream/stream/topic/topic/near/1', const TopicNarrow(5, 'topic')), // TODO(#82): near + ('/#narrow/stream/stream/subject/topic/near/1', const TopicNarrow(5, 'topic')), // TODO(#82): near + ('/#narrow/stream/stream/subject/topic', const TopicNarrow(5, 'topic')), + ('/#narrow/stream/missing/subject/topic', null), + + // pm links + ('/#narrow/dm/1,2-group', DmNarrow(allRecipientIds: [1,2,1982], selfUserId: 1982)), + ('/#narrow/dm/1,2-group/near/1', DmNarrow(allRecipientIds: [1,2,1982], selfUserId: 1982)), // TODO(#82): near + ('/#narrow/dm/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', null), + ('/#narrow/pm-with/1,2-group', DmNarrow(allRecipientIds: [1,2,1982], selfUserId: 1982)), + ('/#narrow/pm-with/1,2-group/near/1', DmNarrow(allRecipientIds: [1,2,1982], selfUserId: 1982)), // TODO(#82): near + ('/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', null), + + // pm link including the current user + // The webapp doesn't generate these, but best to handle them anyway. + ('/#narrow/dm/1,2,1982-group', DmNarrow(allRecipientIds: [1,2,1982], selfUserId: 1982)), + ('/#narrow/pm-with/1,2,1982-group', DmNarrow(allRecipientIds: [1,2,1982], selfUserId: 1982)), + + // is links + // ('/#narrow/is/dm', null), + // ('/#narrow/is/private', null), + // ('/#narrow/is/starred', null), + // ('/#narrow/is/mentioned', null), + ]; + await checkExpectedNarrows(testCases, store: store); + }); + + test('correctly handle MediaWiki-style dot-encoded strings', () async { + final streams = [ + eg.stream(streamId: 1, name: 'some_stream'), + eg.stream(streamId: 2, name: 'some stream'), + eg.stream(streamId: 3, name: 'some.stream'), + ]; + final testCases = [ + ('/#narrow/stream/some_stream', const StreamNarrow(1)), + ('/#narrow/stream/some.20stream', const StreamNarrow(2)), + ('/#narrow/stream/some.2Estream', const StreamNarrow(3)), + ('/#narrow/stream/some_stream/topic/some_topic', const TopicNarrow(1, 'some_topic')), + ('/#narrow/stream/some_stream/topic/some.20topic', const TopicNarrow(1, 'some topic')), + ('/#narrow/stream/some_stream/topic/some.2Etopic', const TopicNarrow(1, 'some.topic')), + ]; + await checkExpectedNarrows(testCases, streams: streams); + }); + + test('old stream links, for stream with hyphens or even looking like new-style', () async { + 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'), + ]; + final testCases = [ + ('/#narrow/stream/test-team/', const StreamNarrow(1)), + ('/#narrow/stream/311/', const StreamNarrow(2)), + ('/#narrow/stream/311-/', const StreamNarrow(3)), + ('/#narrow/stream/311-help/', const StreamNarrow(4)), + ('/#narrow/stream/--help/', const StreamNarrow(5)), + ]; + await checkExpectedNarrows(testCases, streams: streams); + }); + + test('on ambiguous new- or old-style: new wins', () async { + 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'), + ]; + final testCases = [ + ('/#narrow/stream/311/', const StreamNarrow(311)), + ('/#narrow/stream/311-/', const StreamNarrow(311)), + ('/#narrow/stream/311-help/', const StreamNarrow(311)), + ]; + await checkExpectedNarrows(testCases, streams: streams); + }); + }); +}