Skip to content

Commit e3860ae

Browse files
committed
internal links: Reject unrecognized streams
Now getNarrowFromLink returns null on a link to a stream we don't know about, just as it would if the link is malformed. After all, there's nothing useful we're going to do anyway with such a narrow.
1 parent 8f55c7c commit e3860ae

File tree

3 files changed

+29
-13
lines changed

3 files changed

+29
-13
lines changed

src/message/messagesActions.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { getMessageIdFromLink, getNarrowFromLink } from '../utils/internalLinks'
66
import { openLinkWithUserPreference } from '../utils/openLink';
77
import { navigateToChat } from '../nav/navActions';
88
import { FIRST_UNREAD_ANCHOR } from '../anchor';
9-
import { getStreamsById } from '../subscriptions/subscriptionSelectors';
9+
import { getStreamsById, getStreamsByName } from '../subscriptions/subscriptionSelectors';
1010
import * as api from '../api';
1111
import { isUrlOnRealm } from '../utils/url';
1212
import { getOwnUserId } from '../users/userSelectors';
@@ -30,8 +30,9 @@ export const messageLinkPress = (href: string): ThunkAction<Promise<void>> => as
3030
const state = getState();
3131
const auth = getAuth(state);
3232
const streamsById = getStreamsById(state);
33+
const streamsByName = getStreamsByName(state);
3334
const ownUserId = getOwnUserId(state);
34-
const narrow = getNarrowFromLink(href, auth.realm, streamsById, ownUserId);
35+
const narrow = getNarrowFromLink(href, auth.realm, streamsById, streamsByName, ownUserId);
3536
if (narrow) {
3637
const anchor = getMessageIdFromLink(href, auth.realm);
3738
dispatch(doNarrow(narrow, anchor));

src/utils/__tests__/internalLinks-test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ describe('getNarrowFromLink', () => {
261261
url,
262262
new URL('https://example.com'),
263263
new Map(streams.map(s => [s.stream_id, s])),
264+
new Map(streams.map(s => [s.name, s])),
264265
eg.selfUser.user_id,
265266
);
266267

@@ -287,10 +288,9 @@ describe('getNarrowFromLink', () => {
287288
expectStream(`${streamGeneral.stream_id}-`, [streamGeneral], streamGeneral);
288289
});
289290

290-
test('on malformed stream link: treat as old format', () => {
291-
const expectAsName = name => expectStream(name, [streamGeneral], eg.makeStream({ name }));
292-
expectAsName(`-${streamGeneral.stream_id}`);
293-
expectAsName(`${streamGeneral.stream_id}nonsense-general`);
291+
test('on malformed stream link: reject', () => {
292+
expectStream(`-${streamGeneral.stream_id}`, [streamGeneral], null);
293+
expectStream(`${streamGeneral.stream_id}nonsense-general`, [streamGeneral], null);
294294
});
295295

296296
{

src/utils/internalLinks.js

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,12 @@ export const decodeHashComponent = (string: string): string => {
118118
}
119119
};
120120

121-
/** Parse the operand of a `stream` operator, returning a stream name. */
122-
const parseStreamOperand = (operand, streamsById): string => {
121+
/**
122+
* Parse the operand of a `stream` operator, returning a stream name.
123+
*
124+
* Return null if the operand doesn't match any known stream.
125+
*/
126+
const parseStreamOperand = (operand, streamsById, streamsByName): null | string => {
123127
// "New" (2018) format: ${stream_id}-${stream_name} .
124128
const match = /^([\d]+)(?:-.*)?$/.exec(operand);
125129
if (match) {
@@ -131,7 +135,13 @@ const parseStreamOperand = (operand, streamsById): string => {
131135

132136
// Old format: just stream name. This case is relevant indefinitely,
133137
// so that links in old conversations continue to work.
134-
return decodeHashComponent(operand);
138+
const streamName = decodeHashComponent(operand);
139+
const stream = streamsByName.get(streamName);
140+
if (stream) {
141+
return streamName;
142+
}
143+
144+
return null;
135145
};
136146

137147
/** Parse the operand of a `topic` or `subject` operator. */
@@ -154,6 +164,7 @@ export const getNarrowFromLink = (
154164
url: string,
155165
realm: URL,
156166
streamsById: Map<number, Stream>,
167+
streamsByName: Map<string, Stream>,
157168
ownUserId: UserId,
158169
): Narrow | null => {
159170
const type = getLinkType(url, realm);
@@ -169,10 +180,14 @@ export const getNarrowFromLink = (
169180
const ids = parsePmOperand(paths[1]);
170181
return pmNarrowFromRecipients(pmKeyRecipientsFromIds(ids, ownUserId));
171182
}
172-
case 'topic':
173-
return topicNarrow(parseStreamOperand(paths[1], streamsById), parseTopicOperand(paths[3]));
174-
case 'stream':
175-
return streamNarrow(parseStreamOperand(paths[1], streamsById));
183+
case 'topic': {
184+
const streamName = parseStreamOperand(paths[1], streamsById, streamsByName);
185+
return streamName == null ? null : topicNarrow(streamName, parseTopicOperand(paths[3]));
186+
}
187+
case 'stream': {
188+
const streamName = parseStreamOperand(paths[1], streamsById, streamsByName);
189+
return streamName == null ? null : streamNarrow(streamName);
190+
}
176191
case 'special':
177192
try {
178193
return specialNarrow(paths[1]);

0 commit comments

Comments
 (0)