git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH] object-name: fix resolution of object names containing curly braces
Date: Wed, 01 Jan 2025 09:00:59 -0800	[thread overview]
Message-ID: <xmqqpll6wjsk.fsf@gitster.g> (raw)
In-Reply-To: <pull.1844.git.1735699989371.gitgitgadget@gmail.com> (Elijah Newren via GitGitGadget's message of "Wed, 01 Jan 2025 02:53:09 +0000")

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> Given a branch name of 'foo{bar', commands like
>
>     git cat-file -p foo{bar:README.md
>
> 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.
> 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.

Interesting.  I wonder if this looseness was to ensure that we won't
mistake a colon inside "master^{/title with : a colon}" as a start
of a subpath, instead of asking for a commit with a title that
happens to have a colon in it?

> Add tests for both this and 'foo@@{...}' cases, which an initial version
> of this patch broke.

Thanks for being extra careful here.


> Reported-by: Gabriel Amaral <gabriel-amaral@github.com>
> Helped-by: Michael Haggerty <mhagger@github.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>     object-name: fix resolution of object names containing curly braces
>     
>     Maintainer note: this bug dates back to 2006; it is not a regression in
>     this cycle.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1844%2Fnewren%2Fobject-name-fix-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1844/newren/object-name-fix-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1844
>
>  object-name.c       |  8 +++++---
>  t/t1006-cat-file.sh | 17 +++++++++++++++++
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/object-name.c b/object-name.c
> index c892fbe80aa..e92f26b3256 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -2087,12 +2087,14 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
>  		return -1;
>  	}
>  	for (cp = name, bracket_depth = 0; *cp; cp++) {
> -		if (*cp == '{')
> +		if (*cp == '@' && *(cp+1) == '{') {
> +			cp++;
>  			bracket_depth++;
> -		else if (bracket_depth && *cp == '}')
> +		} else if (bracket_depth && *cp == '}') {
>  			bracket_depth--;
> -		else if (!bracket_depth && *cp == ':')
> +		} else if (!bracket_depth && *cp == ':') {
>  			break;
> +		}
>  	}
>  	if (*cp == ':') {
>  		struct object_id tree_oid;
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index d36cd7c0863..252485dac78 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -603,6 +603,23 @@ 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 FUNNYNAMES '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' '
> +	git cat-file -p "foo@@{0}: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 &&
>  	{ cat foo && echo plus; } >foo-plus &&
>
> base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f

  reply	other threads:[~2025-01-01 17:01 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-01  2:53 [PATCH] object-name: fix resolution of object names containing curly braces Elijah Newren via GitGitGadget
2025-01-01 17:00 ` Junio C Hamano [this message]
2025-01-03 23:34   ` Elijah Newren
2025-01-04  2:52     ` Junio C Hamano
2025-01-03  8:16 ` Patrick Steinhardt
2025-01-03 15:46   ` Junio C Hamano
2025-01-03 23:43   ` Elijah Newren
2025-01-04  0:17 ` [PATCH v2 0/2] " Elijah Newren via GitGitGadget
2025-01-04  0:17   ` [PATCH v2 1/2] " Elijah Newren via GitGitGadget
2025-01-04 17:26     ` Junio C Hamano
2025-01-04 18:54       ` Elijah Newren
2025-01-05 16:14     ` Junio C Hamano
2025-01-04  0:17   ` [PATCH v2 2/2] object-name: be more strict in parsing describe-like output Elijah Newren via GitGitGadget
2025-01-04 14:35   ` [PATCH v2 0/2] object-name: fix resolution of object names containing curly braces Junio C Hamano
2025-01-04 15:55     ` Elijah Newren
2025-01-04 17:51       ` Junio C Hamano
2025-01-04 18:55         ` Elijah Newren
2025-01-06 17:29   ` Junio C Hamano
2025-01-06 19:26     ` Elijah Newren
2025-01-06 20:38       ` Junio C Hamano
2025-01-13 17:13   ` [PATCH v3 0/2] object-name: fix a pair of object name resolution issues Elijah Newren via GitGitGadget
2025-01-13 17:13     ` [PATCH v3 1/2] object-name: fix resolution of object names containing curly braces Elijah Newren via GitGitGadget
2025-01-13 17:13     ` [PATCH v3 2/2] object-name: be more strict in parsing describe-like output Elijah Newren via GitGitGadget
2025-01-13 18:15     ` [PATCH v3 0/2] object-name: fix a pair of object name resolution issues Junio C Hamano
2025-01-13 19:26       ` Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqpll6wjsk.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).