mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-04 05:53:43 +00:00 
			
		
		
		
	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.
This commit is contained in:
		@@ -294,6 +294,12 @@ export function slug_to_stream_id(slug: string): number | undefined {
 | 
				
			|||||||
        return stream.stream_id;
 | 
					        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
 | 
					    // Unexpected shape, or the old shape and we don't know of a stream with
 | 
				
			||||||
    // the given name.
 | 
					    // the given name.
 | 
				
			||||||
    return undefined;
 | 
					    return undefined;
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -185,10 +185,10 @@ run_test("test_parse_narrow", () => {
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    assert.equal(hash_util.parse_narrow(["narrow", "BOGUS"]), undefined);
 | 
					    assert.equal(hash_util.parse_narrow(["narrow", "BOGUS"]), undefined);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    // For nonexistent streams, we get an empty string. We don't use this
 | 
					    // For unknown channel IDs, we still parse it; it could be a valid channel we do
 | 
				
			||||||
    // anywhere, and just show "Invalid stream" in the navbar.
 | 
					    // not have access to. We'll end up showing "Invalid stream" in the navbar.
 | 
				
			||||||
    assert.deepEqual(hash_util.parse_narrow(["narrow", "stream", "42-bogus"]), [
 | 
					    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.
 | 
					    // Empty string as a topic name is valid.
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -217,18 +217,17 @@ test("basics", () => {
 | 
				
			|||||||
    assert.equal(stream_data.slug_to_stream_id("social"), 2);
 | 
					    assert.equal(stream_data.slug_to_stream_id("social"), 2);
 | 
				
			||||||
    assert.equal(hash_util.decode_operand("channel", "social"), "2");
 | 
					    assert.equal(hash_util.decode_operand("channel", "social"), "2");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    // These aren't prepended with valid ids nor valid channel names. We
 | 
					    // These aren't prepended with valid ids nor valid channel names. Return
 | 
				
			||||||
    // don't get any stream id from the slug, and the decoded operand (the
 | 
					    // the channel ID, since almost all URLs are the modern format and the
 | 
				
			||||||
    // only caller of `slug_to_stream_id`) returns an empty string (which we
 | 
					    // most likely explanation is an inaccessible channel.
 | 
				
			||||||
    // don't display anywhere, since the channel is invalid).
 | 
					    assert.equal(stream_data.slug_to_stream_id("999-social"), 999);
 | 
				
			||||||
    assert.equal(stream_data.slug_to_stream_id("999-social"), undefined);
 | 
					    assert.equal(hash_util.decode_operand("channel", "999-social"), "999");
 | 
				
			||||||
    assert.equal(hash_util.decode_operand("channel", "999-social"), "");
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
    assert.equal(stream_data.slug_to_stream_id("99-whatever"), undefined);
 | 
					    assert.equal(stream_data.slug_to_stream_id("99-whatever"), 99);
 | 
				
			||||||
    assert.equal(hash_util.decode_operand("channel", "99-whatever"), "");
 | 
					    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(stream_data.slug_to_stream_id("25-or-6-to-4"), 25);
 | 
				
			||||||
    assert.equal(hash_util.decode_operand("channel", "25-or-6-to-4"), "");
 | 
					    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.
 | 
					    // If this is the name of a stream, its id is returned.
 | 
				
			||||||
    const stream_starting_with_25 = {
 | 
					    const stream_starting_with_25 = {
 | 
				
			||||||
@@ -242,8 +241,8 @@ test("basics", () => {
 | 
				
			|||||||
    assert.equal(stream_data.slug_to_stream_id("2something"), undefined);
 | 
					    assert.equal(stream_data.slug_to_stream_id("2something"), undefined);
 | 
				
			||||||
    assert.equal(hash_util.decode_operand("channel", "2something"), "");
 | 
					    assert.equal(hash_util.decode_operand("channel", "2something"), "");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    assert.equal(stream_data.slug_to_stream_id("99"), undefined);
 | 
					    assert.equal(stream_data.slug_to_stream_id("99"), 99);
 | 
				
			||||||
    assert.equal(hash_util.decode_operand("channel", "99"), "");
 | 
					    assert.equal(hash_util.decode_operand("channel", "99"), "99");
 | 
				
			||||||
    // If this is the name of a stream, its id is returned.
 | 
					    // If this is the name of a stream, its id is returned.
 | 
				
			||||||
    const stream_99 = {
 | 
					    const stream_99 = {
 | 
				
			||||||
        name: "99",
 | 
					        name: "99",
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user