From 996d054fe92ff1d7a4a67f08f559e2cd6adbfd71 Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Fri, 7 Feb 2020 20:18:20 +0000
Subject: [PATCH] messages: Send stream_id for stream messages.

This saves a tiny bit of bandwidth, but more
importantly, it protects us against races for
stream name changes.  There's some argument that
if the user is thinking they're sending to
old_stream_name, and unbeknownst to them, the
stream has changed to new_stream_name, then we
should fail.  But I think 99% of the time the
user just wants the message to go that stream
despite any renames.

In order to verify the blueslip error, we
had to turn on error checking, which required
a tiny fix to a place where we left out
a stream_id for add_sub.
---
 frontend_tests/node_tests/compose.js | 15 +++++++++++----
 static/js/compose.js                 | 10 +++++++++-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js
index 48d2bcd26c..5f66f88d8e 100644
--- a/frontend_tests/node_tests/compose.js
+++ b/frontend_tests/node_tests/compose.js
@@ -1,8 +1,6 @@
 set_global('bridge', false);
 
-set_global('blueslip', global.make_zblueslip({
-    error: false, // Ignore errors. We only check for warnings in this module.
-}));
+set_global('blueslip', global.make_zblueslip({}));
 
 const noop = function () {};
 
@@ -876,6 +874,7 @@ function test_raw_file_drop(raw_drop_func) {
 run_test('warn_if_private_stream_is_linked', () => {
     stream_data.add_sub(compose_state.stream_name(), {
         subscribers: new LazySet([1, 2]),
+        stream_id: 99,
     });
 
     let denmark = {
@@ -1660,7 +1659,15 @@ run_test('create_message_object', () => {
 
 
     let message = compose.create_message_object();
-    assert.equal(message.to, 'social');
+    assert.equal(message.to, sub.stream_id);
+    assert.equal(message.topic, 'lunch');
+    assert.equal(message.content, 'burrito');
+
+    blueslip.set_test_data('error', 'Trying to send message with bad stream name: BOGUS STREAM');
+
+    page['#stream_message_recipient_stream'] = 'BOGUS STREAM';
+    message = compose.create_message_object();
+    assert.equal(message.to, 'BOGUS STREAM');
     assert.equal(message.topic, 'lunch');
     assert.equal(message.content, 'burrito');
 
diff --git a/static/js/compose.js b/static/js/compose.js
index 8864c07336..ebdaa44576 100644
--- a/static/js/compose.js
+++ b/static/js/compose.js
@@ -195,11 +195,19 @@ function create_message_object() {
 
     } else {
         const stream_name = compose_state.stream_name();
-        message.to = stream_name;
         message.stream = stream_name;
         const sub = stream_data.get_sub(stream_name);
         if (sub) {
             message.stream_id = sub.stream_id;
+            message.to = sub.stream_id;
+        } else {
+            // We should be validating streams in calling code.  We'll
+            // try to fall back to stream_name here just in case the
+            // user started composing to the old stream name and
+            // manually entered the stream name, and it got past
+            // validation. We should try to kill this code off eventually.
+            blueslip.error('Trying to send message with bad stream name: ' + stream_name);
+            message.to = stream_name;
         }
         util.set_message_topic(message, topic);
     }