From 68c3e86ffabf814227b355d63dacf43cba803a60 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 3 Feb 2025 14:05:41 -0800 Subject: [PATCH] stream_data: Fix parsing of slugs for inaccessible channels. Previously, if we had syntax in a URL slug that looked like a channel ID, but we couldn't find the channel (say, beacuse it's a link to a channel we're not subscribed to), parse_narrow would fail to parse it, resulting in incorrect error handling. This could break rendering of topic links that we want to process via rendered_markdown.ts. I've confirmed that the web app's logic for processing message_view.show does not require its caller to check the channel ID is accessible. The updated logic matches what we do in the mobile apps. --- web/src/stream_data.ts | 6 ++++++ web/tests/hash_util.test.cjs | 6 +++--- web/tests/stream_data.test.cjs | 23 +++++++++++------------ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/web/src/stream_data.ts b/web/src/stream_data.ts index 869da0b757..d08e65f3e3 100644 --- a/web/src/stream_data.ts +++ b/web/src/stream_data.ts @@ -294,6 +294,12 @@ export function slug_to_stream_id(slug: string): number | undefined { return stream.stream_id; } + // Neither format found a channel, so it's inaccessible or doesn't + // exist. But at least we have a stream ID; give that to the caller. + if (newFormatStreamId) { + return newFormatStreamId; + } + // Unexpected shape, or the old shape and we don't know of a stream with // the given name. return undefined; diff --git a/web/tests/hash_util.test.cjs b/web/tests/hash_util.test.cjs index 5031b3e75f..9744b168db 100644 --- a/web/tests/hash_util.test.cjs +++ b/web/tests/hash_util.test.cjs @@ -185,10 +185,10 @@ run_test("test_parse_narrow", () => { assert.equal(hash_util.parse_narrow(["narrow", "BOGUS"]), undefined); - // For nonexistent streams, we get an empty string. We don't use this - // anywhere, and just show "Invalid stream" in the navbar. + // For unknown channel IDs, we still parse it; it could be a valid channel we do + // not have access to. We'll end up showing "Invalid stream" in the navbar. assert.deepEqual(hash_util.parse_narrow(["narrow", "stream", "42-bogus"]), [ - {negated: false, operator: "stream", operand: ""}, + {negated: false, operator: "stream", operand: "42"}, ]); // Empty string as a topic name is valid. diff --git a/web/tests/stream_data.test.cjs b/web/tests/stream_data.test.cjs index 456fabb983..7aa8881e00 100644 --- a/web/tests/stream_data.test.cjs +++ b/web/tests/stream_data.test.cjs @@ -217,18 +217,17 @@ test("basics", () => { assert.equal(stream_data.slug_to_stream_id("social"), 2); assert.equal(hash_util.decode_operand("channel", "social"), "2"); - // These aren't prepended with valid ids nor valid channel names. We - // don't get any stream id from the slug, and the decoded operand (the - // only caller of `slug_to_stream_id`) returns an empty string (which we - // don't display anywhere, since the channel is invalid). - assert.equal(stream_data.slug_to_stream_id("999-social"), undefined); - assert.equal(hash_util.decode_operand("channel", "999-social"), ""); + // These aren't prepended with valid ids nor valid channel names. Return + // the channel ID, since almost all URLs are the modern format and the + // most likely explanation is an inaccessible channel. + assert.equal(stream_data.slug_to_stream_id("999-social"), 999); + assert.equal(hash_util.decode_operand("channel", "999-social"), "999"); - assert.equal(stream_data.slug_to_stream_id("99-whatever"), undefined); - assert.equal(hash_util.decode_operand("channel", "99-whatever"), ""); + assert.equal(stream_data.slug_to_stream_id("99-whatever"), 99); + assert.equal(hash_util.decode_operand("channel", "99-whatever"), "99"); - assert.equal(stream_data.slug_to_stream_id("25-or-6-to-4"), undefined); - assert.equal(hash_util.decode_operand("channel", "25-or-6-to-4"), ""); + assert.equal(stream_data.slug_to_stream_id("25-or-6-to-4"), 25); + assert.equal(hash_util.decode_operand("channel", "25-or-6-to-4"), "25"); // If this is the name of a stream, its id is returned. const stream_starting_with_25 = { @@ -242,8 +241,8 @@ test("basics", () => { assert.equal(stream_data.slug_to_stream_id("2something"), undefined); assert.equal(hash_util.decode_operand("channel", "2something"), ""); - assert.equal(stream_data.slug_to_stream_id("99"), undefined); - assert.equal(hash_util.decode_operand("channel", "99"), ""); + assert.equal(stream_data.slug_to_stream_id("99"), 99); + assert.equal(hash_util.decode_operand("channel", "99"), "99"); // If this is the name of a stream, its id is returned. const stream_99 = { name: "99",