bridge: Fix text channels with split UTF8 chars across frames

.gitignore

@@ -77,7 +77,7 @@ Makefile.in
 /src/common/fail.html.c
 /src/systemd/cockpit*.service
 /src/systemd/cockpit*.socket
-/src/systemd/cockpit-tempfiles.conf
+/src/systemd/tmpfiles.d/cockpit-ws.conf
 /src/tls/cockpit-certificate-helper
 /src/ws/cockpit-desktop
 /src/ws/cockpit.appdata.xml

containers/ws/cockpit-auth-ssh-key

@@ -42,8 +42,8 @@ def usage():
 
 
 def send_frame(content):
-    data = json.dumps(content).encode('utf-8')
-    os.write(1, str(len(data) + 1).encode('utf-8'))
+    data = json.dumps(content).encode()
+    os.write(1, str(len(data) + 1).encode())
     os.write(1, b"\n\n")
     os.write(1, data)
 
@@ -109,7 +109,7 @@ def read_frame(fd):
         size = size - len(d)
         data += d
 
-    return data.decode("UTF-8")
+    return data.decode()
 
 
 def read_auth_reply():
@@ -129,7 +129,7 @@ def decode_basic_header(response):
     assert response
     assert response.startswith(starts), response
 
-    val = base64.b64decode(response[len(starts):].encode('utf-8')).decode("utf-8")
+    val = base64.b64decode(response[len(starts):].encode()).decode()
     user, password = val.split(':', 1)
     return user, password
 
@@ -150,7 +150,7 @@ cat /run/password""")
 
     pass_fd = os.open("/run/password", os.O_CREAT | os.O_EXCL | os.O_WRONLY | os.O_CLOEXEC, mode=0o600)
     try:
-        os.write(pass_fd, password.encode("UTF-8"))
+        os.write(pass_fd, password.encode())
         os.close(pass_fd)
 
         p = subprocess.run(["ssh-add", "-t", "30", fname],

pkg/base1/test-http.js

@@ -131,6 +131,35 @@ QUnit.test("streaming", assert => {
             });
 });
 
+QUnit.test("split UTF8 frames", assert => {
+    const done = assert.async();
+    assert.expect(1);
+
+    cockpit.http(test_server)
+            .get("/mock/split-utf8")
+            .then(resp => assert.equal(resp, "initialfirst half é second halffinal", "correct response"))
+            .finally(done);
+});
+
+QUnit.test("truncated UTF8 frame", assert => {
+    const done = assert.async();
+    assert.expect(3);
+    let received = "";
+
+    cockpit.http(test_server)
+            .get("/mock/truncated-utf8")
+            .stream(block => { received += block })
+            .then(() => assert.ok(false, "should not have succeeded"))
+            // does not include the first byte of é
+            .catch(ex => {
+                // does not include the first byte of é
+                assert.equal(received, "initialfirst half ", "received expected data");
+                assert.equal(ex.problem, "protocol-error", "error code");
+                assert.ok(ex.message.includes("unexpected end of data"), ex.message);
+            })
+            .finally(done);
+});
+
 QUnit.test("close", assert => {
     const done = assert.async();
     assert.expect(3);

pkg/lib/cockpit.js

@@ -3896,7 +3896,7 @@ function factory() {
 
                 if (options.problem) {
                     http_debug("http problem: ", options.problem);
-                    dfd.reject(new BasicError(options.problem));
+                    dfd.reject(new BasicError(options.problem, options.message));
                 } else {
                     const body = buffer.squash();
 

pkg/storaged/crypto/luksmeta-monitor-hack.py

@@ -21,7 +21,7 @@ def b64_decode(data):
 
 
 def get_clevis_config_from_protected_header(protected_header):
-    header = b64_decode(protected_header).decode("utf-8")
+    header = b64_decode(protected_header).decode()
     header_object = json.loads(header)
     clevis = header_object.get("clevis", None)
     if clevis is None:
@@ -85,7 +85,7 @@ def info(dev):
                                                         "-u", "cb6e8904-81ff-40da-a84a-07ab9ab5715e"],
                                                        stderr=subprocess.PIPE)
                     entry["ClevisConfig"] = {
-                        "v": json.dumps(get_clevis_config_from_jwe(luksmeta.decode("utf-8")))
+                        "v": json.dumps(get_clevis_config_from_jwe(luksmeta.decode()))
                     }
                 except (subprocess.CalledProcessError, FileNotFoundError):
                     pass
@@ -98,7 +98,7 @@ def info(dev):
                 try:
                     token = subprocess.check_output(["cryptsetup", "token", "export", dev, "--token-id", match.group(1)],
                                                     stderr=subprocess.PIPE)
-                    token_object = json.loads(token.decode("utf-8"))
+                    token_object = json.loads(token.decode())
                     if token_object.get("type") == "clevis":
                         config = json.dumps(get_clevis_config_from_protected_header(token_object["jwe"]["protected"]))
                         for slot_str in token_object.get("keyslots", []):

src/build_backend.py

@@ -87,7 +87,7 @@ def write(filename: str, data: AnyStr) -> None:
         def beipack_self(main: str, args: str = '') -> bytes:
             from cockpit._vendor.bei import beipack
             contents = {name: wheel.read(name) for name in wheel.namelist()}
-            pack = beipack.pack(contents, main, args=args).encode('utf-8')
+            pack = beipack.pack(contents, main, args=args).encode()
             return lzma.compress(pack, preset=lzma.PRESET_EXTREME)
 
         def write_distinfo(filename: str, lines: Iterable[str]) -> None:

src/cockpit/channel.py

@@ -16,6 +16,7 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 import asyncio
+import codecs
 import json
 import logging
 import traceback
@@ -111,6 +112,8 @@ class Channel(Endpoint):
     # These get filled in from .do_open()
     channel = ''
     group = ''
+    is_binary: bool
+    decoder: 'codecs.IncrementalDecoder | None'
 
     # input
     def do_control(self, command: str, message: JsonObject) -> None:
@@ -124,6 +127,8 @@ def do_control(self, command: str, message: JsonObject) -> None:
                 self._send_pings = True
             self._ack_bytes = get_enum(message, 'send-acks', ['bytes'], None) is not None
             self.group = get_str(message, 'group', 'default')
+            self.is_binary = get_enum(message, 'binary', ['raw'], None) is not None
+            self.decoder = None
             self.freeze_endpoint()
             self.do_open(message)
         elif command == 'ready':
@@ -217,7 +222,17 @@ def ready(self, **kwargs: JsonValue) -> None:
         self.thaw_endpoint()
         self.send_control(command='ready', **kwargs)
 
+    def __decode_frame(self, data: bytes, *, final: bool = False) -> str:
+        assert self.decoder is not None
+        try:
+            return self.decoder.decode(data, final=final)
+        except UnicodeDecodeError as exc:
+            raise ChannelError('protocol-error', message=str(exc)) from exc
+
     def done(self) -> None:
+        # any residue from partial send_data() frames? this is invalid, fail the channel
+        if self.decoder is not None:
+            self.__decode_frame(b'', final=True)
         self.send_control(command='done')
 
     # tasks and close management
@@ -263,8 +278,8 @@ def close(self, close_args: 'JsonObject | None' = None) -> None:
         if not self._tasks:
             self._close_now()
 
-    def send_data(self, data: bytes) -> bool:
-        """Send data and handle book-keeping for flow control.
+    def send_bytes(self, data: bytes) -> bool:
+        """Send binary data and handle book-keeping for flow control.
 
         The flow control is "advisory".  The data is sent immediately, even if
         it's larger than the window.  In general you should try to send packets
@@ -273,6 +288,10 @@ def send_data(self, data: bytes) -> bool:
         Returns True if there is still room in the window, or False if you
         should stop writing for now.  In that case, `.do_resume_send()` will be
         called later when there is more room.
+
+        Be careful with text channels (i.e. without binary="raw"): you are responsible
+        for ensuring that @data is valid UTF-8. This isn't validated here for
+        efficiency reasons.
         """
         self.send_channel_data(self.channel, data)
 
@@ -284,6 +303,34 @@ def send_data(self, data: bytes) -> bool:
 
         return self._out_sequence < self._out_window
 
+    def send_data(self, data: bytes) -> bool:
+        """Send data and transparently handle UTF-8 for text channels
+
+        Use this for channels which can be text, but are not guaranteed to get
+        valid UTF-8 frames -- i.e. multi-byte characters may be split across
+        frames. This is expensive, so prefer send_text() or send_bytes() wherever
+        possible.
+        """
+        if self.is_binary:
+            return self.send_bytes(data)
+
+        # for text channels we must avoid splitting UTF-8 multi-byte characters,
+        # as these can't be sent to a WebSocket (and are often confusing for text streams as well)
+        if self.decoder is None:
+            self.decoder = codecs.getincrementaldecoder('utf-8')(errors='strict')
+        return self.send_text(self.__decode_frame(data))
+
+    def send_text(self, data: str) -> bool:
+        """Send UTF-8 string data and handle book-keeping for flow control.
+
+        Similar to `send_bytes`, but for text data.  The data is sent as UTF-8 encoded bytes.
+        """
+        return self.send_bytes(data.encode())
+
+    def send_json(self, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> bool:
+        pretty = self.json_encoder.encode(create_object(_msg, kwargs)) + '\n'
+        return self.send_text(pretty)
+
     def do_pong(self, message):
         if not self._send_pings:  # huh?
             logger.warning("Got wild pong on channel %s", self.channel)
@@ -299,10 +346,6 @@ def do_resume_send(self) -> None:
 
     json_encoder: 'ClassVar[json.JSONEncoder]' = json.JSONEncoder(indent=2)
 
-    def send_json(self, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> bool:
-        pretty = self.json_encoder.encode(create_object(_msg, kwargs)) + '\n'
-        return self.send_data(pretty.encode())
-
     def send_control(self, command: str, **kwargs: JsonValue) -> None:
         self.send_channel_control(self.channel, command, None, **kwargs)
 

src/cockpit/channels/filesystem.py

@@ -42,7 +42,6 @@
     JsonError,
     JsonObject,
     get_bool,
-    get_enum,
     get_int,
     get_str,
     get_strv,
@@ -123,7 +122,6 @@ class FsReadChannel(GeneratorChannel):
 
     def do_yield_data(self, options: JsonObject) -> Generator[bytes, None, JsonObject]:
         path = get_str(options, 'path')
-        binary = get_enum(options, 'binary', ['raw'], None) is not None
         max_read_size = get_int(options, 'max_read_size', None)
 
         logger.debug('Opening file "%s" for reading', path)
@@ -134,7 +132,7 @@ def do_yield_data(self, options: JsonObject) -> Generator[bytes, None, JsonObjec
                 if max_read_size is not None and buf.st_size > max_read_size:
                     raise ChannelError('too-large')
 
-                if binary and stat.S_ISREG(buf.st_mode):
+                if self.is_binary and stat.S_ISREG(buf.st_mode):
                     self.ready(size_hint=buf.st_size)
                 else:
                     self.ready()
@@ -144,8 +142,8 @@ def do_yield_data(self, options: JsonObject) -> Generator[bytes, None, JsonObjec
                     if data == b'':
                         break
                     logger.debug('  ...sending %d bytes', len(data))
-                    if not binary:
-                        data = data.replace(b'\0', b'').decode('utf-8', errors='ignore').encode('utf-8')
+                    if not self.is_binary:
+                        data = data.replace(b'\0', b'').decode(errors='ignore').encode()
                     yield data
 
             return {'tag': tag_from_stat(buf)}

src/cockpit/channels/http.py

@@ -21,7 +21,7 @@
 import ssl
 
 from ..channel import AsyncChannel, ChannelError
-from ..jsonutil import JsonObject, get_dict, get_enum, get_int, get_object, get_str, typechecked
+from ..jsonutil import JsonObject, get_dict, get_int, get_object, get_str, typechecked
 
 logger = logging.getLogger(__name__)
 
@@ -97,7 +97,6 @@ def request(
     async def run(self, options: JsonObject) -> None:
         logger.debug('open %s', options)
 
-        binary = get_enum(options, 'binary', ['raw'], None) is not None
         method = get_str(options, 'method')
         path = get_str(options, 'path')
         headers = get_object(options, 'headers', lambda d: {k: typechecked(v, str) for k, v in d.items()}, None)
@@ -133,7 +132,7 @@ async def run(self, options: JsonObject) -> None:
         self.send_control(command='response',
                           status=response.status,
                           reason=response.reason,
-                          headers=self.get_headers(response, binary=binary))
+                          headers=self.get_headers(response, binary=self.is_binary))
 
         # Receive the body and finish up
         try:

src/cockpit/channels/metrics.py

@@ -160,7 +160,7 @@ def send_updates(self, samples: Samples, last_samples: Samples):
             self.send_meta(samples, timestamp)
 
         self.last_timestamp = self.next_timestamp
-        self.send_data(json.dumps([data]).encode())
+        self.send_text(json.dumps([data]))
 
     async def run(self, options):
         self.metrics = []

src/cockpit/channels/packages.py

@@ -36,7 +36,7 @@ class PackagesChannel(AsyncChannel):
     def http_error(self, status: int, message: str) -> None:
         template = read_cockpit_data_file('fail.html')
         self.send_json(status=status, reason='ERROR', headers={'Content-Type': 'text/html; charset=utf-8'})
-        self.send_data(template.replace(b'@@message@@', message.encode('utf-8')))
+        self.send_data(template.replace(b'@@message@@', message.encode()))
         self.done()
         self.close()
 

src/cockpit/channels/trivial.py

@@ -28,8 +28,8 @@ class EchoChannel(Channel):
     def do_open(self, options):
         self.ready()
 
-    def do_data(self, data):
-        self.send_data(data)
+    def do_data(self, data: bytes) -> None:
+        self.send_bytes(data)
 
     def do_done(self):
         self.done()

src/cockpit/misc/print.py

@@ -44,7 +44,7 @@ def data(self, channel: str, /, data: bytes) -> None:
 
     def json(self, channel: str, /, **kwargs: object) -> None:
         """Send a json message (built from **kwargs) on a channel"""
-        self.data(channel, json.dumps(kwargs, indent=2).encode('utf-8') + b'\n')
+        self.data(channel, json.dumps(kwargs, indent=2).encode() + b'\n')
 
     def control(self, command: str, **kwargs: Any) -> None:
         """Send a control message, build from **kwargs"""