Passing SFTP options when using SCP

[Damien Miller]

On Wed, 7 Dec 2022, Darren Tucker wrote:

> On Wed, 7 Dec 2022 at 14:23, Damien Miller <djm at mindrot.org> wrote:
> [...]
> > There are no options to do this ATM and at least one of the option
> > letters that sftp uses already has meaning for sftp. These are
> > rarely used, so maybe it makes sent to put them behind a single
> > getopt chat that accepts multiple arguments.
> 
> Similar to ssh-keygen's -M, I like it.   Since you picked a letter not
> in use by sftp either, you could also put it in sftp and provide a
> consistent interface in both.

Here's a fleshed out version of the previous diff, including the same
option for sftp and manpage bits.

diff --git a/scp.1 b/scp.1
index cd23f97..a98df59 100644
--- a/scp.1
+++ b/scp.1
@@ -28,6 +28,7 @@
 .Op Fl o Ar ssh_option
 .Op Fl P Ar port
 .Op Fl S Ar program
+.Op Fl X Ar sftp_option
 .Ar source ... target
 .Sh DESCRIPTION
 .Nm
@@ -278,6 +279,19 @@ and
 to print debugging messages about their progress.
 This is helpful in
 debugging connection, authentication, and configuration problems.
+.It Fl X Ar sftp_option
+Specify an option that controls aspects of SFTP protocol behaviour.
+The valid options are:
+.Bl -tag -width Ds
+.It Cm nrequests Ns = Ns Ar value
+Controls how many concurrent SFTP read or write requests may be in progress
+at any point in time during a download or upload.
+By default 64 requests may be active concurrently.
+.It Cm buffer Ns = Ns Ar value
+Controls the maximum buffer size for a single SFTP read/write operation used
+during download or upload.
+By default a 32KB buffer is used.
+.El
 .El
 .Sh EXIT STATUS
 .Ex -std scp
diff --git a/scp.c b/scp.c
index c7194c2..d401718 100644
--- a/scp.c
+++ b/scp.c
@@ -150,6 +150,10 @@ char *ssh_program = _PATH_SSH_PROGRAM;
 pid_t do_cmd_pid = -1;
 pid_t do_cmd_pid2 = -1;
 
+/* SFTP copy parameters */
+size_t sftp_copy_buflen;
+size_t sftp_nrequests;
+
 /* Needed for sftp */
 volatile sig_atomic_t interrupted = 0;
 
@@ -419,7 +423,7 @@ int
 main(int argc, char **argv)
 {
 	int ch, fflag, tflag, status, n;
-	char **newargv, *argv0;
+	char **newargv, *argv0, *cp;
 	const char *errstr;
 	extern char *optarg;
 	extern int optind;
@@ -452,7 +456,7 @@ main(int argc, char **argv)
 
 	fflag = Tflag = tflag = 0;
 	while ((ch = getopt(argc, argv,
-	    "12346ABCTdfOpqRrstvD:F:J:M:P:S:c:i:l:o:")) != -1) {
+	    "12346ABCTdfOpqRrstvD:F:J:M:P:S:c:i:l:o:X:")) != -1) {
 		switch (ch) {
 		/* User-visible flags. */
 		case '1':
@@ -533,6 +537,24 @@ main(int argc, char **argv)
 			addargs(&remote_remote_args, "-q");
 			showprogress = 0;
 			break;
+		case 'X':
+			/* Please keep in sync with sftp.c -X */
+			if (strncmp(optarg, "buffer=", 7) == 0) {
+				sftp_copy_buflen = strtol(optarg + 7, &cp, 10);
+				if (sftp_copy_buflen == 0 || *cp != '\0') {
+					fatal("Invalid buffer size \"%s\"",
+					     optarg + 7);
+				}
+			} else if (strncmp(optarg, "nrequests=", 10) == 0) {
+				sftp_nrequests = strtol(optarg + 10, &cp, 10);
+				if (sftp_nrequests == 0 || *cp != '\0') {
+					fatal("Invalid number of requests "
+					    "\"%s\"", optarg + 10);
+				}
+			} else {
+				fatal("Invalid -X option");
+			}
+			break;
 
 		/* Server options. */
 		case 'd':
@@ -941,7 +963,8 @@ do_sftp_connect(char *host, char *user, int port, char *sftp_direct,
 		    reminp, remoutp, pidp) < 0)
 			return NULL;
 	}
-	return do_init(*reminp, *remoutp, 32768, 64, limit_kbps);
+	return do_init(*reminp, *remoutp,
+	    sftp_copy_buflen, sftp_nrequests, limit_kbps);
 }
 
 void
diff --git a/sftp-client.c b/sftp-client.c
index a3e7a5d..1907753 100644
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -55,10 +55,10 @@
 extern volatile sig_atomic_t interrupted;
 extern int showprogress;
 
-/* Default size of buffer for up/download */
+/* Default size of buffer for up/download (fix sftp.1 scp.1 if changed) */
 #define DEFAULT_COPY_BUFLEN	32768
 
-/* Default number of concurrent outstanding requests */
+/* Default number of concurrent xfer requests (fix sftp.1 scp.1 if changed) */
 #define DEFAULT_NUM_REQUESTS	64
 
 /* Minimum amount of data to read at a time */
diff --git a/sftp.1 b/sftp.1
index 3b3f2c5..3b430c5 100644
--- a/sftp.1
+++ b/sftp.1
@@ -44,6 +44,7 @@
 .Op Fl R Ar num_requests
 .Op Fl S Ar program
 .Op Fl s Ar subsystem | sftp_server
+.Op Fl X Ar sftp_option
 .Ar destination
 .Sh DESCRIPTION
 .Nm
@@ -320,6 +321,19 @@ does not have an sftp subsystem configured.
 .It Fl v
 Raise logging level.
 This option is also passed to ssh.
+.It Fl X Ar sftp_option
+Specify an option that controls aspects of SFTP protocol behaviour.
+The valid options are:
+.Bl -tag -width Ds
+.It Cm nrequests Ns = Ns Ar value
+Controls how many concurrent SFTP read or write requests may be in progress
+at any point in time during a download or upload.
+By default 64 requests may be active concurrently.
+.It Cm buffer Ns = Ns Ar value
+Controls the maximum buffer size for a single SFTP read/write operation used
+during download or upload.
+By default a 32KB buffer is used.
+.El
 .El
 .Sh INTERACTIVE COMMANDS
 Once in interactive mode,
diff --git a/sftp.c b/sftp.c
index 02547f6..01999e1 100644
--- a/sftp.c
+++ b/sftp.c
@@ -2497,6 +2497,24 @@ main(int argc, char **argv)
 			ssh_program = optarg;
 			replacearg(&args, 0, "%s", ssh_program);
 			break;
+		case 'X':
+			/* Please keep in sync with ssh.c -X */
+			if (strncmp(optarg, "buffer=", 7) == 0) {
+				copy_buffer_len = strtol(optarg + 7, &cp, 10);
+				if (copy_buffer_len == 0 || *cp != '\0') {
+					fatal("Invalid buffer size \"%s\"",
+					     optarg + 7);
+				}
+			} else if (strncmp(optarg, "nrequests=", 10) == 0) {
+				num_requests = strtol(optarg + 10, &cp, 10);
+				if (num_requests == 0 || *cp != '\0') {
+					fatal("Invalid number of requests "
+					    "\"%s\"", optarg + 10);
+				}
+			} else {
+				fatal("Invalid -X option");
+			}
+			break;
 		case 'h':
 		default:
 			usage();