[PATCH v2] remote-curl: make --force-with-lease work with non-ASCII ref names
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jul 21, 2020, 01:15 UTC
- Message-ID
- <20200721011511.1649048-1-sandals@crustytoothpaste.net>
- In-Reply-To
- <CAP8P1T+JQUTVDXsJbrWkiZ2AWZmoptFSRdKssANYrNSFa+Egmg@mail.gmail.com>
When we invoke a remote transport helper and pass an option with an argument, we quote the argument as a C-style string if necessary. This is the case for the cas option, which implements the --force-with-lease command-line flag, when we're passing a non-ASCII refname.
However, the remote curl helper isn't designed to parse such an argument, meaning that if we try to use --force-with-lease with an HTTP push and a non-ASCII refname, we get an error like this:
error: cannot parse expected object name '0000000000000000000000000000000000000000"'
Note the double quote, which get_oid has reminded us is not valid in an hex object ID.
Even if we had been able to parse it, we would send the wrong data to the server: we'd send an escaped ref, which would not behave as the user wanted and might accidentally result in updating or deleting a ref we hadn't intended.
Since we need to expect a quoted C-style string here, just check if the first argument is a double quote, and if so, unquote it. Note that if the refname contains a double quote, then we will have double-quoted it already, so there is no ambiguity.
We test for this case only in the smart protocol, since the DAV-based protocol is not capable of handling this capability. We use UTF-8 because this is nicer in our tests and friendlier to Windows, but the code should work for all non-ASCII refs.
While we're at it, since the name of the option is now well established and isn't going to change, let's inline it instead of using the #define constant.
Reported-by: Frej Bjon <frej.bjon@nemit.fi> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- remote-curl.c | 6 +++++- t/t5541-http-push-smart.sh | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/remote-curl.c b/remote-curl.c index 5cbc6e5002..c9921c552c 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -121,7 +121,11 @@ static int set_option(const char *name, const char *value) } else if (!strcmp(name, "cas")) { struct strbuf val = STRBUF_INIT; - strbuf_addf(&val, "--" CAS_OPT_NAME "=%s", value); + strbuf_addstr(&val, "--force-with-lease="); + if (*value != '"') + strbuf_addstr(&val, value); + else if (unquote_c_style(&val, value, NULL)) + return -1; string_list_append(&cas_options, val.buf); strbuf_release(&val); return 0; diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index 463d0f12e5..187454f5dd 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -479,6 +479,21 @@ test_expect_success 'clone/fetch scrubs password from reflogs' ' ! grep "$HTTPD_URL_USER_PASS" reflog ' +test_expect_success 'Non-ASCII branch name can be used with --force-with-lease' ' + cd "$ROOT_PATH" && + git clone "$HTTPD_URL_USER_PASS/smart/test_repo.git" non-ascii && + cd non-ascii && + git checkout -b rama-de-árbol && + test_commit F && + git push --force-with-lease origin rama-de-árbol && + git ls-remote origin refs/heads/rama-de-árbol >actual && + git ls-remote . refs/heads/rama-de-árbol >expect && + test_cmp expect actual && + git push --delete --force-with-lease origin rama-de-árbol && + git ls-remote origin refs/heads/rama-de-árbol >actual && + test_must_be_empty actual +' + test_expect_success 'colorize errors/hints' ' cd "$ROOT_PATH"/test_repo_clone && test_must_fail git -c color.transport=always -c color.advice=always \