{"thread":{"id":"53862","subject":"Cannot delete remote branch with non-ascii name and --force-with-lease","startedAt":"2020-07-15T09:10:36Z","lastAt":"2020-07-21T01:26:20Z","messageCount":4,"participants":["Frej Bjon","brian m. carlson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"401580","messageId":"CAP8P1T+JQUTVDXsJbrWkiZ2AWZmoptFSRdKssANYrNSFa+Egmg@mail.gmail.com","threadId":"53862","inReplyTo":null,"subject":"Cannot delete remote branch with non-ascii name and --force-with-lease","fromName":"Frej Bjon","fromEmail":"frej.bjon@nemit.fi","sentAt":"2020-07-15T09:10:21Z","receivedAt":"2020-07-15T09:10:36Z","isPatch":false,"sender":{"key":"frej.bjon@nemit.fi","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n- Create a branch with non-ascii character: testbranch-ä\n- Push to a remote with same name\n- Delete branch from remote with --force-with-lease:\n git --force-with-lease --delete origin \"testbranch-ä\"\n\nWhat did you expect to happen? (Expected behavior)\n- The branch to be deleted.\n\nWhat happened instead? (Actual behavior)\n- Git fails with an error\n error: cannot parse expected object name\n'32e33fb544d7b1147c446c60220f23a5df465eff\"'\n\nAnything else you want to add:\n- The command works if using only --force and not --force-with-lease\n- The command works if there's no non-ascii character, e.g. \"testbranch-x\"\n- Tested with gitlab and github as remotes, with the same error\n- My locale in git bash is \"en_US.UTF-8\"\n- The same happens under Ubuntu 20.04 with git 2.25.1\n\n\n[System Info]\ngit version: git version 2.27.0.windows.1\ncpu: x86_64\nbuilt from commit: 907ab1011dce9112700498e034b974ba60f8b407\nsizeof-long: 4\nsizeof-size_t: 8\nuname: Windows 10.0 15063\ncompiler info: gnuc: 10.1\nlibc info: no libc information available\n\n\n[Enabled Hooks]\nNone\n\nRegards,\nFrej Bjon\n"},{"id":"401789","messageId":"20200720224327.1631947-1-sandals@crustytoothpaste.net","threadId":"53862","inReplyTo":"CAP8P1T+JQUTVDXsJbrWkiZ2AWZmoptFSRdKssANYrNSFa+Egmg@mail.gmail.com","subject":"[PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-07-20T22:43:27Z","receivedAt":"2020-07-20T22:44:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we invoke a remote transport helper and pass an option with an\nargument, we quote the argument as a C-style string if necessary. This\nis the case for the cas option, which implements the --force-with-lease\ncommand-line flag, when we're passing a non-ASCII refname.\n\nHowever, the remote curl helper isn't designed to parse such an\nargument, meaning that if we try to use --force-with-lease with an HTTP\npush and a non-ASCII refname, we get an error like this:\n\n error: cannot parse expected object name '0000000000000000000000000000000000000000\"'\n\nNote the double quote, which get_oid has reminded us is not valid in an\nhex object ID.\n\nEven if we had been able to parse it, we would send the wrong data to\nthe server: we'd send an escaped ref, which would not behave as the user\nwanted and might accidentally result in updating or deleting a ref we\nhadn't intended.\n\nSince we need to expect a quoted C-style string here, just check if the\nfirst argument is a double quote, and if so, unquote it. Note that if\nthe refname contains a double quote, then we will have double-quoted it\nalready, so there is no ambiguity.\n\nWe test for this case only in the smart protocol, since the DAV-based\nprotocol is not capable of handling this capability. We use UTF-8\nbecause this is nicer in our tests and friendlier to Windows, but the\ncode should work for all non-ASCII refs.\n\nReported-by: Frej Bjon \nSigned-off-by: brian m. carlson \n---\n remote-curl.c | 8 +++++++-\n t/t5541-http-push-smart.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 5cbc6e5002..ccf0c27daf 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -121,7 +121,13 @@ static int set_option(const char *name, const char *value)\n \t}\n \telse if (!strcmp(name, \"cas\")) {\n \t\tstruct strbuf val = STRBUF_INIT;\n-\t\tstrbuf_addf(&val, \"--\" CAS_OPT_NAME \"=%s\", value);\n+\t\tstrbuf_addstr(&val, \"--\" CAS_OPT_NAME \"=\");\n+\t\tif (*value == '\"') {\n+\t\t\tif (unquote_c_style(&val, value, NULL))\n+\t\t\t\treturn -1;\n+\t\t} else {\n+\t\t\tstrbuf_addstr(&val, value);\n+\t\t}\n \t\tstring_list_append(&cas_options, val.buf);\n \t\tstrbuf_release(&val);\n \t\treturn 0;\ndiff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh\nindex 463d0f12e5..187454f5dd 100755\n--- a/t/t5541-http-push-smart.sh\n+++ b/t/t5541-http-push-smart.sh\n@@ -479,6 +479,21 @@ test_expect_success 'clone/fetch scrubs password from reflogs' '\n \t! grep \"$HTTPD_URL_USER_PASS\" reflog\n '\n \n+test_expect_success 'Non-ASCII branch name can be used with --force-with-lease' '\n+\tcd \"$ROOT_PATH\" &&\n+\tgit clone \"$HTTPD_URL_USER_PASS/smart/test_repo.git\" non-ascii &&\n+\tcd non-ascii &&\n+\tgit checkout -b rama-de-árbol &&\n+\ttest_commit F &&\n+\tgit push --force-with-lease origin rama-de-árbol &&\n+\tgit ls-remote origin refs/heads/rama-de-árbol >actual &&\n+\tgit ls-remote . refs/heads/rama-de-árbol >expect &&\n+\ttest_cmp expect actual &&\n+\tgit push --delete --force-with-lease origin rama-de-árbol &&\n+\tgit ls-remote origin refs/heads/rama-de-árbol >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'colorize errors/hints' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_must_fail git -c color.transport=always -c color.advice=always \\\n"},{"id":"401792","messageId":"xmqqlfjd91ll.fsf@gitster.c.googlers.com","threadId":"53862","inReplyTo":"20200720224327.1631947-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] remote-curl: make --force-with-lease work with non-ASCII ref names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-21T00:02:46Z","receivedAt":"2020-07-21T00:04:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 5cbc6e5002..ccf0c27daf 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -121,7 +121,13 @@ static int set_option(const char *name, const char *value)\n> \t}\n> \telse if (!strcmp(name, \"cas\")) {\n> \t\tstruct strbuf val = STRBUF_INIT;\n> -\t\tstrbuf_addf(&val, \"--\" CAS_OPT_NAME \"=%s\", value);\n> +\t\tstrbuf_addstr(&val, \"--\" CAS_OPT_NAME \"=\");\n> +\t\tif (*value == '\"') {\n> +\t\t\tif (unquote_c_style(&val, value, NULL))\n> +\t\t\t\treturn -1;\n> +\t\t} else {\n> +\t\t\tstrbuf_addstr(&val, value);\n> +\t\t}\n\nI wonder if\n\n\t\tif (*value != '\"')\n\t\t\tstrbuf_addstr(&val, value);\n\t\telse if (unquote_c_style(&val, value, NULL))\n\t\t\treturn -1; /* error */\n\nis easier to read without having to use {braces}, but that's quite\na minor point.\n\nA clean-up opportunity I can see here is to declare that we have\nfound a good value for CAS_OPT_NAME and inline the string and remove\nthe C preprocessor macro, but that is obviously an unrelated change\nto this fix.\n\nThanks.\n"},{"id":"401801","messageId":"20200721011511.1649048-1-sandals@crustytoothpaste.net","threadId":"53862","inReplyTo":"CAP8P1T+JQUTVDXsJbrWkiZ2AWZmoptFSRdKssANYrNSFa+Egmg@mail.gmail.com","subject":"[PATCH v2] remote-curl: make --force-with-lease work with non-ASCII ref names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-07-21T01:15:11Z","receivedAt":"2020-07-21T01:26:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we invoke a remote transport helper and pass an option with an\nargument, we quote the argument as a C-style string if necessary. This\nis the case for the cas option, which implements the --force-with-lease\ncommand-line flag, when we're passing a non-ASCII refname.\n\nHowever, the remote curl helper isn't designed to parse such an\nargument, meaning that if we try to use --force-with-lease with an HTTP\npush and a non-ASCII refname, we get an error like this:\n\n error: cannot parse expected object name '0000000000000000000000000000000000000000\"'\n\nNote the double quote, which get_oid has reminded us is not valid in an\nhex object ID.\n\nEven if we had been able to parse it, we would send the wrong data to\nthe server: we'd send an escaped ref, which would not behave as the user\nwanted and might accidentally result in updating or deleting a ref we\nhadn't intended.\n\nSince we need to expect a quoted C-style string here, just check if the\nfirst argument is a double quote, and if so, unquote it. Note that if\nthe refname contains a double quote, then we will have double-quoted it\nalready, so there is no ambiguity.\n\nWe test for this case only in the smart protocol, since the DAV-based\nprotocol is not capable of handling this capability. We use UTF-8\nbecause this is nicer in our tests and friendlier to Windows, but the\ncode should work for all non-ASCII refs.\n\nWhile we're at it, since the name of the option is now well established\nand isn't going to change, let's inline it instead of using the #define\nconstant.\n\nReported-by: Frej Bjon \nSigned-off-by: brian m. carlson \n---\n remote-curl.c | 6 +++++-\n t/t5541-http-push-smart.sh | 15 +++++++++++++++\n 2 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 5cbc6e5002..c9921c552c 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -121,7 +121,11 @@ static int set_option(const char *name, const char *value)\n \t}\n \telse if (!strcmp(name, \"cas\")) {\n \t\tstruct strbuf val = STRBUF_INIT;\n-\t\tstrbuf_addf(&val, \"--\" CAS_OPT_NAME \"=%s\", value);\n+\t\tstrbuf_addstr(&val, \"--force-with-lease=\");\n+\t\tif (*value != '\"')\n+\t\t\tstrbuf_addstr(&val, value);\n+\t\telse if (unquote_c_style(&val, value, NULL))\n+\t\t\treturn -1;\n \t\tstring_list_append(&cas_options, val.buf);\n \t\tstrbuf_release(&val);\n \t\treturn 0;\ndiff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh\nindex 463d0f12e5..187454f5dd 100755\n--- a/t/t5541-http-push-smart.sh\n+++ b/t/t5541-http-push-smart.sh\n@@ -479,6 +479,21 @@ test_expect_success 'clone/fetch scrubs password from reflogs' '\n \t! grep \"$HTTPD_URL_USER_PASS\" reflog\n '\n \n+test_expect_success 'Non-ASCII branch name can be used with --force-with-lease' '\n+\tcd \"$ROOT_PATH\" &&\n+\tgit clone \"$HTTPD_URL_USER_PASS/smart/test_repo.git\" non-ascii &&\n+\tcd non-ascii &&\n+\tgit checkout -b rama-de-árbol &&\n+\ttest_commit F &&\n+\tgit push --force-with-lease origin rama-de-árbol &&\n+\tgit ls-remote origin refs/heads/rama-de-árbol >actual &&\n+\tgit ls-remote . refs/heads/rama-de-árbol >expect &&\n+\ttest_cmp expect actual &&\n+\tgit push --delete --force-with-lease origin rama-de-árbol &&\n+\tgit ls-remote origin refs/heads/rama-de-árbol >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'colorize errors/hints' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \ttest_must_fail git -c color.transport=always -c color.advice=always \\\n"}]}