[PATCH v2 0/2] object-name: fix resolution of object names containing curly braces
- From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Jan 4, 2025, 00:17 UTC
- Message-ID
- <pull.1844.v2.git.1735949870.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1844.git.1735699989371.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