| Message ID | 20200720224327.1631947-1-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | remote-curl: make --force-with-lease work with non-ASCII ref names | expand |
"brian m. carlson" <sandals@crustytoothpaste.net> writes: > diff --git a/remote-curl.c b/remote-curl.c > index 5cbc6e5002..ccf0c27daf 100644 > --- a/remote-curl.c > +++ b/remote-curl.c > @@ -121,7 +121,13 @@ 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, "--" CAS_OPT_NAME "="); > + if (*value == '"') { > + if (unquote_c_style(&val, value, NULL)) > + return -1; > + } else { > + strbuf_addstr(&val, value); > + } I wonder if if (*value != '"') strbuf_addstr(&val, value); else if (unquote_c_style(&val, value, NULL)) return -1; /* error */ is easier to read without having to use {braces}, but that's quite a minor point. A clean-up opportunity I can see here is to declare that we have found a good value for CAS_OPT_NAME and inline the string and remove the C preprocessor macro, but that is obviously an unrelated change to this fix. Thanks.
diff --git a/remote-curl.c b/remote-curl.c index 5cbc6e5002..ccf0c27daf 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -121,7 +121,13 @@ 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, "--" CAS_OPT_NAME "="); + if (*value == '"') { + if (unquote_c_style(&val, value, NULL)) + return -1; + } else { + strbuf_addstr(&val, value); + } 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 \
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. Reported-by: Frej Bjon <frej.bjon@nemit.fi> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- remote-curl.c | 8 +++++++- t/t5541-http-push-smart.sh | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)