diff --git a/package.json b/package.json index 97a00a5a1a3..096d6d3c60b 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,6 @@ "lodash.isequal": "^4.4.0", "lodash.isplainobject": "^4.0.6", "lodash.omit": "^4.5.0", - "lodash.unescape": "^4.0.1", "lodash.union": "^4.6.0", "lodash.uniqby": "^4.4.0", "react": "16.11.0", diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index 5eb81dcce71..4439e0d8679 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -80,6 +80,23 @@ const makeUniqueRandInt = (itemsType: string, end: number): (() => number) => { /** Return a string that's almost surely different every time. */ export const randString = () => randInt(2 ** 54).toString(36); +const intRange = (start, len) => Array.from({ length: len }, (k, i) => i + start); + +/** A string with diverse characters to exercise encoding/decoding bugs. */ +/* eslint-disable prefer-template */ +export const diverseCharacters = + // The whole range of lowest code points, including control codes + // and ASCII punctuation like `"` and `&` used in various syntax... + String.fromCharCode(...intRange(0, 0x100)) + // ... some characters from other scripts... + + 'いい文字🎇' + // ... some unpaired surrogates, which JS strings can have... + + String.fromCharCode(...intRange(0xdbf0, 0x20)) + // ... some characters beyond the BMP (≥ U+10000)... + + '𐂷𠂢' + // ... and some code points at the very end of the Unicode range. + + String.fromCodePoint(...intRange(0x10fff0, 0x10)); + /* ======================================================================== * Users and bots */ diff --git a/src/boot/store.js b/src/boot/store.js index d8a71347a3a..f18e5e450de 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -19,6 +19,8 @@ import ZulipAsyncStorage from './ZulipAsyncStorage'; import createMigration from '../redux-persist-migrate/index'; import { provideLoggingContext } from './loggingContext'; import { tryGetActiveAccount } from '../account/accountsSelectors'; +import { keyFromNarrow } from '../utils/narrow'; +import { objectFromEntries } from '../jsBackport'; if (process.env.NODE_ENV === 'development') { // Chrome dev tools for Immutable. @@ -225,6 +227,18 @@ const migrations: { [string]: (GlobalState) => GlobalState } = { // `AvatarURL`. '18': dropCache, + // Change format of keys representing narrows, from JSON to our format. + '19': state => ({ + ...dropCache(state), + drafts: objectFromEntries( + // Note this will migrate straight to our current format, even after + // that changes from when this migration was written! That saves us + // from duplicating `keyFromNarrow` here... but calls for care in + // migrations for future changes to `keyFromNarrow`. + Object.keys(state.drafts).map(key => [keyFromNarrow(JSON.parse(key)), state.drafts[key]]), + ), + }), + // TIP: When adding a migration, consider just using `dropCache`. }; diff --git a/src/caughtup/caughtUpReducer.js b/src/caughtup/caughtUpReducer.js index 762ed435c8a..dd4f01f87ae 100644 --- a/src/caughtup/caughtUpReducer.js +++ b/src/caughtup/caughtUpReducer.js @@ -12,7 +12,7 @@ import { import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../anchor'; import { NULL_OBJECT } from '../nullObjects'; import { DEFAULT_CAUGHTUP } from './caughtUpSelectors'; -import { isSearchNarrow } from '../utils/narrow'; +import { isSearchNarrow, keyFromNarrow } from '../utils/narrow'; const initialState: CaughtUpState = NULL_OBJECT; @@ -89,7 +89,7 @@ export default (state: CaughtUpState = initialState, action: Action): CaughtUpSt if (isSearchNarrow(action.narrow)) { return state; } - const key = JSON.stringify(action.narrow); + const key = keyFromNarrow(action.narrow); let caughtUp; if (action.foundNewest !== undefined && action.foundOldest !== undefined) { /* This should always be the case for Zulip Server v1.8 or newer. */ diff --git a/src/caughtup/caughtUpSelectors.js b/src/caughtup/caughtUpSelectors.js index 968008c6d92..49964b78d3f 100644 --- a/src/caughtup/caughtUpSelectors.js +++ b/src/caughtup/caughtUpSelectors.js @@ -1,6 +1,7 @@ /* @flow strict-local */ import type { CaughtUp, CaughtUpState, GlobalState, Narrow } from '../types'; import { NULL_OBJECT } from '../nullObjects'; +import { keyFromNarrow } from '../utils/narrow'; /** The value implicitly represented by a missing entry in CaughtUpState. */ export const DEFAULT_CAUGHTUP: CaughtUp = { @@ -11,4 +12,4 @@ export const DEFAULT_CAUGHTUP: CaughtUp = { export const getCaughtUp = (state: GlobalState): CaughtUpState => state.caughtUp || NULL_OBJECT; export const getCaughtUpForNarrow = (state: GlobalState, narrow: Narrow): CaughtUp => - getCaughtUp(state)[JSON.stringify(narrow)] || DEFAULT_CAUGHTUP; + getCaughtUp(state)[keyFromNarrow(narrow)] || DEFAULT_CAUGHTUP; diff --git a/src/chat/__tests__/fetchingReducer-test.js b/src/chat/__tests__/fetchingReducer-test.js index 62f01e4a2b5..64093bd808b 100644 --- a/src/chat/__tests__/fetchingReducer-test.js +++ b/src/chat/__tests__/fetchingReducer-test.js @@ -3,7 +3,7 @@ import deepFreeze from 'deep-freeze'; import * as eg from '../../__tests__/lib/exampleData'; import fetchingReducer from '../fetchingReducer'; -import { HOME_NARROW, HOME_NARROW_STR, streamNarrow } from '../../utils/narrow'; +import { HOME_NARROW, HOME_NARROW_STR, streamNarrow, keyFromNarrow } from '../../utils/narrow'; import { MESSAGE_FETCH_START, MESSAGE_FETCH_ERROR } from '../../actionConstants'; import { DEFAULT_FETCHING } from '../fetchingSelectors'; @@ -44,7 +44,7 @@ describe('fetchingReducer', () => { const expectedState = { [HOME_NARROW_STR]: { older: false, newer: false }, - [JSON.stringify(streamNarrow('some stream'))]: { older: true, newer: false }, + [keyFromNarrow(streamNarrow('some stream'))]: { older: true, newer: false }, }; const newState = fetchingReducer(initialState, action); diff --git a/src/chat/__tests__/narrowsReducer-test.js b/src/chat/__tests__/narrowsReducer-test.js index f4700739a2a..f661b6dd717 100644 --- a/src/chat/__tests__/narrowsReducer-test.js +++ b/src/chat/__tests__/narrowsReducer-test.js @@ -12,6 +12,7 @@ import { streamNarrow, topicNarrow, STARRED_NARROW_STR, + keyFromNarrow, } from '../../utils/narrow'; import { MESSAGE_FETCH_ERROR, @@ -23,11 +24,11 @@ import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../../anchor'; import * as eg from '../../__tests__/lib/exampleData'; describe('narrowsReducer', () => { - const privateNarrowStr = JSON.stringify(pm1to1NarrowFromUser(eg.otherUser)); - const groupNarrowStr = JSON.stringify(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser])); - const streamNarrowStr = JSON.stringify(streamNarrow(eg.stream.name)); + const privateNarrowStr = keyFromNarrow(pm1to1NarrowFromUser(eg.otherUser)); + const groupNarrowStr = keyFromNarrow(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser])); + const streamNarrowStr = keyFromNarrow(streamNarrow(eg.stream.name)); const egTopic = eg.streamMessage().subject; - const topicNarrowStr = JSON.stringify(topicNarrow(eg.stream.name, egTopic)); + const topicNarrowStr = keyFromNarrow(topicNarrow(eg.stream.name, egTopic)); describe('EVENT_NEW_MESSAGE', () => { test('if not caught up in narrow, do not add message in home narrow', () => { @@ -145,7 +146,7 @@ describe('narrowsReducer', () => { }); test('message sent to self is stored correctly', () => { - const narrowWithSelfStr = JSON.stringify(pm1to1NarrowFromUser(eg.selfUser)); + const narrowWithSelfStr = keyFromNarrow(pm1to1NarrowFromUser(eg.selfUser)); const initialState = Immutable.Map({ [HOME_NARROW_STR]: [], [narrowWithSelfStr]: [], @@ -381,7 +382,7 @@ describe('narrowsReducer', () => { const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3], - [JSON.stringify(pm1to1NarrowFromUser(eg.otherUser))]: [], + [keyFromNarrow(pm1to1NarrowFromUser(eg.otherUser))]: [], }); const newState = narrowsReducer(initialState, action); diff --git a/src/chat/__tests__/narrowsSelectors-test.js b/src/chat/__tests__/narrowsSelectors-test.js index 70d5f0388a5..ebc921df2a6 100644 --- a/src/chat/__tests__/narrowsSelectors-test.js +++ b/src/chat/__tests__/narrowsSelectors-test.js @@ -16,6 +16,7 @@ import { topicNarrow, STARRED_NARROW, pmNarrowFromUsersUnsafe, + keyFromNarrow, } from '../../utils/narrow'; import { NULL_SUBSCRIPTION } from '../../nullObjects'; import * as eg from '../../__tests__/lib/exampleData'; @@ -32,7 +33,7 @@ describe('getMessagesForNarrow', () => { test('if no outbox messages returns messages with no change', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - '[]': [123], + [HOME_NARROW_STR]: [123], }), messages, outbox: [], @@ -47,7 +48,7 @@ describe('getMessagesForNarrow', () => { test('combine messages and outbox in same narrow', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - '[]': [123], + [HOME_NARROW_STR]: [123], }), messages, outbox: [outboxMessage], @@ -80,7 +81,7 @@ describe('getMessagesForNarrow', () => { test('do not combine messages and outbox in different narrow', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - [JSON.stringify(pmNarrowFromEmail('john@example.com'))]: [123], + [keyFromNarrow(pmNarrowFromEmail('john@example.com'))]: [123], }), messages, outbox: [outboxMessage], @@ -97,7 +98,7 @@ describe('getFirstMessageId', () => { test('return undefined when there are no messages', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - '[]': [], + [HOME_NARROW_STR]: [], }), outbox: [], }); @@ -110,7 +111,7 @@ describe('getFirstMessageId', () => { test('returns first message id', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - '[]': [1, 2, 3], + [HOME_NARROW_STR]: [1, 2, 3], }), messages: { [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */, @@ -130,7 +131,7 @@ describe('getLastMessageId', () => { test('return undefined when there are no messages', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - '[]': [], + [HOME_NARROW_STR]: [], }), messages: {}, outbox: [], @@ -144,7 +145,7 @@ describe('getLastMessageId', () => { test('returns last message id', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - '[]': [1, 2, 3], + [HOME_NARROW_STR]: [1, 2, 3], }), messages: { [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */, diff --git a/src/chat/fetchingReducer.js b/src/chat/fetchingReducer.js index 9a551a9c632..76fdcb4fe38 100644 --- a/src/chat/fetchingReducer.js +++ b/src/chat/fetchingReducer.js @@ -10,7 +10,7 @@ import { } from '../actionConstants'; import { NULL_OBJECT } from '../nullObjects'; import { DEFAULT_FETCHING } from './fetchingSelectors'; -import { isSearchNarrow } from '../utils/narrow'; +import { isSearchNarrow, keyFromNarrow } from '../utils/narrow'; const initialState: FetchingState = NULL_OBJECT; @@ -21,7 +21,7 @@ const messageFetchStart = (state, action) => { return state; } - const key = JSON.stringify(action.narrow); + const key = keyFromNarrow(action.narrow); const currentValue = state[key] || DEFAULT_FETCHING; return { @@ -34,7 +34,7 @@ const messageFetchStart = (state, action) => { }; const messageFetchError = (state, action) => { - const key = JSON.stringify(action.narrow); + const key = keyFromNarrow(action.narrow); if (isSearchNarrow(action.narrow)) { return state; @@ -51,7 +51,7 @@ const messageFetchComplete = (state, action) => { if (isSearchNarrow(action.narrow)) { return state; } - const key = JSON.stringify(action.narrow); + const key = keyFromNarrow(action.narrow); const currentValue = state[key] || DEFAULT_FETCHING; return { diff --git a/src/chat/fetchingSelectors.js b/src/chat/fetchingSelectors.js index 09a5bdb1b95..173af5498aa 100644 --- a/src/chat/fetchingSelectors.js +++ b/src/chat/fetchingSelectors.js @@ -1,9 +1,10 @@ /* @flow strict-local */ import type { Fetching, GlobalState, Narrow } from '../types'; import { getFetching } from '../directSelectors'; +import { keyFromNarrow } from '../utils/narrow'; /** The value implicitly represented by a missing entry in FetchingState. */ export const DEFAULT_FETCHING = { older: false, newer: false }; export const getFetchingForNarrow = (state: GlobalState, narrow: Narrow): Fetching => - getFetching(state)[JSON.stringify(narrow)] || DEFAULT_FETCHING; + getFetching(state)[keyFromNarrow(narrow)] || DEFAULT_FETCHING; diff --git a/src/chat/narrowsReducer.js b/src/chat/narrowsReducer.js index 7e37a275ed0..623122231f4 100644 --- a/src/chat/narrowsReducer.js +++ b/src/chat/narrowsReducer.js @@ -22,6 +22,7 @@ import { MENTIONED_NARROW_STR, STARRED_NARROW_STR, isSearchNarrow, + keyFromNarrow, } from '../utils/narrow'; const initialState: NarrowsState = Immutable.Map(); @@ -31,7 +32,7 @@ const messageFetchComplete = (state, action) => { if (isSearchNarrow(action.narrow)) { return state; } - const key = JSON.stringify(action.narrow); + const key = keyFromNarrow(action.narrow); const fetchedMessageIds = action.messages.map(message => message.id); const replaceExisting = action.anchor === FIRST_UNREAD_ANCHOR || action.anchor === LAST_MESSAGE_ANCHOR; @@ -55,7 +56,7 @@ const eventNewMessage = (state, action) => { const narrowsForMessage = getNarrowsForMessage(message, action.ownUser, flags); narrowsForMessage.forEach(narrow => { - const key = JSON.stringify(narrow); + const key = keyFromNarrow(narrow); const value = stateMutable.get(key); if (!value) { diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index 192c28c2e4d..dc1ffb53660 100644 --- a/src/chat/narrowsSelectors.js +++ b/src/chat/narrowsSelectors.js @@ -28,6 +28,7 @@ import { emailsOfGroupPmNarrow, isMessageInNarrow, caseNarrowDefault, + keyFromNarrow, } from '../utils/narrow'; import { shouldBeMuted } from '../utils/message'; import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects'; @@ -57,7 +58,7 @@ export const outboxMessagesForNarrow: Selector = createSelecto ); export const getFetchedMessageIdsForNarrow = (state: GlobalState, narrow: Narrow) => - getAllNarrows(state).get(JSON.stringify(narrow)) || NULL_ARRAY; + getAllNarrows(state).get(keyFromNarrow(narrow)) || NULL_ARRAY; const getFetchedMessagesForNarrow: Selector = createSelector( getFetchedMessageIdsForNarrow, diff --git a/src/drafts/__tests__/draftsReducer-test.js b/src/drafts/__tests__/draftsReducer-test.js index 9d70e8d371e..46d88e412b3 100644 --- a/src/drafts/__tests__/draftsReducer-test.js +++ b/src/drafts/__tests__/draftsReducer-test.js @@ -3,11 +3,11 @@ import deepFreeze from 'deep-freeze'; import { NULL_OBJECT } from '../../nullObjects'; import draftsReducer from '../draftsReducer'; import { DRAFT_UPDATE } from '../../actionConstants'; -import { topicNarrow } from '../../utils/narrow'; +import { topicNarrow, keyFromNarrow } from '../../utils/narrow'; describe('draftsReducer', () => { const testNarrow = topicNarrow('denmark', 'denmark2'); - const testNarrowStr = JSON.stringify(testNarrow); + const testNarrowStr = keyFromNarrow(testNarrow); describe('DRAFT_UPDATE', () => { test('add a new draft key drafts', () => { diff --git a/src/drafts/__tests__/draftsSelectors-test.js b/src/drafts/__tests__/draftsSelectors-test.js index 18d22f1231b..c33b95d99b5 100644 --- a/src/drafts/__tests__/draftsSelectors-test.js +++ b/src/drafts/__tests__/draftsSelectors-test.js @@ -1,14 +1,14 @@ import deepFreeze from 'deep-freeze'; import { getDraftForNarrow } from '../draftsSelectors'; -import { topicNarrow } from '../../utils/narrow'; +import { topicNarrow, keyFromNarrow } from '../../utils/narrow'; describe('getDraftForNarrow', () => { test('return content of draft if exists', () => { const narrow = topicNarrow('stream', 'topic'); const state = deepFreeze({ drafts: { - [JSON.stringify(narrow)]: 'content', + [keyFromNarrow(narrow)]: 'content', }, }); @@ -21,7 +21,7 @@ describe('getDraftForNarrow', () => { const narrow = topicNarrow('stream', 'topic'); const state = deepFreeze({ drafts: { - [JSON.stringify(narrow)]: 'content', + [keyFromNarrow(narrow)]: 'content', }, }); diff --git a/src/drafts/draftsReducer.js b/src/drafts/draftsReducer.js index 6738a78c8ee..cb6675b3fc3 100644 --- a/src/drafts/draftsReducer.js +++ b/src/drafts/draftsReducer.js @@ -2,11 +2,12 @@ import type { DraftsState, Action } from '../types'; import { DRAFT_UPDATE, LOGOUT, ACCOUNT_SWITCH } from '../actionConstants'; import { NULL_OBJECT } from '../nullObjects'; +import { keyFromNarrow } from '../utils/narrow'; const initialState = NULL_OBJECT; const draftUpdate = (state, action) => { - const narrowStr = JSON.stringify(action.narrow); + const narrowStr = keyFromNarrow(action.narrow); if (action.content.trim().length === 0) { // New content is blank; delete the draft. diff --git a/src/drafts/draftsSelectors.js b/src/drafts/draftsSelectors.js index ed497869471..2b4015b0b03 100644 --- a/src/drafts/draftsSelectors.js +++ b/src/drafts/draftsSelectors.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import type { Narrow, GlobalState } from '../types'; +import { keyFromNarrow } from '../utils/narrow'; export const getDraftForNarrow = (state: GlobalState, narrow: Narrow): string => - state.drafts[JSON.stringify(narrow)] || ''; + state.drafts[keyFromNarrow(narrow)] || ''; diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 99444fc192a..2006b68c64f 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -17,14 +17,14 @@ import { import { FIRST_UNREAD_ANCHOR } from '../../anchor'; import type { Message } from '../../api/modelTypes'; import type { ServerMessage } from '../../api/messages/getMessages'; -import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow'; +import { streamNarrow, HOME_NARROW, HOME_NARROW_STR, keyFromNarrow } from '../../utils/narrow'; import { GravatarURL } from '../../utils/avatar'; import * as eg from '../../__tests__/lib/exampleData'; const mockStore = configureStore([thunk]); const narrow = streamNarrow('some stream'); -const streamNarrowStr = JSON.stringify(narrow); +const streamNarrowStr = keyFromNarrow(narrow); global.FormData = class FormData {}; diff --git a/src/reduxTypes.js b/src/reduxTypes.js index 08d459d4e26..5a1a42d2255 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -70,7 +70,7 @@ export type CaughtUp = {| /** * Info about how completely we know the messages in each narrow. * - * The keys correspond to the keys in `MessagesState`. + * The keys correspond to the keys in `NarrowsState`. * * See `CaughtUp` for details on what each value means. */ @@ -79,6 +79,11 @@ export type CaughtUpState = { [narrow: string]: CaughtUp, }; +/** + * The user's draft message, if any, in each conversation. + * + * The keys correspond to the keys in `NarrowsState`. + */ export type DraftsState = { // TODO(flow-v0.126): Should be exact. See note in src/utils/jsonable.js. [narrow: string]: string, @@ -92,6 +97,8 @@ export type Fetching = {| /** * Info about which narrows we're actively fetching more messages from. * + * The keys correspond to the keys in `NarrowsState`. + * * See also: `CaughtUpState`, `NarrowsState`. */ export type FetchingState = { @@ -175,7 +182,7 @@ export type MuteState = MuteTuple[]; * to; see `MessagesState` for more context. The data here should * correspond exactly to the data in `MessagesState`. * - * Keys are `JSON.stringify`-encoded `Narrow` objects. + * Keys are those given by `keyFromNarrow`. * Values are sorted lists of message IDs. * * See also: diff --git a/src/topics/__tests__/topicsSelectors-test.js b/src/topics/__tests__/topicsSelectors-test.js index 50729663700..7bc1ba47efc 100644 --- a/src/topics/__tests__/topicsSelectors-test.js +++ b/src/topics/__tests__/topicsSelectors-test.js @@ -2,7 +2,7 @@ import Immutable from 'immutable'; import { getTopicsForNarrow, getLastMessageTopic, getTopicsForStream } from '../topicSelectors'; -import { HOME_NARROW, streamNarrow } from '../../utils/narrow'; +import { HOME_NARROW, streamNarrow, keyFromNarrow } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; describe('getTopicsForNarrow', () => { @@ -47,7 +47,7 @@ describe('getLastMessageTopic', () => { const message2 = eg.streamMessage({ id: 2, subject: 'some topic' }); const state = eg.reduxState({ narrows: Immutable.Map({ - [JSON.stringify(narrow)]: [1, 2], + [keyFromNarrow(narrow)]: [1, 2], }), messages: { [message1.id]: message1, diff --git a/src/utils/__tests__/narrow-test.js b/src/utils/__tests__/narrow-test.js index 70be9f78ae9..05f34029074 100644 --- a/src/utils/__tests__/narrow-test.js +++ b/src/utils/__tests__/narrow-test.js @@ -21,9 +21,11 @@ import { isStreamOrTopicNarrow, getNarrowsForMessage, getNarrowForReply, - parseNarrowString, + parseNarrow, STARRED_NARROW, MENTIONED_NARROW, + pm1to1NarrowFromUser, + keyFromNarrow, } from '../narrow'; import type { Narrow, Message } from '../../types'; import * as eg from '../../__tests__/lib/exampleData'; @@ -418,11 +420,46 @@ describe('isSameNarrow', () => { }); }); -describe('parseNarrowString', () => { - test('straightforward arrays are parsed', () => { - expect(parseNarrowString('[]')).toEqual([]); - expect(parseNarrowString('[{"operator":"hey"}]')).toEqual([ - { operator: 'hey' }, - ]); +describe('keyFromNarrow+parseNarrow', () => { + const baseNarrows = [ + ['whole stream', streamNarrow(eg.stream.name)], + ['stream conversation', topicNarrow(eg.stream.name, 'a topic')], + ['1:1 PM conversation, non-self', pm1to1NarrowFromUser(eg.otherUser)], + ['self-1:1 conversation', pm1to1NarrowFromUser(eg.selfUser)], + ['group-PM conversation', pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser])], + ['all-messages ("home")', HOME_NARROW], + ['is:starred', STARRED_NARROW], + ['is:mentioned', MENTIONED_NARROW], + ['all-PMs', ALL_PRIVATE_NARROW], + ['search narrow', SEARCH_NARROW('a query')], + ]; + + // The only character not allowed in Zulip stream names is '\x00'. + // (See `check_stream_name` in zulip.git:zerver/lib/streams.py.) + // Try approximately everything else. + /* eslint-disable no-control-regex */ + const diverseCharacters = eg.diverseCharacters.replace(/\x00/g, ''); + const htmlEntities = 'h & t & &lquo;ml"'; + const awkwardNarrows = [ + ['whole stream about awkward characters', streamNarrow(diverseCharacters)], + ['whole stream about HTML entities', streamNarrow(htmlEntities)], + [ + 'stream conversation about awkward characters', + topicNarrow(diverseCharacters, `regarding ${diverseCharacters}`), + ], + [ + 'stream conversation about HTML entities', + topicNarrow(htmlEntities, `regarding ${htmlEntities}`), + ], + ['search narrow for awkward characters', SEARCH_NARROW(diverseCharacters)], + ['search narrow for HTML entities', SEARCH_NARROW(htmlEntities)], + ]; + + describe('round-trips', () => { + for (const [description, narrow] of [...baseNarrows, ...awkwardNarrows]) { + test(description, () => { + expect(parseNarrow(keyFromNarrow(narrow))).toEqual(narrow); + }); + } }); }); diff --git a/src/utils/narrow.js b/src/utils/narrow.js index bba9bf09192..a40e1f5a019 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -1,6 +1,5 @@ /* @flow strict-local */ import isEqual from 'lodash.isequal'; -import unescape from 'lodash.unescape'; import type { Narrow, Message, Outbox, User, UserOrBot } from '../types'; import { @@ -12,14 +11,14 @@ import { type PmKeyUsers, } from './recipient'; +/* eslint-disable no-use-before-define */ + export const isSameNarrow = (narrow1: Narrow, narrow2: Narrow): boolean => Array.isArray(narrow1) && Array.isArray(narrow2) && isEqual(narrow1, narrow2); -export const parseNarrowString = (narrowStr: string): Narrow => JSON.parse(unescape(narrowStr)); - export const HOME_NARROW: Narrow = []; -export const HOME_NARROW_STR: string = '[]'; +export const HOME_NARROW_STR: string = keyFromNarrow(HOME_NARROW); /** * A PM narrow, either 1:1 or group. @@ -130,15 +129,15 @@ export const specialNarrow = (operand: string): Narrow => [ export const STARRED_NARROW = specialNarrow('starred'); -export const STARRED_NARROW_STR = JSON.stringify(STARRED_NARROW); +export const STARRED_NARROW_STR = keyFromNarrow(STARRED_NARROW); export const MENTIONED_NARROW = specialNarrow('mentioned'); -export const MENTIONED_NARROW_STR = JSON.stringify(MENTIONED_NARROW); +export const MENTIONED_NARROW_STR = keyFromNarrow(MENTIONED_NARROW); export const ALL_PRIVATE_NARROW = specialNarrow('private'); -export const ALL_PRIVATE_NARROW_STR = JSON.stringify(ALL_PRIVATE_NARROW); +export const ALL_PRIVATE_NARROW_STR = keyFromNarrow(ALL_PRIVATE_NARROW); export const streamNarrow = (stream: string): Narrow => [ { @@ -208,7 +207,7 @@ export function caseNarrow(narrow: Narrow, cases: NarrowCases): T { export function caseNarrowPartial(narrow: Narrow, cases: $Shape>): T { const err = (type: string) => (): empty => { - throw new Error(`unexpected ${type} narrow: ${JSON.stringify(narrow)}`); + throw new Error(`unexpected ${type} narrow: ${keyFromNarrow(narrow)}`); }; return caseNarrow( narrow, @@ -251,6 +250,96 @@ export function caseNarrowDefault( ); } +/** + * The key we use for this narrow in our Redux data structures. + * + * See in particular `NarrowsState`, `CaughtUpState`, `FetchingState`, + * and `DraftsState`. + */ +export function keyFromNarrow(narrow: Narrow): string { + // The ":s" bit in several of these is to keep them disjoint, out of an + // abundance of caution, from future keys that use numeric IDs. + return caseNarrow(narrow, { + // NB if you're changing any of these: be sure to do a migration. + // Take a close look at migration 19 and any later related migrations. + + stream: name => `stream:s:${name}`, + // '\x00' is the one character not allowed in Zulip stream names. + // (See `check_stream_name` in zulip.git:zerver/lib/streams.py.) + topic: (streamName, topic) => `topic:s:${streamName}\x00${topic}`, + + pm: emails => `pm:s:${emails.join(',')}`, + + home: () => 'all', + starred: () => 'starred', + mentioned: () => 'mentioned', + allPrivate: () => 'all-pm', + search: query => `search:${query}`, + }); +} + +/** + * Parse a narrow previously encoded with keyFromNarrow. + */ +export const parseNarrow = (narrowStr: string): Narrow => { + const makeError = () => new Error('parseNarrow: bad narrow'); + + const tag = /^.*?(?::|$)/.exec(narrowStr)?.[0] ?? ''; + const rest = narrowStr.substr(tag.length); + // invariant: tag + rest === narrowStr + switch (tag) { + case 'stream:': { + if (!rest.startsWith('s:')) { + throw makeError(); + } + const name = rest.substr('s:'.length); + return streamNarrow(name); + } + + case 'topic:': { + // The `/s` regexp flag means the `.` patterns match absolutely + // anything. By default they reject certain "newline" characters, + // which in principle could appear in stream names or topics. + // eslint-disable-next-line no-control-regex + const match = /^s:(.*?)\x00(.*)/s.exec(rest); + if (!match) { + throw makeError(); + } + return topicNarrow(match[1], match[2]); + } + + case 'pm:': { + if (!rest.startsWith('s:')) { + throw makeError(); + } + const emails = rest.substr('s:'.length).split(','); + return pmNarrowFromEmails(emails); + } + + case 'search:': { + return SEARCH_NARROW(rest); + } + + default: + if (rest !== '') { + throw makeError(); + } + + switch (tag) { + case 'all': + return HOME_NARROW; + case 'starred': + return STARRED_NARROW; + case 'mentioned': + return MENTIONED_NARROW; + case 'all-pm': + return ALL_PRIVATE_NARROW; + default: + throw makeError(); + } + } +}; + export const isHomeNarrow = (narrow?: Narrow): boolean => !!narrow && caseNarrowDefault(narrow, { home: () => true }, () => false); diff --git a/src/utils/recipient.js b/src/utils/recipient.js index bc8483ebb2b..1df7a16c91c 100644 --- a/src/utils/recipient.js +++ b/src/utils/recipient.js @@ -198,9 +198,9 @@ export const pmUiRecipientsFromMessage = ( * * `pmUiRecipientsFromMessage`, which gives a set of users to show in the * UI. * - * * The `Narrow` type and its constructors in `narrow.js`, which with - * `JSON.stringify` we use to make keys to identify narrows in general, - * including stream and topic narrows. + * * The `Narrow` type and its constructors in `narrow.js`, which we use to + * make keys to identify narrows in general, including stream and topic + * narrows. * * * `normalizeRecipients`, `normalizeRecipientsSansMe`, and * `pmUnreadsKeyFromMessage`, which do the same job as this function with diff --git a/src/webview/html/messageHeaderAsHtml.js b/src/webview/html/messageHeaderAsHtml.js index 95b96acb6d7..b474b239d5c 100644 --- a/src/webview/html/messageHeaderAsHtml.js +++ b/src/webview/html/messageHeaderAsHtml.js @@ -2,7 +2,13 @@ import template from './template'; import type { Message, Narrow, Outbox } from '../../types'; import type { BackgroundData } from '../MessageList'; -import { streamNarrow, topicNarrow, caseNarrow, pmNarrowFromRecipients } from '../../utils/narrow'; +import { + streamNarrow, + topicNarrow, + caseNarrow, + pmNarrowFromRecipients, + keyFromNarrow, +} from '../../utils/narrow'; import { foregroundColorFromBackground } from '../../utils/color'; import { humanDate } from '../../utils/date'; import { @@ -42,7 +48,7 @@ export default ( if (item.type === 'stream' && headerStyle === 'topic+date') { const streamName = streamNameOfStreamMessage(item); - const topicNarrowStr = JSON.stringify(topicNarrow(streamName, item.subject)); + const topicNarrowStr = keyFromNarrow(topicNarrow(streamName, item.subject)); const topicHtml = renderSubject(item); return template` @@ -63,8 +69,8 @@ export default ( const backgroundColor = stream ? stream.color : 'hsl(0, 0%, 80%)'; const textColor = foregroundColorFromBackground(backgroundColor); - const streamNarrowStr = JSON.stringify(streamNarrow(streamName)); - const topicNarrowStr = JSON.stringify(topicNarrow(streamName, item.subject)); + const streamNarrowStr = keyFromNarrow(streamNarrow(streamName)); + const topicNarrowStr = keyFromNarrow(topicNarrow(streamName, item.subject)); const topicHtml = renderSubject(item); return template` @@ -86,7 +92,7 @@ export default ( if (item.type === 'private' && headerStyle === 'full') { const keyRecipients = pmKeyRecipientsFromMessage(item, ownUser); const narrowObj = pmNarrowFromRecipients(keyRecipients); - const narrowStr = JSON.stringify(narrowObj); + const narrowStr = keyFromNarrow(narrowObj); const uiRecipients = pmUiRecipientsFromMessage(item, ownUser); return template` diff --git a/src/webview/webViewEventHandlers.js b/src/webview/webViewEventHandlers.js index 553666aadf8..115fc749205 100644 --- a/src/webview/webViewEventHandlers.js +++ b/src/webview/webViewEventHandlers.js @@ -12,7 +12,7 @@ import { showToast } from '../utils/info'; import { isUrlAnImage } from '../utils/url'; import * as logging from '../utils/logging'; import { filterUnreadMessagesInRange } from '../utils/unread'; -import { parseNarrowString } from '../utils/narrow'; +import { parseNarrow } from '../utils/narrow'; import { fetchOlder, fetchNewer, @@ -229,7 +229,7 @@ export const handleMessageListEvent = (props: Props, _: GetText, event: MessageL } case 'narrow': - props.dispatch(doNarrow(parseNarrowString(event.narrow))); + props.dispatch(doNarrow(parseNarrow(event.narrow))); break; case 'image':