Merge pull request #5833 from grondo/issue#5832

fix handling of `Ctrl-SPACE` (`NUL`) over job ptys (e.g. under `flux job attach`)

src/common/libterminus/client.c

@@ -271,8 +271,8 @@ static void pty_client_data (struct flux_pty_client *c, flux_future_t *f)
         llog_error (c, "unpack: %s", error.text);
         return;
     }
-    if (write (STDIN_FILENO, data, len) < 0) {
-        llog_error (c, "data decode failed: %s", strerror (errno));
+    if (write (STDOUT_FILENO, data, len) < 0) {
+        llog_error (c, "write %zu bytes: %s", len, strerror (errno));
         return;
     }
 }

src/common/libterminus/pty.c

@@ -646,24 +646,37 @@ int pty_data_unpack (const flux_msg_t *msg,
 {
     const char *encoding = NULL;
     void *data;
-    int len;
+    size_t len;
+    json_t *o;
+    json_error_t error;
 
-    if (flux_msg_unpack (msg,
-                         "{s:s s?s}",
-                         "data", &data,
-                         "encoding", &encoding) < 0)
+    /* Note: Use json_unpack_ex(3) in order to pass the JSON_ALLOW_NUL flag.
+     *  This is necessary since pty data packed with 's#' may contain NUL
+     *  characters, especially in the case of ^@/Ctrl-Space/etc, which is
+     *  encoded as a NUL character (empty string with len=1).
+     */
+    if (flux_msg_unpack (msg, "o", &o) < 0)
         return errprintf (errp,
                           "failed to unpack data msg: %s",
                           strerror (errno));
-    len = strlen (data);
-
+    if (json_unpack_ex (o,
+                        NULL,
+                        JSON_ALLOW_NUL,
+                        "{s:s% s?s}",
+                        "data", &data, &len,
+                        "encoding", &encoding) < 0) {
+        errno = EPROTO;
+        return errprintf (errp,
+                          "failed to unpack data msg: %s",
+                          error.text);
+    }
     if (encoding) {
         if (streq (encoding, "base64")) {
             char *b64data = NULL;
             size_t b64len;
             if (decode_base64 (data, len, &b64data, &b64len) < 0) {
                 return errprintf (errp,
-                                  "failed to decode %d bytes of base64",
+                                  "failed to decode %zu bytes of base64",
                                   len);
             }
             if (flux_msg_aux_set (msg, NULL, b64data, free) < 0) {

t/t2612-job-shell-pty.t

@@ -128,4 +128,15 @@ test_expect_success 'pty: no hang when invalid command is run under pty' '
 	test_expect_code 127 run_timeout 15 \
 		flux run -o pty.interactive nosuchcommand
 '
+# Note: test below uses printf(1) to send [Ctrl-SPACE (NUL), newline, Ctrl-D]
+# over the pty connection and expects ^@ (the standard representation of NUL)
+# to appear in output.
+nul_ctrl_d() {
+	python3 -c 'print("\x00\n\x04", end=None)'
+}
+test_expect_success 'pty: NUL (Ctrl-SPACE) character not dropped' '
+	nul_ctrl_d | flux run -vvv -o pty.interactive -o pty.capture cat -v &&
+	flux job eventlog -HLp output $(flux job last) &&
+	flux job eventlog -HLp output $(flux job last) | grep \\^@
+'
 test_done