git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] remote-curl: unquote incoming push-options

From
Jeff King <peff@peff.net>
Date
Feb 19, 2018, 19:50 UTC
Message-ID
<20180219195014.GB988@sigill.intra.peff.net>
In-Reply-To
<20180219194709.GA837@sigill.intra.peff.net>

The transport-helper protocol c-style quotes the value of any options passed to the helper via the "option <key> <value>" directive. However, remote-curl doesn't actually unquote the push-option values, meaning that we will send the quoted version to the other side (whereas git-over-ssh would send the raw value).

The pack-protocol.txt documentation defines the push-options as a series of VCHARs, which excludes most characters that would need quoting. But:

  1. You can still see the bug with a valid push-option that
     starts with a double-quote (since that triggers
     quoting).
  2. We do currently handle any non-NUL characters correctly
     in git-over-ssh. So even though the spec does not say
     that we need to handle most quoted characters, it's
     nice if our behavior is consistent between protocols.

There are two new tests: the "direct" one shows that this already works in the non-http case, and the http one covers this bugfix.

Reported-by: Jon Simons <jon@jonsimons.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 remote-curl.c           | 11 ++++++++++-
 t/t5545-push-options.sh | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/remote-curl.c b/remote-curl.c
index 6ec5352435..f5b3d22e26 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -13,6 +13,7 @@
 #include "credential.h"
 #include "sha1-array.h"
 #include "send-pack.h"
+#include "quote.h"
 
 static struct remote *remote;
 /* always ends with a trailing slash */
@@ -145,7 +146,15 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	} else if (!strcmp(name, "push-option")) {
-		string_list_append(&options.push_options, value);
+		if (*value != '"')
+			string_list_append(&options.push_options, value);
+		else {
+			struct strbuf unquoted = STRBUF_INIT;
+			if (unquote_c_style(&unquoted, value, NULL) < 0)
+				die("invalid quoting in push-option value");
+			string_list_append_nodup(&options.push_options,
+						 strbuf_detach(&unquoted, NULL));
+		}
 		return 0;
 
 #if LIBCURL_VERSION_NUM >= 0x070a08
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index c64dee2127..b47a95871c 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -217,6 +217,15 @@ test_expect_success 'invalid push option in config' '
 	test_refs master HEAD@{1}
 '
 
+test_expect_success 'push options keep quoted characters intact (direct)' '
+	mk_repo_pair &&
+	git -C upstream config receive.advertisePushOptions true &&
+	test_commit -C workbench one &&
+	git -C workbench push --push-option="\"embedded quotes\"" up master &&
+	echo "\"embedded quotes\"" >expect &&
+	test_cmp expect upstream/.git/hooks/pre-receive.push_options
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
@@ -260,6 +269,15 @@ test_expect_success 'push options work properly across http' '
 	test_cmp expect actual
 '
 
+test_expect_success 'push options keep quoted characters intact (http)' '
+	mk_http_pair true &&
+
+	test_commit -C test_http_clone one &&
+	git -C test_http_clone push --push-option="\"embedded quotes\"" origin master &&
+	echo "\"embedded quotes\"" >expect &&
+	test_cmp expect "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git/hooks/pre-receive.push_options
+'
+
 stop_httpd
 
 test_done
-- 
2.16.2.552.gea2a3cf654
Previous: Jeff KingNext: Brandon Williams
Message 3 of 4 in “quoting bug sending push-options over http”
  1. 0/2 quoting bug sending push-options over httpJeff King, Feb 19, 2018
  2. 1/2 t5545: factor out http repository setupJeff King, Feb 19, 2018
  3. 2/2 remote-curl: unquote incoming push-optionsJeff King, Feb 19, 2018
  4. Brandon WilliamsFeb 20, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.