Negotation env lists can specify "client & server"

4 files changed, 69 insertions, 37 deletions

diff --git a/NEWS.md b/NEWS.md
index 2d81eb66..d579b955 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -17,7 +17,8 @@ Protocol: 31 (unchanged)
 ### ENHANCEMENTS:
 
  - Allow the server side to restrict checksum & compression choices via
-   the same environment variables the client uses.
+   the same environment variables the client uses.  Allow the env vars
+   to be divided into "client list & server list" by the "`&`" char.
 
  - Simplify how the negotiation environment variables apply to older rsync
    versions.
diff --git a/batch.c b/batch.c
index 805acc6f..582c8c16 100644
--- a/batch.c
+++ b/batch.c
@@ -269,14 +269,6 @@ void write_batch_shell_file(void)
 			err |= write_opt("--exclude-from", "-");
 	}
 
-	/* We need to make sure that any protocol-based or negotiated choices get accurately
-	 * reflected in the options we save AND that we avoid any need for --read-batch to
-	 * do a string-based negotiation (since we don't write them into the file). */
-	if (do_compression)
-		err |= write_opt("--compress-choice", compress_choice);
-	if (strchr(checksum_choice, ',') || xfersum_type != parse_csum_name(NULL, -1))
-		err |= write_opt("--checksum-choice", checksum_choice);
-
 	/* Elide the filename args from the option list, but scan for them in reverse. */
 	for (i = raw_argc-1, j = cooked_argc-1; i > 0 && j >= 0; i--) {
 		if (strcmp(raw_argv[i], cooked_argv[j]) == 0) {
diff --git a/compat.c b/compat.c
index ba14a8c5..bf0b4e6d 100644
--- a/compat.c
+++ b/compat.c
@@ -20,6 +20,7 @@
  */
 
 #include "rsync.h"
+#include "itypes.h"
 
 extern int am_server;
 extern int am_sender;
@@ -109,6 +110,9 @@ struct name_num_obj valid_compressions = {
 #define CF_INPLACE_PARTIAL_DIR (1<<6)
 #define CF_VARINT_FLIST_FLAGS (1<<7)
 
+#define ENV_CHECKSUM 0
+#define ENV_COMPRESS 1
+
 static const char *client_info;
 
 /* The server makes sure that if either side only supports a pre-release
@@ -262,10 +266,14 @@ static void init_nno_saw(struct name_num_obj *nno, int val)
 static int parse_nni_str(struct name_num_obj *nno, const char *from, char *tobuf, int tobuf_len)
 {
 	char *to = tobuf, *tok = NULL;
-	int cnt = 0;
+	int saw_tok = 0, cnt = 0;
 
 	while (1) {
-		if (*from == ' ' || !*from) {
+		int at_space = isSpace(from);
+		char ch = *from++;
+		if (ch == '&')
+			ch = '\0';
+		if (!ch || at_space) {
 			if (tok) {
 				struct name_num_item *nni = get_nni_by_name(nno, tok, to - tok);
 				if (nni && !nno->saw[nni->num]) {
@@ -279,9 +287,10 @@ static int parse_nni_str(struct name_num_obj *nno, const char *from, char *tobuf
 					}
 				} else
 					to = tok - (tok != tobuf);
+				saw_tok = 1;
 				tok = NULL;
 			}
-			if (!*from++)
+			if (!ch)
 				break;
 			continue;
 		}
@@ -294,10 +303,13 @@ static int parse_nni_str(struct name_num_obj *nno, const char *from, char *tobuf
 			to = tok - (tok != tobuf);
 			break;
 		}
-		*to++ = *from++;
+		*to++ = ch;
 	}
 	*to = '\0';
 
+	if (saw_tok && to == tobuf)
+		return strlcpy(tobuf, "INVALID", MAX_NSTR_STRLEN);
+
 	return to - tobuf;
 }
 
@@ -349,14 +361,36 @@ static void recv_negotiate_str(int f_in, struct name_num_obj *nno, char *tmpbuf,
 	exit_cleanup(RERR_UNSUPPORTED);
 }
 
+static const char *getenv_nstr(int etype)
+{
+	const char *env_str = getenv(etype == ENV_COMPRESS ? "RSYNC_COMPRESS_LIST" : "RSYNC_CHECKSUM_LIST");
+
+	/* When writing a batch file, we always negotiate an old-style choice. */
+	if (write_batch) 
+		env_str = etype == ENV_COMPRESS ? "zlib" : protocol_version >= 30 ? "md5" : "md4";
+
+	if (am_server && env_str) {
+		char *cp = strchr(env_str, '&');
+		if (cp)
+			env_str = cp + 1;
+	}
+
+	return env_str;
+}
+
 /* If num2 < 0 then the caller is checking compress values, otherwise checksum values. */
 void validate_choice_vs_env(int num1, int num2)
 {
 	struct name_num_obj *nno = num2 < 0 ? &valid_compressions : &valid_checksums;
-	const char *list_str = getenv(num2 < 0 ? "RSYNC_COMPRESS_LIST" : "RSYNC_CHECKSUM_LIST");
+	const char *list_str = getenv_nstr(num2 < 0 ? ENV_COMPRESS : ENV_CHECKSUM);
 	char tmpbuf[MAX_NSTR_STRLEN];
 
-	if (!list_str || !*list_str)
+	if (!list_str)
+		return;
+
+	while (isSpace(list_str)) list_str++;
+
+	if (!*list_str)
 		return;
 
 	init_nno_saw(nno, 0);
@@ -422,17 +456,15 @@ int get_default_nno_list(struct name_num_obj *nno, char *to_buf, int to_buf_len,
 	return len;
 }
 
-static void send_negotiate_str(int f_out, struct name_num_obj *nno, const char *env_name)
+static void send_negotiate_str(int f_out, struct name_num_obj *nno, int etype)
 {
 	char tmpbuf[MAX_NSTR_STRLEN];
-	const char *list_str = getenv(env_name);
+	const char *list_str = getenv_nstr(etype);
 	int len;
 
 	if (list_str && *list_str) {
 		init_nno_saw(nno, 0);
 		len = parse_nni_str(nno, list_str, tmpbuf, MAX_NSTR_STRLEN);
-		if (!len)
-			len = strlcpy(tmpbuf, "FAIL", MAX_NSTR_STRLEN);
 		list_str = tmpbuf;
 	} else
 		list_str = NULL;
@@ -447,15 +479,10 @@ static void send_negotiate_str(int f_out, struct name_num_obj *nno, const char *
 			rprintf(FINFO, "Client %s list (on client): %s\n", nno->type, tmpbuf);
 	}
 
-	if (local_server) {
-		/* A local server doesn't bother to send/recv the strings, it just constructs
-		 * and parses the same string on both sides. */
-		recv_negotiate_str(-1, nno, tmpbuf, len);
-	} else if (do_negotiated_strings) {
-		/* Each side sends their list of valid names to the other side and then both sides
-		 * pick the first name in the client's list that is also in the server's list. */
+	/* Each side sends their list of valid names to the other side and then both sides
+	 * pick the first name in the client's list that is also in the server's list. */
+	if (do_negotiated_strings)
 		write_vstring(f_out, tmpbuf, len);
-	}
 }
 
 static void negotiate_the_strings(int f_in, int f_out)
@@ -463,10 +490,10 @@ static void negotiate_the_strings(int f_in, int f_out)
 	/* We send all the negotiation strings before we start to read them to help avoid a slow startup. */
 
 	if (!checksum_choice)
-		send_negotiate_str(f_out, &valid_checksums, "RSYNC_CHECKSUM_LIST");
+		send_negotiate_str(f_out, &valid_checksums, ENV_CHECKSUM);
 
 	if (do_compression && !compress_choice)
-		send_negotiate_str(f_out, &valid_compressions, "RSYNC_COMPRESS_LIST");
+		send_negotiate_str(f_out, &valid_compressions, ENV_COMPRESS);
 
 	if (valid_checksums.saw) {
 		char tmpbuf[MAX_NSTR_STRLEN];
@@ -645,10 +672,8 @@ void setup_protocol(int f_out,int f_in)
 			if (local_server || strchr(client_info, 'I') != NULL)
 				compat_flags |= CF_INPLACE_PARTIAL_DIR;
 			if (local_server || strchr(client_info, 'v') != NULL) {
-				if (!write_batch || protocol_version >= 30) {
-					do_negotiated_strings = 1;
-					compat_flags |= CF_VARINT_FLIST_FLAGS;
-				}
+				do_negotiated_strings = 1;
+				compat_flags |= CF_VARINT_FLIST_FLAGS;
 			}
 			if (strchr(client_info, 'V') != NULL) { /* Support a pre-release 'V' that got superseded */
 				if (!write_batch)
@@ -697,6 +722,9 @@ void setup_protocol(int f_out,int f_in)
 #endif
 	}
 
+	if (read_batch)
+		do_negotiated_strings = 0;
+
 	if (need_unsorted_flist && (!am_sender || inc_recurse))
 		unsort_ndx = ++file_extra_cnt;
 
diff --git a/rsync.1.md b/rsync.1.md
index f58cedff..3708e1e1 100644
--- a/rsync.1.md
+++ b/rsync.1.md
@@ -1506,7 +1506,10 @@ your home directory (remove the '=' for that).
     transfer checksum separately from the pre-transfer checksum, and it ignores
     "auto" and all unknown checksum names.  If the remote rsync is not new
     enough to handle a checksum negotiation list, its list is assumed to
-    consist of a single "md5" or "md4" item based on the protocol version.
+    consist of a single "md5" or "md4" item based on the protocol version.  If
+    the environment variable contains a "`&`" character, the string is
+    separated into the client list & server list, either one of which can be
+    empty (giving that side the default list).
 
     The use of the `--checksum-choice` option overrides this environment list.
 
@@ -2295,11 +2298,14 @@ your home directory (remove the '=' for that).
 
     When both sides of the transfer are at least 3.2.0, rsync chooses the first
     algorithm in the client's list of choices that is also in the server's list
-    of choices.  Your default order can be customized by setting the environment
+    of choices.  The default order can be customized by setting the environment
     variable RSYNC_COMPRESS_LIST to a space-separated list of acceptable
     compression names.  If no common compress choice is found, the client exits
-    with an error.  If the remote rsync is too old to support checksum negotiation,
-    its list is assumed to be "zlib".
+    with an error.  If the remote rsync is too old to support checksum
+    negotiation, its list is assumed to be "zlib".  If the environment variable
+    contains a "`&`" character, the string is separated into the client list &
+    server list, either one of which can be empty (giving that side the default
+    list).
 
     There are some older rsync versions that were configured to reject a `-z`
     option and require the use of `-zz` because their compression library was
@@ -3104,6 +3110,11 @@ your home directory (remove the '=' for that).
     with `--read-batch`.  See the "BATCH MODE" section for details, and also
     the `--only-write-batch` option.
 
+    This option overrides the negotiated checksum & compress lists and always
+    negotiates a choice based on old-school md5/md4/zlib choices.  If you want
+    a more modern choice, use the `--checksum-choice` (`--cc`) and/or
+    `--compress-choice` (`--zc`) options.
+
 0.  `--only-write-batch=FILE`
 
     Works like `--write-batch`, except that no updates are made on the