[PATCH v2 0/2] object-name: fix resolution of object names containing curly braces

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

Maintainer note: these bugs both date back to 2006; neither is a regression
in this cycle.

Changes since v1:

 * Covered the ^{...} cases, and added a testcase for those
 * Added a second patch for another bug discovered by the same reporter,
   where branch:path/to/file/named/major-gaffed is interpreted as a request
   for a commit (namely affed) rather than a blob. (At least, assuming
   commit affed exists)

The second patch has some backward compatibility concerns. People used to be
able to do e.g. git show ${garbage}-g${hash}. I tightened it to
${valid_refname}-${number}-g${hash}, but do we want to allow e.g.
${valid_refname}-g${hash} (allowing the count to be omitted) or maybe even
allow a subset of invalid refnames?

Also for the second patch, while the repository the reporter found the issue
in was something else, I found two open source examples:

 * lore.git: git cat-file -t master:random/path/major-gaffed
 * git.git: git cat-file -t super-invalid~///\\.....@.lock-gfd0bba94e

Elijah Newren (2):
  object-name: fix resolution of object names containing curly braces
  object-name: be more strict in parsing describe-like output

 object-name.c       | 63 ++++++++++++++++++++++++++++++++++++++++++---
 t/t1006-cat-file.sh | 31 +++++++++++++++++++++-
 t/t6120-describe.sh | 22 ++++++++++++++++
 3 files changed, 111 insertions(+), 5 deletions(-)


base-commit: 063bcebf0c917140ca0e705cbe0fdea127e90086
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1844%2Fnewren%2Fobject-name-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1844/newren/object-name-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1844

Range-diff vs v1:

 1:  1671b773fcc ! 1:  13f68bebe90 object-name: fix resolution of object names containing curly braces
     @@ Commit message
          should succeed (assuming that branch had a README.md file, of course).
          However, the change in cce91a2caef9 (Change 'master@noon' syntax to
          'master@{noon}'., 2006-05-19) presumed that curly braces would always
     -    come after an '@' and be paired, causing 'foo{bar:README.md' to
     -    entirely miss the ':' and assume there's no object being referenced.
     +    come after an '@' or '^' and be paired, causing e.g. 'foo{bar:README.md'
     +    to entirely miss the ':' and assume there's no object being referenced.
          In short, git would report:
      
              fatal: Not a valid object name foo{bar:README.md
      
          Change the parsing to only make the assumption of paired curly braces
     -    immediately after a '@' character appears.
     +    immediately after either a '@' or '^' character appears.
      
     -    Add tests for both this and 'foo@@{...}' cases, which an initial version
     -    of this patch broke.
     +    Add tests for this, as well as for a few other test cases that initial
     +    versions of this patch broke:
     +      * 'foo@@{...}'
     +      * 'foo^{/${SEARCH_TEXT_WITH_COLON}}:${PATH}'
      
          Reported-by: Gabriel Amaral <gabriel-amaral@github.com>
          Helped-by: Michael Haggerty <mhagger@github.com>
     @@ object-name.c: static enum get_oid_result get_oid_with_context_1(struct reposito
       	}
       	for (cp = name, bracket_depth = 0; *cp; cp++) {
      -		if (*cp == '{')
     -+		if (*cp == '@' && *(cp+1) == '{') {
     ++		if (*(cp+1) == '{' && (*cp == '@' || *cp == '^')) {
      +			cp++;
       			bracket_depth++;
      -		else if (bracket_depth && *cp == '}')
     @@ object-name.c: static enum get_oid_result get_oid_with_context_1(struct reposito
       		struct object_id tree_oid;
      
       ## t/t1006-cat-file.sh ##
     +@@ t/t1006-cat-file.sh: test_expect_success "setup" '
     + 	git config extensions.objectformat $test_hash_algo &&
     + 	git config extensions.compatobjectformat $test_compat_hash_algo &&
     + 	echo_without_newline "$hello_content" > hello &&
     +-	git update-index --add hello
     ++	git update-index --add hello &&
     ++	git commit -m "add hello file"
     + '
     + 
     + run_blob_tests () {
      @@ t/t1006-cat-file.sh: test_expect_success FUNNYNAMES '--batch-check, -Z with newline in input' '
       	test_cmp expect actual
       '
       
     -+test_expect_success FUNNYNAMES 'setup with curly braches in input' '
     -+	git branch "foo{bar" &&
     -+	git branch "foo@"
     ++test_expect_success 'setup with curly braches in input' '
     ++	git branch "foo{bar" HEAD &&
     ++	git branch "foo@" HEAD
      +'
      +
     -+test_expect_success FUNNYNAMES 'object reference with curly brace' '
     ++test_expect_success 'object reference with curly brace' '
      +	git cat-file -p "foo{bar:hello" >actual &&
      +	git cat-file -p HEAD:hello >expect &&
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success FUNNYNAMES 'object reference with at-sign' '
     ++test_expect_success 'object reference with at-sign' '
      +	git cat-file -p "foo@@{0}:hello" >actual &&
      +	git cat-file -p HEAD:hello >expect &&
      +	test_cmp expect actual
      +'
     ++
     ++test_expect_success 'setup with commit with colon' '
     ++	git commit-tree -m "testing: just a bunch of junk" HEAD^{tree} >out &&
     ++	git branch other $(cat out)
     ++'
     ++
     ++test_expect_success 'object reference via commit text search' '
     ++	git cat-file -p "other^{/testing:}:hello" >actual &&
     ++	git cat-file -p HEAD:hello >expect &&
     ++	test_cmp expect actual
     ++'
      +
       test_expect_success 'setup blobs which are likely to delta' '
       	test-tool genrandom foo 10240 >foo &&
 -:  ----------- > 2:  31f1c37b31a object-name: be more strict in parsing describe-like output

-- 
gitgitgadget