fix: address 8-bit output re-review findings

src/client/client_cli.c

@@ -211,7 +211,7 @@ static int apply_table_option(Config* config, const OptionEntry* entry, const ch
 /* Parse CLI arguments into config. Returns 0 on success, -1 on error, 1 for help/clean-exit. */
 int parse_args(Config* config, int argc, char* argv[], int* positional_args,
                int* positional_count) {
-  log_set_8_bit_output(config->eight_bit_output);
+  protocol_set_8_bit_output(config->eight_bit_output);
   for (int i = 1; i < argc; i++) {
     const OptionEntry* entry = find_table_option(argv[i]);
     if (entry) {
@@ -226,7 +226,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
         return -1;
       }
       if (entry->offset == offsetof(Config, eight_bit_output))
-        log_set_8_bit_output(true);
+        protocol_set_8_bit_output(true);
       continue;
     }
 
@@ -302,7 +302,10 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
       log_message(LOG_LEVEL_INFO, "Enabled Chunk Serialization");
     } else if (opt_is(argv[i], "--server-port", NULL) && i + 1 < argc) {
       if (!parse_positive_int(argv[++i], &config->server_port)) {
-        log_message(LOG_LEVEL_ERROR, "invalid --server-port value: %s", argv[i]);
+        char* escaped = output_escape(argv[i], false);
+        log_message(LOG_LEVEL_ERROR, "invalid --server-port value: %s",
+                    escaped ? escaped : "<allocation failed>");
+        free(escaped);
         return -1;
       }
       if (config->server_port > 65535) {
@@ -340,7 +343,10 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
       }
       FILE* lf = fopen(argv[++i], "a");
       if (!lf) {
-        log_message(LOG_LEVEL_ERROR, "could not open log file '%s': %s", argv[i], strerror(errno));
+        char* escaped = output_escape(argv[i], false);
+        log_message(LOG_LEVEL_ERROR, "could not open log file '%s': %s",
+                    escaped ? escaped : "<allocation failed>", strerror(errno));
+        free(escaped);
         return -1;
       }
       config->log_file = lf;
@@ -366,14 +372,18 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
         return -1;
       }
     } else if (argv[i][0] == '-') {
-      fprintf(stderr, "Unknown option: %s\n", argv[i]);
+      char* escaped = output_escape(argv[i], false);
+      fprintf(stderr, "Unknown option: %s\n", escaped ? escaped : "<allocation failed>");
+      free(escaped);
       print_usage();
       return -1;
     } else {
       if (*positional_count < 2)
         positional_args[(*positional_count)++] = i;
       else {
-        fprintf(stderr, "Unexpected argument: %s\n", argv[i]);
+        char* escaped = output_escape(argv[i], false);
+        fprintf(stderr, "Unexpected argument: %s\n", escaped ? escaped : "<allocation failed>");
+        free(escaped);
         print_usage();
         return -1;
       }
@@ -385,7 +395,10 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args,
 static int read_patterns_from_file(const char* filepath, char*** patterns, int* count) {
   FILE* fp = fopen(filepath, "r");
   if (!fp) {
-    log_message(LOG_LEVEL_ERROR, "could not open pattern file '%s': %s", filepath, strerror(errno));
+    char* escaped = output_escape(filepath, false);
+    log_message(LOG_LEVEL_ERROR, "could not open pattern file '%s': %s",
+                escaped ? escaped : "<allocation failed>", strerror(errno));
+    free(escaped);
     return -1;
   }
   char* line = NULL;

src/server/server.c

@@ -232,7 +232,7 @@ void handler(int file_descriptor) {
     protocol_session_unbind();
     return;
   }
-  log_set_8_bit_output(config->eight_bit_output);
+  protocol_set_8_bit_output(config->eight_bit_output);
   if (!authorized_root) {
     log_message(LOG_LEVEL_ERROR, "No server-side destination root configured");
     config_delete(config);
@@ -401,12 +401,17 @@ int main(int argc, char* argv[]) {
       char* end;
       long p = strtol(argv[++i], &end, 10);
       if (*end || p <= 0 || p > 65535) {
-        fprintf(stderr, "Error: invalid port '%s' (must be 1-65535)\n", argv[i]);
+        char* escaped = output_escape(argv[i], false);
+        fprintf(stderr, "Error: invalid port '%s' (must be 1-65535)\n",
+                escaped ? escaped : "<allocation failed>");
+        free(escaped);
         return 1;
       }
       port = (int)p;
     } else if (argv[i][0] == '-') {
-      fprintf(stderr, "Unknown option: %s\n", argv[i]);
+      char* escaped = output_escape(argv[i], false);
+      fprintf(stderr, "Unknown option: %s\n", escaped ? escaped : "<allocation failed>");
+      free(escaped);
       print_server_usage();
       return 1;
     }
@@ -416,7 +421,10 @@ int main(int argc, char* argv[]) {
   signal(SIGINT, cleanup);
   signal(SIGTERM, cleanup);
   if (!configure_authorization(destination_root)) {
-    fprintf(stderr, "Error: invalid destination root '%s'\n", destination_root);
+    char* escaped = output_escape(destination_root, false);
+    fprintf(stderr, "Error: invalid destination root '%s'\n",
+            escaped ? escaped : "<allocation failed>");
+    free(escaped);
     return 1;
   }
   if (stdio_mode) {

src/shared/config.c

@@ -220,8 +220,10 @@ void config_delete(Config* config) {
  * helper call order in config_send and config_receive unchanged when adding
  * fields. */
 static bool send_core_fields(int fd, const Config* c) {
-  return send_str(fd, c->version) && send_int(fd, c->eight_bit_output) &&
-         send_str(fd, c->send_directory) && send_str(fd, c->receive_root_directory) &&
+  if (!send_str(fd, c->version) || !send_int(fd, c->eight_bit_output))
+    return false;
+  protocol_set_8_bit_output(c->eight_bit_output);
+  return send_str(fd, c->send_directory) && send_str(fd, c->receive_root_directory) &&
          send_int(fd, c->save_to_disk) && send_int(fd, c->use_multithreading) &&
          send_int(fd, c->use_chunk_serialization) && send_int(fd, c->use_compression) &&
          send_int(fd, c->use_metadata) && send_int(fd, c->compression_level) &&
@@ -262,7 +264,7 @@ static bool receive_core_fields(int fd, Config* c) {
   int value;
   if (!receive_wire_bool(fd, &c->eight_bit_output))
     return false;
-  log_set_8_bit_output(c->eight_bit_output);
+  protocol_set_8_bit_output(c->eight_bit_output);
   c->send_directory = receive_str(fd);
   c->receive_root_directory = receive_str(fd);
   if (!c->send_directory || !c->receive_root_directory)
@@ -363,8 +365,10 @@ Config* config_receive(int file_descriptor) {
   if (!config->version)
     goto error;
   if (strcmp(config->version, PROTOCOL_VERSION) != 0) {
-    fprintf(stderr, "Protocol version mismatch: client=%s, server=%s\n", config->version,
-            PROTOCOL_VERSION);
+    char* escaped_version = output_escape(config->version, false);
+    fprintf(stderr, "Protocol version mismatch: client=%s, server=%s\n",
+            escaped_version ? escaped_version : "<allocation failed>", PROTOCOL_VERSION);
+    free(escaped_version);
     send_status(file_descriptor, STATUS_ERROR);
     goto error;
   }
@@ -376,7 +380,10 @@ Config* config_receive(int file_descriptor) {
     goto error;
   if (config->compress_choice[0] != '\0' && strcmp(config->compress_choice, "zstd") != 0 &&
       strcmp(config->compress_choice, "none") != 0) {
-    fprintf(stderr, "Unsupported compression choice: %s\n", config->compress_choice);
+    char* escaped_choice = output_escape(config->compress_choice, config->eight_bit_output);
+    fprintf(stderr, "Unsupported compression choice: %s\n",
+            escaped_choice ? escaped_choice : "<allocation failed>");
+    free(escaped_choice);
     send_status(file_descriptor, STATUS_ERROR);
     goto error;
   }

src/shared/log.c

@@ -8,7 +8,7 @@
 static const char* log_level_strings[] = {"DEBUG", "INFO", "WARN", "ERROR"};
 static LogLevel current_log_level = LOG_LEVEL_WARNING;
 static FILE* log_fp = NULL;
-static bool eight_bit_output = false;
+static _Thread_local bool eight_bit_output;
 
 void set_log_level(LogLevel level) {
   current_log_level = level;

src/shared/protocol.c

@@ -45,6 +45,7 @@ void io_set_fds(int read_fd, int write_fd) {
   legacy_io_session.read_fd = read_fd;
   legacy_io_session.write_fd = write_fd;
   legacy_io_session.ssl = NULL;
+  legacy_io_session.eight_bit_output = false;
   legacy_io_session.total_allocated_bytes = 0;
   protocol_session_set_bwlimit(&legacy_io_session, global_bwlimit());
 }
@@ -60,6 +61,7 @@ void protocol_session_init(ProtocolSession* session, int read_fd, int write_fd)
 
 void protocol_session_bind(ProtocolSession* session) {
   bound_session = session;
+  log_set_8_bit_output(session && session->eight_bit_output);
 }
 
 void protocol_session_unbind(void) {
@@ -104,6 +106,19 @@ void protocol_session_set_bwlimit(ProtocolSession* session, unsigned long long b
   session->bw_last_refill_nsec = now.tv_nsec;
 }
 
+void protocol_session_set_8_bit_output(ProtocolSession* session, bool enabled) {
+  if (!session)
+    return;
+  session->eight_bit_output = enabled;
+  if (session == bound_session)
+    log_set_8_bit_output(enabled);
+}
+
+void protocol_set_8_bit_output(bool enabled) {
+  ProtocolSession* session = bound_session ? bound_session : &legacy_io_session;
+  protocol_session_set_8_bit_output(session, enabled);
+}
+
 static void bw_throttle_session(ProtocolSession* session, size_t bytes_written) {
   if (session->bwlimit == 0)
     return;
@@ -329,7 +344,7 @@ bool protocol_send_str(ProtocolSession* session, const char* data) {
     return false;
   if (!protocol_send_n_data(session, data, size))
     return false;
-  char* escaped = output_escape(data, log_get_8_bit_output());
+  char* escaped = output_escape(data, session->eight_bit_output);
   log_message(LOG_LEVEL_DEBUG, "Send String: %s", escaped ? escaped : "<allocation failed>");
   free(escaped);
   return true;
@@ -359,7 +374,7 @@ char* protocol_receive_str(ProtocolSession* session) {
   }
   data[size] = '\0';
   session->total_allocated_bytes += size + 1;
-  char* escaped = output_escape(data, log_get_8_bit_output());
+  char* escaped = output_escape(data, session->eight_bit_output);
   log_message(LOG_LEVEL_DEBUG, "Received String: %s", escaped ? escaped : "<allocation failed>");
   free(escaped);
   return data;

src/shared/protocol.h

@@ -36,6 +36,7 @@ typedef struct ProtocolSession {
   long long bw_last_refill_sec;
   long bw_last_refill_nsec;
   unsigned long long total_allocated_bytes;
+  bool eight_bit_output;
 } ProtocolSession;
 
 typedef int Status;
@@ -65,6 +66,8 @@ void protocol_session_bind(ProtocolSession* session);
 void protocol_session_unbind(void);
 void protocol_session_set_ssl(ProtocolSession* session, SSL* ssl);
 void protocol_session_set_bwlimit(ProtocolSession* session, unsigned long long bytes_per_sec);
+void protocol_session_set_8_bit_output(ProtocolSession* session, bool enabled);
+void protocol_set_8_bit_output(bool enabled);
 bool protocol_send_n_data(ProtocolSession* session, const void* data, size_t data_size);
 bool protocol_receive_n_data(ProtocolSession* session, void* data, size_t data_size);
 bool protocol_send_str(ProtocolSession* session, const char* data);

src/shared/transport_ssh.c

@@ -71,7 +71,9 @@ static int parse_remote_dest(const char* dest, RemoteDest* r) {
 Client* client_connect_ssh(const char* destination, int port, const char* server_path) {
   RemoteDest r;
   if (parse_remote_dest(destination, &r) != 0) {
-    fprintf(stderr, "Invalid remote destination: %s\n", destination);
+    char* escaped = output_escape(destination, false);
+    fprintf(stderr, "Invalid remote destination: %s\n", escaped ? escaped : "<allocation failed>");
+    free(escaped);
     return NULL;
   }
 
@@ -169,8 +171,11 @@ Client* client_connect_ssh(const char* destination, int port, const char* server
     close(sv[0]);
     waitpid(pid, NULL, 0);
     remote_dest_destroy(&r);
+    const char* path = server_path ? server_path : "fastsync-server";
+    char* escaped = output_escape(path, false);
     fprintf(stderr, "Error: could not launch '%s --stdio' on remote\n",
-            server_path ? server_path : "fastsync-server");
+            escaped ? escaped : "<allocation failed>");
+    free(escaped);
     return NULL;
   }
 

src/shared/transport_tcp.c

@@ -1,6 +1,7 @@
 #include "transport_tcp.h"
 #include "log.h"
 #include "protocol.h"
+#include "utils.h"
 #include <arpa/inet.h>
 #include <errno.h>
 #include <netdb.h>
@@ -190,7 +191,10 @@ bool tcp_connect_socket(Client* client, char* host, int port) {
 
   int err = getaddrinfo(host, port_str, &hints, &result);
   if (err != 0 || result == NULL) {
-    fprintf(stderr, "Could not resolve host: %s (%s)\n", host, gai_strerror(err));
+    char* escaped_host = output_escape(host, false);
+    fprintf(stderr, "Could not resolve host: %s (%s)\n",
+            escaped_host ? escaped_host : "<allocation failed>", gai_strerror(err));
+    free(escaped_host);
     return false;
   }
 

src/shared/transport_tls.c

@@ -2,6 +2,7 @@
 #include "log.h"
 #include "protocol.h"
 #include "transport_tcp.h"
+#include "utils.h"
 #include <arpa/inet.h>
 #include <openssl/err.h>
 #include <openssl/ssl.h>
@@ -65,13 +66,19 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
       return NULL;
     }
     if (SSL_CTX_use_certificate_file(ctx, cert, SSL_FILETYPE_PEM) <= 0) {
-      log_message(LOG_LEVEL_ERROR, "Failed to load certificate: %s", cert);
+      char* escaped = output_escape(cert, false);
+      log_message(LOG_LEVEL_ERROR, "Failed to load certificate: %s",
+                  escaped ? escaped : "<allocation failed>");
+      free(escaped);
       log_ssl_errors();
       SSL_CTX_free(ctx);
       return NULL;
     }
     if (SSL_CTX_use_PrivateKey_file(ctx, key, SSL_FILETYPE_PEM) <= 0) {
-      log_message(LOG_LEVEL_ERROR, "Failed to load private key: %s", key);
+      char* escaped = output_escape(key, false);
+      log_message(LOG_LEVEL_ERROR, "Failed to load private key: %s",
+                  escaped ? escaped : "<allocation failed>");
+      free(escaped);
       log_ssl_errors();
       SSL_CTX_free(ctx);
       return NULL;
@@ -85,7 +92,10 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
 
   if (ca_path) {
     if (!SSL_CTX_load_verify_locations(ctx, ca_path, NULL)) {
-      log_message(LOG_LEVEL_ERROR, "Failed to load CA: %s", ca_path);
+      char* escaped = output_escape(ca_path, false);
+      log_message(LOG_LEVEL_ERROR, "Failed to load CA: %s",
+                  escaped ? escaped : "<allocation failed>");
+      free(escaped);
       log_ssl_errors();
       SSL_CTX_free(ctx);
       return NULL;

tests/test_shared_utils.c

@@ -1,8 +1,27 @@
 #include "test_shared_utils.h"
 #include "utils.h"
+#include "protocol.h"
 #include "test_utils.h"
 #include <stdlib.h>
 #include <string.h>
+#include <threads.h>
+
+typedef struct {
+  bool eight_bit_output;
+  const char* expected;
+  int failed;
+} EscapeThreadArgs;
+
+static int escape_thread(void* arg) {
+  EscapeThreadArgs* args = arg;
+  for (int i = 0; i < 1000; i++) {
+    char* escaped = output_escape("x\xc3\xa9\n", args->eight_bit_output);
+    if (!escaped || strcmp(escaped, args->expected) != 0)
+      args->failed = 1;
+    free(escaped);
+  }
+  return 0;
+}
 
 void test_shared_utils() {
   char high_bit[] = {'a', (char)0xc3, (char)0xa9, '\n', '\0'};
@@ -13,6 +32,26 @@ void test_shared_utils() {
   EXPECT_EQ_STR(escaped, "a\xc3\xa9\\#012");
   free(escaped);
 
+  ProtocolSession safe_session;
+  ProtocolSession eight_bit_session;
+  protocol_session_init(&safe_session, -1, -1);
+  protocol_session_init(&eight_bit_session, -1, -1);
+  protocol_session_set_8_bit_output(&safe_session, false);
+  protocol_session_set_8_bit_output(&eight_bit_session, true);
+  EXPECT_FALSE(safe_session.eight_bit_output);
+  EXPECT_TRUE(eight_bit_session.eight_bit_output);
+
+  EscapeThreadArgs safe_args = {false, "x\\#303\\#251\\#012", 0};
+  EscapeThreadArgs eight_bit_args = {true, "x\xc3\xa9\\#012", 0};
+  thrd_t safe_thread;
+  thrd_t eight_bit_thread;
+  EXPECT_EQ_INT(thrd_create(&safe_thread, escape_thread, &safe_args), thrd_success);
+  EXPECT_EQ_INT(thrd_create(&eight_bit_thread, escape_thread, &eight_bit_args), thrd_success);
+  EXPECT_EQ_INT(thrd_join(safe_thread, NULL), thrd_success);
+  EXPECT_EQ_INT(thrd_join(eight_bit_thread, NULL), thrd_success);
+  EXPECT_FALSE(safe_args.failed);
+  EXPECT_FALSE(eight_bit_args.failed);
+
   // Test str_dup
   const char* dup_null = str_dup(NULL);
   EXPECT_NULL(dup_null);