From 6cdffd06d6ce1e997963790f9206fc981715fbbb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 19 Feb 2018 14:48:44 -0500 Subject: [PATCH 1/2] t5545: factor out http repository setup We repeat many lines of setup code in the two http tests, and further tests would need to repeat it again. Let's factor this out into a function. Incidentally, this also fixes an unlikely bug: if the httpd root path contains a double-quote, our test_when_finished would barf due to improper quoting (we escape the embedded quotes, but not the $, meaning we expand the variable before the eval). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5545-push-options.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh index 463783789c..c64dee2127 100755 --- a/t/t5545-push-options.sh +++ b/t/t5545-push-options.sh @@ -220,14 +220,20 @@ test_expect_success 'invalid push option in config' ' . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -test_expect_success 'push option denied properly by http server' ' +# set up http repository for fetching/pushing, with push options config +# bool set to $1 +mk_http_pair () { test_when_finished "rm -rf test_http_clone" && - test_when_finished "rm -rf \"$HTTPD_DOCUMENT_ROOT_PATH\"/upstream.git" && + test_when_finished 'rm -rf "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git' && mk_repo_pair && - git -C upstream config receive.advertisePushOptions false && + git -C upstream config receive.advertisePushOptions "$1" && git -C upstream config http.receivepack true && cp -R upstream/.git "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git && - git clone "$HTTPD_URL"/smart/upstream test_http_clone && + git clone "$HTTPD_URL"/smart/upstream test_http_clone +} + +test_expect_success 'push option denied properly by http server' ' + mk_http_pair false && test_commit -C test_http_clone one && test_must_fail git -C test_http_clone push --push-option=asdf origin master 2>actual && test_i18ngrep "the receiving end does not support push options" actual && @@ -235,13 +241,7 @@ test_expect_success 'push option denied properly by http server' ' ' test_expect_success 'push options work properly across http' ' - test_when_finished "rm -rf test_http_clone" && - test_when_finished "rm -rf \"$HTTPD_DOCUMENT_ROOT_PATH\"/upstream.git" && - mk_repo_pair && - git -C upstream config receive.advertisePushOptions true && - git -C upstream config http.receivepack true && - cp -R upstream/.git "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git && - git clone "$HTTPD_URL"/smart/upstream test_http_clone && + mk_http_pair true && test_commit -C test_http_clone one && git -C test_http_clone push origin master && From 90dce21eb0fcf28096e661a3dd3b4e93fa0bccb5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 19 Feb 2018 14:50:14 -0500 Subject: [PATCH 2/2] remote-curl: unquote incoming push-options The transport-helper protocol c-style quotes the value of any options passed to the helper via the "option " 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 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- 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 0053b09549..c8a42614e5 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 */ @@ -142,7 +143,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