Merge pull request 'Fix 11 Gitea issues — bugs, quality, security, and refactoring' (#47) from fix/gitea-issues into main

Reviewed-on: #47

src/shared/compression.c

@@ -7,7 +7,6 @@
 #define INITIAL_DECOMPRESS_BUF_SIZE (1024 * 1024)
 
 Data* data_compress(Data* data_to_compress, int compression_level) {
-  (void)compression_level;
   log_message(LOG_LEVEL_DEBUG, "Starting to compress data");
   size_t dst_size = ZSTD_compressBound(data_to_compress->size);
   Data* compressed_data = data_create_empty(dst_size);
@@ -21,6 +20,14 @@ Data* data_compress(Data* data_to_compress, int compression_level) {
     return NULL;
   }
 
+  size_t zret = ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, compression_level);
+  if (ZSTD_isError(zret)) {
+    log_message(LOG_LEVEL_ERROR, "Failed to set compression level: %s", ZSTD_getErrorName(zret));
+    ZSTD_freeCCtx(cctx);
+    data_destroy(compressed_data);
+    return NULL;
+  }
+
   ZSTD_inBuffer input = {data_to_compress->data, data_to_compress->size, 0};
   ZSTD_outBuffer output = {compressed_data->data, dst_size, 0};
 

src/shared/config.c

@@ -45,6 +45,8 @@ Config* config_create(char* version, char* send_directory, char* receive_directo
   config->tls_cert = NULL;
   config->tls_key = NULL;
   config->tls_ca = NULL;
+  config->server_host = str_dup("127.0.0.1");
+  config->server_port = 8080;
   return config;
 }
 
@@ -88,6 +90,7 @@ void config_delete(Config* config) {
   free(config->tls_cert);
   free(config->tls_key);
   free(config->tls_ca);
+  free(config->server_host);
   free(config);
 }
 
@@ -110,7 +113,7 @@ bool config_send(int file_descriptor, const Config* config) {
     return false;
   if (!send_int(file_descriptor, config->compression_level))
     return false;
-  if (!send_int(file_descriptor, (int)config->chunk_size))
+  if (!send_n_data(file_descriptor, &config->chunk_size, sizeof(config->chunk_size)))
     return false;
   if (!send_int(file_descriptor, config->use_sendfile))
     return false;
@@ -138,6 +141,7 @@ Config* config_receive(int file_descriptor) {
   Config* config = (Config*)malloc(sizeof(Config));
   if (config == NULL)
     return NULL;
+  memset(config, 0, sizeof(*config));
   config->version = receive_str(file_descriptor);
   if (!config->version) {
     free(config);
@@ -183,9 +187,8 @@ Config* config_receive(int file_descriptor) {
   if (!receive_int(file_descriptor, &tmp))
     goto error;
   config->compression_level = tmp;
-  if (!receive_int(file_descriptor, &tmp))
+  if (!receive_n_data(file_descriptor, &config->chunk_size, sizeof(config->chunk_size)))
     goto error;
-  config->chunk_size = (unsigned long long)tmp;
   if (!receive_int(file_descriptor, &tmp))
     goto error;
   config->use_sendfile = tmp;
@@ -218,6 +221,8 @@ Config* config_receive(int file_descriptor) {
   config->tls_cert = NULL;
   config->tls_key = NULL;
   config->tls_ca = NULL;
+  config->server_host = str_dup("127.0.0.1");
+  config->server_port = 8080;
   if (!send_status(file_descriptor, STATUS_OK))
     goto error;
   return config;
@@ -226,6 +231,7 @@ error:
   free(config->version);
   free(config->send_directory);
   free(config->receive_root_directory);
+  free(config->server_host);
   free(config);
   return NULL;
 }

src/shared/config.h

@@ -35,12 +35,14 @@ typedef struct Config {
   uint32_t delta_block_size;
   unsigned long long delta_max_file_size;
   bool use_tls;
+  char* server_host;
+  int server_port;
   char* tls_cert;
   char* tls_key;
   char* tls_ca;
 } Config;
 
-#define PROTOCOL_VERSION "1.2.0"
+#define PROTOCOL_VERSION "1.3.0"
 #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024)
 
 Config* config_create(char* version, char* send_directory, char* receive_directory,

src/shared/file.c

@@ -409,33 +409,54 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) {
 }
 
 bool to_disk(const char* path, const void* data, unsigned long long data_size) {
-  char* directory = str_dup(path);
-  char* dir_to_free = directory;
-  directory = dirname(directory);
-  if (!mkdir_r(directory)) {
-    free(dir_to_free);
+  // dirname() may modify its argument and may return a pointer to static storage.
+  // We must use a copy of the result to be safe.
+  char* path_dup = str_dup(path);
+  if (!path_dup)
     return false;
+  const char* dir_result = dirname(path_dup);
+  char* directory = str_dup(dir_result);
+  free(path_dup);
+  if (!directory)
+    return false;
+
+  bool ok = true;
+  if (!mkdir_r(directory)) {
+    ok = false;
+    goto done;
   }
   FILE* file_pointer = fopen(path, "wb");
   if (file_pointer == NULL) {
     perror("Could not open File");
-    free(dir_to_free);
-    return false;
+    ok = false;
+    goto done;
   }
   if (fwrite(data, 1, data_size, file_pointer) != data_size) {
     perror("Failed to write all data to disk");
     fclose(file_pointer);
-    free(dir_to_free);
-    return false;
+    ok = false;
+    goto done;
   }
   fclose(file_pointer);
-  free(dir_to_free);
-  return true;
+
+done:
+  free(directory);
+  return ok;
 }
 
 bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level,
                         bool send_path) {
-  (void)compression_level;
+  // sendfile is incompatible with compression (kernel zero-copy).
+  // If compression is requested, fall back to the regular send path.
+  // NOTE: This is a safety net only — callers must ensure compression_level == 0
+  // before calling file_send_sendfile. The fallback to file_send_single_calls
+  // preserves the send_path contract, but callers should not rely on it for
+  // correctness (the --sendfile flag is validated to be mutually exclusive with
+  // -c/--compress at the CLI layer).
+  if (compression_level > 0)
+    return file_send_single_calls(file, file_descriptor, use_metadata, compression_level,
+                                  send_path);
+
   if (send_path && !send_str(file_descriptor, file->path))
     return false;
   if (use_metadata && !metadata_send(file_descriptor, file->metadata))

src/shared/metadata.c

@@ -1,6 +1,8 @@
 #include "metadata.h"
 #include "file.h"
+#include "log.h"
 #include "protocol.h"
+#include <errno.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
@@ -96,13 +98,15 @@ FileMetadata* metadata_receive(int file_descriptor, int* ok) {
 void file_restore_metadata(const char* path, FileMetadata* metadata) {
   if (metadata == NULL)
     return;
-  chmod(path, metadata->mode & 07777);
-  int chown_ret = chown(path, metadata->uid, metadata->gid);
-  (void)chown_ret;
+  if (chmod(path, metadata->mode & 07777) != 0)
+    log_message(LOG_LEVEL_WARNING, "Failed to chmod %s: %s", path, strerror(errno));
+  if (chown(path, metadata->uid, metadata->gid) != 0)
+    log_message(LOG_LEVEL_WARNING, "Failed to chown %s: %s", path, strerror(errno));
   struct timespec times[2];
   times[0].tv_sec = 0;
   times[0].tv_nsec = UTIME_OMIT;
   times[1].tv_sec = metadata->mtime_sec;
   times[1].tv_nsec = metadata->mtime_nsec;
-  utimensat(AT_FDCWD, path, times, 0);
+  if (utimensat(AT_FDCWD, path, times, 0) != 0)
+    log_message(LOG_LEVEL_WARNING, "Failed to set timestamps on %s: %s", path, strerror(errno));
 }

src/shared/transport_ssh.c

@@ -1,4 +1,5 @@
 #include "transport_ssh.h"
+#include "utils.h"
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -8,39 +9,58 @@
 #include <unistd.h>
 
 typedef struct {
-  char user[256];
-  char host[256];
-  char remote_path[4096];
+  char* user;
+  char* host;
+  char* remote_path;
 } RemoteDest;
 
+static void remote_dest_destroy(RemoteDest* r) {
+  free(r->user);
+  free(r->host);
+  free(r->remote_path);
+}
+
 static int parse_remote_dest(const char* dest, RemoteDest* r) {
+  memset(r, 0, sizeof(*r));
   const char* colon = strchr(dest, ':');
   if (!colon)
     return -1;
 
-  size_t remote_path_len = strlen(colon + 1);
-  if (remote_path_len >= sizeof(r->remote_path))
+  r->remote_path = str_dup(colon + 1);
+  if (!r->remote_path)
     return -1;
-  memcpy(r->remote_path, colon + 1, remote_path_len + 1);
 
   const char* at = memchr(dest, '@', colon - dest);
   if (at) {
     size_t user_len = at - dest;
-    if (user_len >= sizeof(r->user))
+    r->user = malloc(user_len + 1);
+    if (!r->user) {
+      remote_dest_destroy(r);
       return -1;
+    }
     memcpy(r->user, dest, user_len);
     r->user[user_len] = '\0';
 
     size_t host_len = colon - at - 1;
-    if (host_len >= sizeof(r->host))
+    r->host = malloc(host_len + 1);
+    if (!r->host) {
+      remote_dest_destroy(r);
       return -1;
+    }
     memcpy(r->host, at + 1, host_len);
     r->host[host_len] = '\0';
   } else {
-    r->user[0] = '\0';
-    size_t host_len = colon - dest;
-    if (host_len >= sizeof(r->host))
+    r->user = str_dup("");
+    if (!r->user) {
+      remote_dest_destroy(r);
       return -1;
+    }
+    size_t host_len = colon - dest;
+    r->host = malloc(host_len + 1);
+    if (!r->host) {
+      remote_dest_destroy(r);
+      return -1;
+    }
     memcpy(r->host, dest, host_len);
     r->host[host_len] = '\0';
   }
@@ -57,6 +77,7 @@ Client* client_connect_ssh(const char* destination, int port) {
   int sv[2];
   if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) < 0) {
     perror("socketpair failed");
+    remote_dest_destroy(&r);
     return NULL;
   }
 
@@ -71,6 +92,7 @@ Client* client_connect_ssh(const char* destination, int port) {
     perror("pipe failed");
     close(sv[0]);
     close(sv[1]);
+    remote_dest_destroy(&r);
     return NULL;
   }
 
@@ -81,6 +103,7 @@ Client* client_connect_ssh(const char* destination, int port) {
     close(sv[1]);
     close(exec_pipe[0]);
     close(exec_pipe[1]);
+    remote_dest_destroy(&r);
     return NULL;
   }
 
@@ -88,7 +111,6 @@ Client* client_connect_ssh(const char* destination, int port) {
     close(sv[0]);
     close(exec_pipe[0]);
     fcntl(exec_pipe[1], F_SETFD, FD_CLOEXEC);
-
     if (sv[1] != STDIN_FILENO)
       dup2(sv[1], STDIN_FILENO);
     if (sv[1] != STDOUT_FILENO)
@@ -97,7 +119,7 @@ Client* client_connect_ssh(const char* destination, int port) {
       close(sv[1]);
 
     char ssh_user[512];
-    if (r.user[0] != '\0')
+    if (r.user && r.user[0] != '\0')
       snprintf(ssh_user, sizeof(ssh_user), "%s@%s", r.user, r.host);
     else
       snprintf(ssh_user, sizeof(ssh_user), "%s", r.host);
@@ -138,10 +160,13 @@ Client* client_connect_ssh(const char* destination, int port) {
   if (n > 0) {
     close(sv[0]);
     waitpid(pid, NULL, 0);
+    remote_dest_destroy(&r);
     fprintf(stderr, "Error: could not launch 'fastsync-server --stdio' on remote\n");
     return NULL;
   }
 
+  remote_dest_destroy(&r);
+
   Client* client = malloc(sizeof(Client));
   if (client == NULL) {
     close(sv[0]);

src/shared/utils.c

@@ -2,6 +2,7 @@
 #include "array_list.h"
 #include "libgen.h"
 #include <dirent.h>
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -26,7 +27,8 @@ bool mkdir_r(const char* path) {
     path_current[0] = '\0';
   }
   const char* delimiter = "/";
-  const char* part = strtok(path_duplicate, delimiter);
+  char* saveptr;
+  const char* part = strtok_r(path_duplicate, delimiter, &saveptr);
   bool ok = true;
   while (part != NULL) {
     strcpy(path_current_position, part);
@@ -41,7 +43,7 @@ bool mkdir_r(const char* path) {
         break;
       }
     }
-    part = strtok(NULL, delimiter);
+    part = strtok_r(NULL, delimiter, &saveptr);
   }
   free(path_duplicate);
   free(path_current);
@@ -81,10 +83,22 @@ bool glob_match(const char* pattern, const char* str) {
   return *str == '\0';
 }
 
+static bool is_dir_in_manifest(const char* rel_path, ArrayList* manifest) {
+  size_t len = strlen(rel_path);
+  for (int i = 0; i < manifest->size; i++) {
+    const char* entry = (const char*)manifest->items[i];
+    // Check if entry starts with rel_path + '/' or matches exactly
+    if (strncmp(entry, rel_path, len) == 0 && (entry[len] == '/' || entry[len] == '\0'))
+      return true;
+  }
+  return false;
+}
+
 static void delete_extras_walk(const char* abs_path, const char* rel_path, ArrayList* manifest) {
   DIR* dir = opendir(abs_path);
   if (!dir)
     return;
+  bool all_removed = true;
   struct dirent* entry;
   while ((entry = readdir(dir)) != NULL) {
     if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
@@ -99,6 +113,11 @@ static void delete_extras_walk(const char* abs_path, const char* rel_path, Array
     }
     if (S_ISDIR(st.st_mode)) {
       delete_extras_walk(child_abs, child_rel, manifest);
+      // After recursion, try to remove the subdirectory if it's now empty.
+      // Ignore ENOENT: the recursive call may have already removed it.
+      if (rmdir(child_abs) != 0 && errno != ENOENT) {
+        all_removed = false;
+      }
     } else {
       // Check if relative path is in manifest
       bool found = false;
@@ -111,13 +130,19 @@ static void delete_extras_walk(const char* abs_path, const char* rel_path, Array
       if (!found) {
         unlink(child_abs);
         fprintf(stderr, "  Deleted: %s\n", child_rel);
+      } else {
+        all_removed = false;
       }
     }
     free(child_abs);
     free(child_rel);
   }
   closedir(dir);
-  rmdir(abs_path);
+  // Only remove the directory itself if it is not in the manifest
+  // and contained no kept entries.
+  if (all_removed && rel_path[0] != '\0' && !is_dir_in_manifest(rel_path, manifest)) {
+    rmdir(abs_path);
+  }
 }
 
 void delete_extras(const char* dest_root, ArrayList* manifest) {