* Cannot delete remote branch with non-ascii name and --force-with-lease
@ 2020-07-15 9:10 Frej Bjon
2020-07-20 22:43 ` [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names brian m. carlson
2020-07-21 1:15 ` [PATCH v2] " brian m. carlson
0 siblings, 2 replies; 4+ messages in thread
From: Frej Bjon @ 2020-07-15 9:10 UTC (permalink / raw)
To: git
What did you do before the bug happened? (Steps to reproduce your issue)
- Create a branch with non-ascii character: testbranch-ä
- Push to a remote with same name
- Delete branch from remote with --force-with-lease:
git --force-with-lease --delete origin "testbranch-ä"
What did you expect to happen? (Expected behavior)
- The branch to be deleted.
What happened instead? (Actual behavior)
- Git fails with an error
error: cannot parse expected object name
'32e33fb544d7b1147c446c60220f23a5df465eff"'
Anything else you want to add:
- The command works if using only --force and not --force-with-lease
- The command works if there's no non-ascii character, e.g. "testbranch-x"
- Tested with gitlab and github as remotes, with the same error
- My locale in git bash is "en_US.UTF-8"
- The same happens under Ubuntu 20.04 with git 2.25.1
[System Info]
git version: git version 2.27.0.windows.1
cpu: x86_64
built from commit: 907ab1011dce9112700498e034b974ba60f8b407
sizeof-long: 4
sizeof-size_t: 8
uname: Windows 10.0 15063
compiler info: gnuc: 10.1
libc info: no libc information available
[Enabled Hooks]
None
Regards,
Frej Bjon
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names
2020-07-15 9:10 Cannot delete remote branch with non-ascii name and --force-with-lease Frej Bjon
@ 2020-07-20 22:43 ` brian m. carlson
2020-07-21 0:02 ` Junio C Hamano
2020-07-21 1:15 ` [PATCH v2] " brian m. carlson
1 sibling, 1 reply; 4+ messages in thread
From: brian m. carlson @ 2020-07-20 22:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Frej Bjon
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(-)
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 \
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names
2020-07-20 22:43 ` [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names brian m. carlson
@ 2020-07-21 0:02 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2020-07-21 0:02 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Frej Bjon
"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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] remote-curl: make --force-with-lease work with non-ASCII ref names
2020-07-15 9:10 Cannot delete remote branch with non-ascii name and --force-with-lease Frej Bjon
2020-07-20 22:43 ` [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names brian m. carlson
@ 2020-07-21 1:15 ` brian m. carlson
1 sibling, 0 replies; 4+ messages in thread
From: brian m. carlson @ 2020-07-21 1:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Frej Bjon
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 \
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-21 1:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 9:10 Cannot delete remote branch with non-ascii name and --force-with-lease Frej Bjon
2020-07-20 22:43 ` [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names brian m. carlson
2020-07-21 0:02 ` Junio C Hamano
2020-07-21 1:15 ` [PATCH v2] " brian m. carlson
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).