Attachment 3594 Details for Bug 3431 – don't overwrite files

[patch] don't overwrite files

sftp-overwrite.diff (text/plain), 10.70 KB, created by Damien Miller on 2022-05-13 13:10:49 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2022-05-13 13:10:49 AEST

Size: 10.70 KB

patch

obsolete

>commit 0121e6dce88b355047588bf3696ed5a6807b4c5f
>Author: Damien Miller <djm@mindrot.org>
>Date:   Thu Apr 28 12:50:14 2022 +1000
>
>    scp foo localhost:foo / scp localhost:foo foo
>
>diff --git a/scp.c b/scp.c
>index fc5d99e..72da1ed 100644
>--- a/scp.c
>+++ b/scp.c
>@@ -1278,11 +1278,11 @@ source_sftp(int argc, char *src, char *targ, struct sftp_conn *conn)
> 
> 	if (src_is_dir && iamrecursive) {
> 		if (upload_dir(conn, src, abs_dst, pflag,
>-		    SFTP_PROGRESS_ONLY, 0, 0, 1) != 0) {
>+		    SFTP_PROGRESS_ONLY, 0, 0, 1, 1) != 0) {
> 			error("failed to upload directory %s to %s", src, targ);
> 			errs = 1;
> 		}
>-	} else if (do_upload(conn, src, abs_dst, pflag, 0, 0) != 0) {
>+	} else if (do_upload(conn, src, abs_dst, pflag, 0, 0, 1) != 0) {
> 		error("failed to upload file %s to %s", src, targ);
> 		errs = 1;
> 	}
>@@ -1519,11 +1519,11 @@ sink_sftp(int argc, char *dst, const char *src, struct sftp_conn *conn)
> 		debug("Fetching %s to %s\n", g.gl_pathv[i], abs_dst);
> 		if (globpath_is_dir(g.gl_pathv[i]) && iamrecursive) {
> 			if (download_dir(conn, g.gl_pathv[i], abs_dst, NULL,
>-			    pflag, SFTP_PROGRESS_ONLY, 0, 0, 1) == -1)
>+			    pflag, SFTP_PROGRESS_ONLY, 0, 0, 1, 1) == -1)
> 				err = -1;
> 		} else {
> 			if (do_download(conn, g.gl_pathv[i], abs_dst, NULL,
>-			    pflag, 0, 0) == -1)
>+			    pflag, 0, 0, 1) == -1)
> 				err = -1;
> 		}
> 		free(abs_dst);
>diff --git a/sftp-client.c b/sftp-client.c
>index 403a2a8..3349c0a 100644
>--- a/sftp-client.c
>+++ b/sftp-client.c
>@@ -1560,7 +1560,7 @@ progress_meter_path(const char *path)
> int
> do_download(struct sftp_conn *conn, const char *remote_path,
>     const char *local_path, Attrib *a, int preserve_flag, int resume_flag,
>-    int fsync_flag)
>+    int fsync_flag, int inplace_flag)
> {
> 	struct sshbuf *msg;
> 	u_char *handle;
>@@ -1607,8 +1607,8 @@ do_download(struct sftp_conn *conn, const char *remote_path,
> 	    &handle, &handle_len) != 0)
> 		return -1;
> 
>-	local_fd = open(local_path,
>-	    O_WRONLY | O_CREAT | (resume_flag ? 0 : O_TRUNC), mode | S_IWUSR);
>+	local_fd = open(local_path, O_WRONLY | O_CREAT |
>+	((resume_flag || inplace_flag) ? 0 : O_TRUNC), mode | S_IWUSR);
> 	if (local_fd == -1) {
> 		error("open local \"%s\": %s", local_path, strerror(errno));
> 		goto fail;
>@@ -1827,7 +1827,7 @@ do_download(struct sftp_conn *conn, const char *remote_path,
> static int
> download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
>     int depth, Attrib *dirattrib, int preserve_flag, int print_flag,
>-    int resume_flag, int fsync_flag, int follow_link_flag)
>+    int resume_flag, int fsync_flag, int follow_link_flag, int inplace_flag)
> {
> 	int i, ret = 0;
> 	SFTP_DIRENT **dir_entries;
>@@ -1886,7 +1886,7 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
> 			if (download_dir_internal(conn, new_src, new_dst,
> 			    depth + 1, &(dir_entries[i]->a), preserve_flag,
> 			    print_flag, resume_flag,
>-			    fsync_flag, follow_link_flag) == -1)
>+			    fsync_flag, follow_link_flag, inplace_flag) == -1)
> 				ret = -1;
> 		} else if (S_ISREG(dir_entries[i]->a.perm) ||
> 		    (follow_link_flag && S_ISLNK(dir_entries[i]->a.perm))) {
>@@ -1898,7 +1898,8 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
> 			if (do_download(conn, new_src, new_dst,
> 			    S_ISLNK(dir_entries[i]->a.perm) ? NULL :
> 			    &(dir_entries[i]->a),
>-			    preserve_flag, resume_flag, fsync_flag) == -1) {
>+			    preserve_flag, resume_flag, fsync_flag,
>+			    inplace_flag) == -1) {
> 				error("Download of file %s to %s failed",
> 				    new_src, new_dst);
> 				ret = -1;
>@@ -1936,7 +1937,7 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
> int
> download_dir(struct sftp_conn *conn, const char *src, const char *dst,
>     Attrib *dirattrib, int preserve_flag, int print_flag, int resume_flag,
>-    int fsync_flag, int follow_link_flag)
>+    int fsync_flag, int follow_link_flag, int inplace_flag)
> {
> 	char *src_canon;
> 	int ret;
>@@ -1948,26 +1949,25 @@ download_dir(struct sftp_conn *conn, const char *src, const char *dst,
> 
> 	ret = download_dir_internal(conn, src_canon, dst, 0,
> 	    dirattrib, preserve_flag, print_flag, resume_flag, fsync_flag,
>-	    follow_link_flag);
>+	    follow_link_flag, inplace_flag);
> 	free(src_canon);
> 	return ret;
> }
> 
> int
> do_upload(struct sftp_conn *conn, const char *local_path,
>-    const char *remote_path, int preserve_flag, int resume, int fsync_flag)
>+    const char *remote_path, int preserve_flag, int resume,
>+    int fsync_flag, int inplace_flag)
> {
> 	int r, local_fd;
>-	u_int status = SSH2_FX_OK;
>-	u_int id;
>-	u_char type;
>+	u_int openmode, id, status = SSH2_FX_OK, reordered = 0;
> 	off_t offset, progress_counter;
>-	u_char *handle, *data;
>+	u_char type, *handle, *data;
> 	struct sshbuf *msg;
> 	struct stat sb;
>-	Attrib a, *c = NULL;
>-	u_int32_t startid;
>-	u_int32_t ackid;
>+	Attrib a, t, *c = NULL;
>+	u_int32_t startid, ackid;
>+	u_int64_t highwater = 0;
> 	struct request *ack = NULL;
> 	struct requests acks;
> 	size_t handle_len;
>@@ -2019,10 +2019,15 @@ do_upload(struct sftp_conn *conn, const char *local_path,
> 		}
> 	}
> 
>+	openmode = SSH2_FXF_WRITE|SSH2_FXF_CREAT;
>+	if (resume)
>+		openmode |= SSH2_FXF_APPEND;
>+	else if (!inplace_flag)
>+		openmode |= SSH2_FXF_TRUNC;
>+
> 	/* Send open request */
>-	if (send_open(conn, remote_path, "dest", SSH2_FXF_WRITE|SSH2_FXF_CREAT|
>-	    (resume ? SSH2_FXF_APPEND : SSH2_FXF_TRUNC),
>-	    &a, &handle, &handle_len) != 0) {
>+	if (send_open(conn, remote_path, "dest", openmode, &a,
>+	    &handle, &handle_len) != 0) {
> 		close(local_fd);
> 		return -1;
> 	}
>@@ -2103,6 +2108,12 @@ do_upload(struct sftp_conn *conn, const char *local_path,
> 			    ack->id, ack->len, (unsigned long long)ack->offset);
> 			++ackid;
> 			progress_counter += ack->len;
>+			if (!reordered && ack->offset <= highwater)
>+				highwater = ack->offset + ack->len;
>+			else if (!reordered && ack->offset > highwater) {
>+				debug3_f("server reordered ACKs");
>+				reordered = 1;
>+			}
> 			free(ack);
> 		}
> 		offset += len;
>@@ -2120,6 +2131,14 @@ do_upload(struct sftp_conn *conn, const char *local_path,
> 		status = SSH2_FX_FAILURE;
> 	}
> 
>+	if ((resume || inplace_flag) && (status != SSH2_FX_OK || interrupted)) {
>+		debug("truncating at %llu", (unsigned long long)highwater);
>+		attrib_clear(&t);
>+		t.flags = SSH2_FILEXFER_ATTR_SIZE;
>+		t.size = highwater;
>+		do_fsetstat(conn, handle, handle_len, &a);
>+	}
>+
> 	if (close(local_fd) == -1) {
> 		error("close local \"%s\": %s", local_path, strerror(errno));
> 		status = SSH2_FX_FAILURE;
>@@ -2143,7 +2162,7 @@ do_upload(struct sftp_conn *conn, const char *local_path,
> static int
> upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
>     int depth, int preserve_flag, int print_flag, int resume, int fsync_flag,
>-    int follow_link_flag)
>+    int follow_link_flag, int inplace_flag)
> {
> 	int ret = 0;
> 	DIR *dirp;
>@@ -2221,12 +2240,13 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
> 
> 			if (upload_dir_internal(conn, new_src, new_dst,
> 			    depth + 1, preserve_flag, print_flag, resume,
>-			    fsync_flag, follow_link_flag) == -1)
>+			    fsync_flag, follow_link_flag, inplace_flag) == -1)
> 				ret = -1;
> 		} else if (S_ISREG(sb.st_mode) ||
> 		    (follow_link_flag && S_ISLNK(sb.st_mode))) {
> 			if (do_upload(conn, new_src, new_dst,
>-			    preserve_flag, resume, fsync_flag) == -1) {
>+			    preserve_flag, resume, fsync_flag,
>+			    inplace_flag) == -1) {
> 				error("upload \"%s\" to \"%s\" failed",
> 				    new_src, new_dst);
> 				ret = -1;
>@@ -2246,7 +2266,7 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
> int
> upload_dir(struct sftp_conn *conn, const char *src, const char *dst,
>     int preserve_flag, int print_flag, int resume, int fsync_flag,
>-    int follow_link_flag)
>+    int follow_link_flag, int inplace_flag)
> {
> 	char *dst_canon;
> 	int ret;
>@@ -2257,7 +2277,7 @@ upload_dir(struct sftp_conn *conn, const char *src, const char *dst,
> 	}
> 
> 	ret = upload_dir_internal(conn, src, dst_canon, 0, preserve_flag,
>-	    print_flag, resume, fsync_flag, follow_link_flag);
>+	    print_flag, resume, fsync_flag, follow_link_flag, inplace_flag);
> 
> 	free(dst_canon);
> 	return ret;
>diff --git a/sftp-client.h b/sftp-client.h
>index 6c62821..a235145 100644
>--- a/sftp-client.h
>+++ b/sftp-client.h
>@@ -135,28 +135,29 @@ int do_fsync(struct sftp_conn *conn, u_char *, u_int);
>  * Download 'remote_path' to 'local_path'. Preserve permissions and times
>  * if 'pflag' is set
>  */
>-int do_download(struct sftp_conn *, const char *, const char *,
>-    Attrib *, int, int, int);
>+int do_download(struct sftp_conn *, const char *, const char *, Attrib *,
>+    int, int, int, int);
> 
> /*
>  * Recursively download 'remote_directory' to 'local_directory'. Preserve
>  * times if 'pflag' is set
>  */
>-int download_dir(struct sftp_conn *, const char *, const char *,
>-    Attrib *, int, int, int, int, int);
>+int download_dir(struct sftp_conn *, const char *, const char *, Attrib *,
>+    int, int, int, int, int, int);
> 
> /*
>  * Upload 'local_path' to 'remote_path'. Preserve permissions and times
>  * if 'pflag' is set
>  */
>-int do_upload(struct sftp_conn *, const char *, const char *, int, int, int);
>+int do_upload(struct sftp_conn *, const char *, const char *,
>+    int, int, int, int);
> 
> /*
>  * Recursively upload 'local_directory' to 'remote_directory'. Preserve
>  * times if 'pflag' is set
>  */
>-int upload_dir(struct sftp_conn *, const char *, const char *, int, int, int,
>-    int, int);
>+int upload_dir(struct sftp_conn *, const char *, const char *,
>+    int, int, int, int, int, int);
> 
> /*
>  * Download a 'from_path' from the 'from' connection and upload it to
>diff --git a/sftp.c b/sftp.c
>index e47adf5..c34df62 100644
>--- a/sftp.c
>+++ b/sftp.c
>@@ -676,12 +676,12 @@ process_get(struct sftp_conn *conn, const char *src, const char *dst,
> 		if (globpath_is_dir(g.gl_pathv[i]) && (rflag || global_rflag)) {
> 			if (download_dir(conn, g.gl_pathv[i], abs_dst, NULL,
> 			    pflag || global_pflag, 1, resume,
>-			    fflag || global_fflag, 0) == -1)
>+			    fflag || global_fflag, 0, 0) == -1)
> 				err = -1;
> 		} else {
> 			if (do_download(conn, g.gl_pathv[i], abs_dst, NULL,
> 			    pflag || global_pflag, resume,
>-			    fflag || global_fflag) == -1)
>+			    fflag || global_fflag, 0) == -1)
> 				err = -1;
> 		}
> 		free(abs_dst);
>@@ -770,12 +770,12 @@ process_put(struct sftp_conn *conn, const char *src, const char *dst,
> 		if (globpath_is_dir(g.gl_pathv[i]) && (rflag || global_rflag)) {
> 			if (upload_dir(conn, g.gl_pathv[i], abs_dst,
> 			    pflag || global_pflag, 1, resume,
>-			    fflag || global_fflag, 0) == -1)
>+			    fflag || global_fflag, 0, 0) == -1)
> 				err = -1;
> 		} else {
> 			if (do_upload(conn, g.gl_pathv[i], abs_dst,
> 			    pflag || global_pflag, resume,
>-			    fflag || global_fflag) == -1)
>+			    fflag || global_fflag, 0) == -1)
> 				err = -1;
> 		}
> 	}

Flags:

dtucker: ok+

Actions: View | Diff

Attachments on bug 3431: 3594