ws: Support Content-Length in the fsread1 channel

Cockpit navigator and SOS report offer a file download option,
downloading a file in the browser shows the download speed but no
estimation of how long it takes. This is due to the lack of the
Content-Length header which could be added via the external channel
options however the `fsread1` channel already has all the information.

The `fsread1` channel now emits a `size-hint` in the ready message which
cockpit-ws picks up and adds a Content-Length header. The header is only
included for the binary channel mode.

Closes: cockpit-project#19861

doc/protocol.md

@@ -1026,6 +1026,9 @@ The following options can be specified in the "open" control message:
 
  * "path": The path name of the file to read.
 
+The ready message contains a "size-hint" when the channel is opened
+with the "binary" option set to "raw".
+
 The channel will return the content of the file in one or more
 messages.  As with "stream", the boundaries of the messages are
 arbitrary.

pkg/base1/test-external.js

@@ -26,6 +26,38 @@ QUnit.test("external get", function (assert) {
     req.send();
 });
 
+QUnit.test("external fsread1", async assert => {
+    const done = assert.async();
+    assert.expect(5);
+    const resp = await cockpit.spawn(["bash", "-c", "size=$(stat --format '%s' /usr/lib/os-release); echo $size"]);
+    const filesize = resp.replace(/\n$/, "");
+
+    /* The query string used to open the channel */
+    const query = window.btoa(JSON.stringify({
+        payload: "fsread1",
+        path: '/usr/lib/os-release',
+        binary: "raw",
+        external: {
+            "content-disposition": 'attachment; filename="foo"',
+            "content-type": "application/octet-stream",
+        }
+    }));
+
+    const req = new XMLHttpRequest();
+    req.open("GET", "/cockpit/channel/" + cockpit.transport.csrf_token + '?' + query);
+    req.onreadystatechange = function() {
+        if (req.readyState == 4) {
+            assert.equal(req.status, 200, "got right status");
+            assert.equal(req.statusText, "OK", "got right reason");
+            assert.equal(req.getResponseHeader("Content-Type"), "application/octet-stream", "default type");
+            assert.equal(req.getResponseHeader("Content-Disposition"), 'attachment; filename="foo"', "default type");
+            assert.equal(req.getResponseHeader("Content-Length"), parseInt(filesize), "expected file size");
+            done();
+        }
+    };
+    req.send();
+});
+
 QUnit.test("external headers", function (assert) {
     const done = assert.async();
     assert.expect(3);

src/bridge/cockpitfsread.c

@@ -282,7 +282,16 @@ cockpit_fsread_prepare (CockpitChannel *channel)
   self->sig_read = g_signal_connect (self->pipe, "read", G_CALLBACK (on_pipe_read), self);
   self->sig_close = g_signal_connect (self->pipe, "close", G_CALLBACK (on_pipe_close), self);
 
-  cockpit_channel_ready (channel, NULL);
+  const gchar *binary;
+  if (S_ISREG(statbuf.st_mode) && cockpit_json_get_string (options, "binary", "", &binary) && g_str_equal (binary, "raw"))
+    {
+      g_autoptr(JsonObject) message = json_object_new ();
+      json_object_set_int_member (message, "size-hint", statbuf.st_size);
+      cockpit_channel_ready (channel, message);
+    } else {
+      cockpit_channel_ready (channel, NULL);
+    }
+
 
 out:
   if (fd >= 0)
@@ -338,6 +347,7 @@ cockpit_fsread_class_init (CockpitFsreadClass *klass)
  * @transport: the transport to send/receive messages on
  * @channel_id: the channel id
  * @path: the path name of the file to read
+ * @binary: set binary to "raw"
  *
  * This function is mainly used by tests. The usual way
  * to get a #CockpitFsread is via cockpit_channel_open()
@@ -347,7 +357,8 @@ cockpit_fsread_class_init (CockpitFsreadClass *klass)
 CockpitChannel *
 cockpit_fsread_open (CockpitTransport *transport,
                      const gchar *channel_id,
-                     const gchar *path)
+                     const gchar *path,
+                     gboolean binary)
 {
   CockpitChannel *channel;
   JsonObject *options;
@@ -357,6 +368,8 @@ cockpit_fsread_open (CockpitTransport *transport,
   options = json_object_new ();
   json_object_set_string_member (options, "path", path);
   json_object_set_string_member (options, "payload", "fsread1");
+  if (binary)
+    json_object_set_string_member (options, "binary", "raw");
 
   channel = g_object_new (COCKPIT_TYPE_FSREAD,
                           "transport", transport,

src/bridge/cockpitfsread.h

@@ -32,7 +32,8 @@ GType              cockpit_fsread_get_type     (void) G_GNUC_CONST;
 
 CockpitChannel *   cockpit_fsread_open         (CockpitTransport *transport,
                                                 const gchar *channel_id,
-                                                const gchar *path);
+                                                const gchar *path,
+						gboolean binary);
 
 gchar *            cockpit_get_file_tag         (const gchar *path);
 

src/bridge/test-fs.c

@@ -90,9 +90,9 @@ setup (TestCase *tc,
 
 static void
 setup_fsread_channel (TestCase *tc,
-                      const gchar *path)
+                      const gchar *path, gboolean binary)
 {
-  tc->channel = cockpit_fsread_open (COCKPIT_TRANSPORT (tc->transport), "1234", path);
+  tc->channel = cockpit_fsread_open (COCKPIT_TRANSPORT (tc->transport), "1234", path, binary);
   tc->channel_closed = FALSE;
   g_signal_connect (tc->channel, "closed", G_CALLBACK (on_channel_close), tc);
   cockpit_channel_prepare (tc->channel);
@@ -282,7 +282,7 @@ test_read_simple (TestCase *tc,
   set_contents (tc->test_path, "Hello!");
   tag = cockpit_get_file_tag (tc->test_path);
 
-  setup_fsread_channel (tc, tc->test_path);
+  setup_fsread_channel (tc, tc->test_path, FALSE);
   wait_channel_closed (tc);
 
   assert_received (tc, "Hello!");
@@ -295,17 +295,37 @@ test_read_simple (TestCase *tc,
 
   control = mock_transport_pop_control (tc->transport);
   g_assert (json_object_get_member (control, "problem") == NULL);
+  /* binary only option */
+  g_assert (json_object_get_member (control, "size-hint") == NULL);
   g_assert_cmpstr (json_object_get_string_member (control, "tag"), ==, tag);
   g_free (tag);
 }
 
+static void
+test_read_binary_size_hint (TestCase *tc,
+                            gconstpointer unused)
+{
+  JsonObject *control;
+  struct stat statbuf;
+
+  set_contents (tc->test_path, "Hello!");
+  stat (tc->test_path, &statbuf);
+
+  setup_fsread_channel (tc, tc->test_path, TRUE);
+  wait_channel_closed (tc);
+
+  control = mock_transport_pop_control (tc->transport);
+  g_assert_cmpstr (json_object_get_string_member (control, "command"), ==, "ready");
+  g_assert_cmpint (json_object_get_int_member (control, "size-hint"), ==, statbuf.st_size);
+}
+
 static void
 test_read_non_existent (TestCase *tc,
                         gconstpointer unused)
 {
   JsonObject *control;
 
-  setup_fsread_channel (tc, "/non/existent");
+  setup_fsread_channel (tc, "/non/existent", FALSE);
   wait_channel_closed (tc);
 
   assert_received (tc, "");
@@ -330,7 +350,7 @@ test_read_denied (TestCase *tc,
   set_contents (tc->test_path, "Hello!");
   g_assert (chmod (tc->test_path, 0) >= 0);
 
-  setup_fsread_channel (tc, tc->test_path);
+  setup_fsread_channel (tc, tc->test_path, FALSE);
   wait_channel_closed (tc);
 
   control = mock_transport_pop_control (tc->transport);
@@ -344,7 +364,7 @@ test_read_changed (TestCase *tc,
   JsonObject *control;
 
   set_contents (tc->test_path, "Hello!");
-  setup_fsread_channel (tc, tc->test_path);
+  setup_fsread_channel (tc, tc->test_path, FALSE);
 
   {
     sleep(1);
@@ -376,7 +396,7 @@ test_read_replaced (TestCase *tc,
   set_contents (tc->test_path, "Hello!");
   tag = cockpit_get_file_tag (tc->test_path);
 
-  setup_fsread_channel (tc, tc->test_path);
+  setup_fsread_channel (tc, tc->test_path, FALSE);
 
   {
     FILE *f = fopen (tc->test_path_2, "w");
@@ -412,7 +432,7 @@ test_read_removed (TestCase *tc,
   set_contents (tc->test_path, "Hello!");
   tag = cockpit_get_file_tag (tc->test_path);
 
-  setup_fsread_channel (tc, tc->test_path);
+  setup_fsread_channel (tc, tc->test_path, FALSE);
 
   {
     g_assert (unlink (tc->test_path) >= 0);
@@ -449,7 +469,7 @@ test_read_non_mmappable (TestCase *tc,
       return;
     }
 
-  setup_fsread_channel (tc, path);
+  setup_fsread_channel (tc, path, FALSE);
   wait_channel_closed (tc);
 
   control = mock_transport_pop_control (tc->transport);
@@ -997,6 +1017,8 @@ main (int argc,
 
   g_test_add ("/fsread/simple", TestCase, NULL,
               setup, test_read_simple, teardown);
+  g_test_add ("/fsread/binary", TestCase, NULL,
+              setup, test_read_binary_size_hint, teardown);
   g_test_add ("/fsread/non-existent", TestCase, NULL,
               setup, test_read_non_existent, teardown);
   g_test_add ("/fsread/denied", TestCase, NULL,

src/cockpit/channels/filesystem.py

@@ -126,7 +126,10 @@ def do_yield_data(self, options: JsonObject) -> GeneratorChannel.DataGenerator:
                 if max_read_size is not None and buf.st_size > max_read_size:
                     raise ChannelError('too-large')
 
-                self.ready()
+                if binary and stat.S_ISREG(buf.st_mode):
+                    self.ready(size_hint=buf.st_size)
+                else:
+                    self.ready()
 
                 while True:
                     data = filep.read1(Channel.BLOCK_SIZE)

src/ws/cockpitchannelresponse.c

@@ -408,6 +408,15 @@ cockpit_channel_response_control (CockpitChannel *channel,
         }
     }
 
+  if (g_str_equal (command, "ready"))
+    {
+      gint64 content_length;
+      if (cockpit_json_get_int (options, "size-hint", -1, &content_length) && content_length != -1)
+        ensure_headers (self, 200, "OK", content_length);
+
+      return TRUE;
+    }
+
   if (g_str_equal (command, "done"))
     {
       ensure_headers (self, 200, "OK", 0);

test/pytest/mocktransport.py

@@ -54,15 +54,17 @@ async def check_open(
         channel=None,
         problem=None,
         reply_keys: Optional[JsonObject] = None,
+        absent_keys: 'Iterable[str]' = (),
         **kwargs,
     ):
         assert isinstance(self.protocol, Router)
         ch = self.send_open(payload, channel, **kwargs)
         if problem is None:
-            await self.assert_msg('', command='ready', channel=ch, **(reply_keys or {}))
+            await self.assert_msg('', command='ready', channel=ch, absent_keys=absent_keys, **(reply_keys or {}))
             # it's possible that the channel already closed
         else:
-            await self.assert_msg('', command='close', channel=ch, problem=problem, **(reply_keys or {}))
+            await self.assert_msg('', command='close', channel=ch, problem=problem, absent_keys=absent_keys,
+                                  **(reply_keys or {}))
             assert ch not in self.protocol.open_channels
         return ch
 
@@ -120,9 +122,12 @@ async def assert_data(self, expected_channel: str, expected_data: bytes) -> None
         assert channel == expected_channel
         assert data == expected_data
 
-    async def assert_msg(self, expected_channel: str, **kwargs: JsonValue) -> JsonObject:
+    async def assert_msg(self, expected_channel: str, absent_keys: 'Iterable[str]' = (),
+                         **kwargs: JsonValue) -> JsonObject:
         msg = await self.next_msg(expected_channel)
         assert msg == dict(msg, **{k.replace('_', '-'): v for k, v in kwargs.items()}), msg
+        for absent_key in absent_keys:
+            assert absent_key not in msg
         return msg
 
     # D-Bus helpers

test/pytest/test_bridge.py

@@ -419,6 +419,29 @@ async def test_fsread1_errors(transport):
                                reply_keys={'message': "attribute 'max_read_size': must have type int"})
 
 
+@pytest.mark.asyncio
+async def test_fsread1_size_hint(transport):
+    data = None
+    stat = os.stat('/usr/lib/os-release')
+    with open('/usr/lib/os-release', 'rb') as fp:
+        data = fp.read()
+    ch = await transport.check_open('fsread1', path='/usr/lib/os-release', binary='raw',
+                                    reply_keys={'size-hint': stat.st_size})
+    await transport.assert_data(ch, data)
+
+
+@pytest.mark.asyncio
+async def test_fsread1_size_hint_absent(transport):
+    # non-binary fsread1 has no size-hint
+    await transport.check_open('fsread1', path='/etc/passwd', absent_keys=['size-hint'])
+
+
+@pytest.mark.asyncio
+async def test_fsread1_size_hint_absent_char_device(transport):
+    # character device fsread1 has no size-hint
+    await transport.check_open('fsread1', path='/dev/null', binary='raw', absent_keys=['size-hint'])
+
+
 @pytest.mark.asyncio
 async def test_fslist1_no_watch(transport):
     tempdir = tempfile.TemporaryDirectory()
@@ -658,7 +681,8 @@ async def open(
             **kwargs: JsonValue
     ) -> 'FsInfoClient':
         channel = await transport.check_open('fsinfo', path=str(path), attrs=attrs,
-                                             fnmatch=fnmatch, reply_keys=None, **kwargs)
+                                             fnmatch=fnmatch, reply_keys=None,
+                                             absent_keys=(), **kwargs)
         return cls(transport, channel)
 
     async def next_state(self) -> JsonObject: