Bug#4201 - HiddenStores temporary files not removed when exceeding qu…

…ota using

SCP.

Make sure to clean up the HiddenStores temporary file when there was an error,
either when writing OR when closing the HiddenStores file (as when quota
is exceeded).

contrib/mod_sftp/scp.c

@@ -1383,48 +1383,52 @@ static int recv_path(pool *p, uint32_t channel_id, struct scp_path *sp,
     }
   }
 
-  /* The uploaded file may be smaller than an existing file; call
-   * pr_fsio_truncate() to ensure proper file size.
-   */
-  if (S_ISREG(sp->st_mode)) {
-    pr_trace_msg(trace_channel, 9, "truncating file '%s' to %" PR_LU " bytes",
-      sp->fh->fh_path, (pr_off_t) sp->filesz);
+  if (sp->wrote_errors == FALSE) {
+    /* The uploaded file may be smaller than an existing file; call
+     * pr_fsio_truncate() to ensure proper file size.
+     */
+    if (S_ISREG(sp->st_mode)) {
+      pr_trace_msg(trace_channel, 9, "truncating file '%s' to %" PR_LU " bytes",
+        sp->fh->fh_path, (pr_off_t) sp->filesz);
 
-    if (pr_fsio_ftruncate(sp->fh, sp->filesz) < 0) {
-      int xerrno = errno;
+      if (pr_fsio_ftruncate(sp->fh, sp->filesz) < 0) {
+        int xerrno = errno;
 
-      pr_trace_msg(trace_channel, 2, "error truncating '%s' to %" PR_LU
-        " bytes: %s", sp->best_path, (pr_off_t) sp->filesz, strerror(xerrno));
-      write_confirm(p, channel_id, 1,
-        pstrcat(p, sp->filename, ": error truncating file: ", strerror(xerrno),
-        NULL));
+        pr_trace_msg(trace_channel, 2, "error truncating '%s' to %" PR_LU
+          " bytes: %s", sp->best_path, (pr_off_t) sp->filesz, strerror(xerrno));
+        write_confirm(p, channel_id, 1,
+          pstrcat(p, sp->filename, ": error truncating file: ",
+          strerror(xerrno), NULL));
 
-      sp->wrote_errors = TRUE;
+        sp->wrote_errors = TRUE;
+      }
     }
   }
 
-  /* If the SFTPOption for ignoring perms for SCP uploads is set, then
-   * skip the chmod on the upload file.
-   */
-  if (!(sftp_opts & SFTP_OPT_IGNORE_SCP_UPLOAD_PERMS)) { 
-    pr_trace_msg(trace_channel, 9, "setting perms %04o on file '%s'",
-      (unsigned int) sp->perms, sp->fh->fh_path);
+  if (sp->wrote_errors == FALSE) {
+    /* If the SFTPOption for ignoring perms for SCP uploads is set, then
+     * skip the chmod on the upload file.
+     */
+    if (!(sftp_opts & SFTP_OPT_IGNORE_SCP_UPLOAD_PERMS)) { 
+      pr_trace_msg(trace_channel, 9, "setting perms %04o on file '%s'",
+        (unsigned int) sp->perms, sp->fh->fh_path);
 
-    if (pr_fsio_fchmod(sp->fh, sp->perms) < 0) {
-      int xerrno = errno;
+      if (pr_fsio_fchmod(sp->fh, sp->perms) < 0) {
+        int xerrno = errno;
 
-      pr_trace_msg(trace_channel, 2, "error setting mode %04o on '%s': %s",
-        (unsigned int) sp->perms, sp->best_path, strerror(xerrno));
-      write_confirm(p, channel_id, 1,
-        pstrcat(p, sp->filename, ": error setting mode: ", strerror(xerrno),
-        NULL));
+        pr_trace_msg(trace_channel, 2, "error setting mode %04o on '%s': %s",
+          (unsigned int) sp->perms, sp->best_path, strerror(xerrno));
+        write_confirm(p, channel_id, 1,
+          pstrcat(p, sp->filename, ": error setting mode: ", strerror(xerrno),
+          NULL));
 
-      sp->wrote_errors = TRUE;
-    }
+        sp->wrote_errors = TRUE;
+      }
 
-  } else {
-    pr_trace_msg(trace_channel, 7, "SFTPOption 'IgnoreSCPUploadPerms' "
-      "configured, ignoring perms sent by client");
+    } else {
+      pr_trace_msg(trace_channel, 7, "SFTPOption 'IgnoreSCPUploadPerms' "
+        "configured, ignoring perms sent by client");
+    }
   }
 
   if (sp->fh) {
@@ -1449,27 +1453,46 @@ static int recv_path(pool *p, uint32_t channel_id, struct scp_path *sp,
 
     sp->fh = NULL;
 
-    if (sp->hiddenstore == TRUE &&
-        res == 0) {
-      /* This is a HiddenStores file, and needs to be renamed to the real
-       * path (i.e. sp->best_path).
-       */
+    if (sp->hiddenstore == TRUE) {
+      if (sp->wrote_errors == TRUE) {
+        /* There was an error writing this HiddenStores file; be sure to clean
+         * things up.
+         */
+        pr_trace_msg(trace_channel, 8, "deleting HiddenStores path '%s'",
+          curr_path);
+
+        if (pr_fsio_unlink(curr_path) < 0) {
+          if (errno != ENOENT) {
+            pr_log_debug(DEBUG0, MOD_SFTP_VERSION
+              ": error deleting HiddenStores file '%s': %s", curr_path,
+              strerror(errno));
+          }
+        }
 
-      pr_trace_msg(trace_channel, 8, "renaming HiddenStores path '%s' to '%s'",
-        curr_path, sp->best_path);
+      } else {
+        /* This is a HiddenStores file, and needs to be renamed to the real
+         * path (i.e. sp->best_path).
+         */
+        pr_trace_msg(trace_channel, 8,
+          "renaming HiddenStores path '%s' to '%s'", curr_path, sp->best_path);
 
-      res = pr_fsio_rename(curr_path, sp->best_path);
-      if (res < 0) {
-        int xerrno = errno;
+        res = pr_fsio_rename(curr_path, sp->best_path);
+        if (res < 0) {
+          int xerrno = errno;
 
-        pr_log_pri(PR_LOG_WARNING, "Rename of %s to %s failed: %s",
-          curr_path, sp->best_path, strerror(xerrno));
+          pr_log_pri(PR_LOG_WARNING, "Rename of %s to %s failed: %s",
+            curr_path, sp->best_path, strerror(xerrno));
 
-        (void) pr_log_writefile(sftp_logfd, MOD_SFTP_VERSION,
-          "renaming of HiddenStore path '%s' to '%s' failed: %s",
-          curr_path, sp->best_path, strerror(xerrno));
+          (void) pr_log_writefile(sftp_logfd, MOD_SFTP_VERSION,
+            "renaming of HiddenStore path '%s' to '%s' failed: %s",
+            curr_path, sp->best_path, strerror(xerrno));
 
-        pr_fsio_unlink(curr_path);
+          if (pr_fsio_unlink(curr_path) < 0) {
+            pr_trace_msg(trace_channel, 1,
+              "error deleting HiddenStores file '%s': %s", curr_path,
+              strerror(errno));
+          }
+        }
       }
     }
   }