Cannot delete remote branch with non-ascii name and --force-with-lease

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

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(-)

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

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.

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(-)