Re: [PATCH v6 14/16] refs/iterator: implement seeking for packed-ref iterators

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 4, 2025, 10:05 UTC
Message-ID: <Z--u2DlaPij_j7zX@pks.im>
In-Reply-To: <Z--tomMthXftrdYA@pks.im>

On Fri, Apr 04, 2025 at 12:00:07PM +0200, Patrick Steinhardt wrote:

Show 49 quoted lines

> On Thu, Apr 03, 2025 at 12:56:39PM -0700, Elijah Newren wrote:
> > On Wed, Mar 12, 2025 at 11:42 PM Patrick Steinhardt <ps@pks.im> wrote:
> > >
> > 
> > > @@ -951,12 +954,41 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)
> > >                                             &iter->oid, iter->flags))
> > >                         continue;
> > >
> > > +               while (prefix && *prefix) {
> > > +                       if (*refname < *prefix)
> > > +                               BUG("packed-refs backend yielded reference preceding its prefix");
> > 
> > I just triggered this bug upon a "git pull" in an internal repository:
> > 
> > $ git pull
> > remote: Enumerating objects: 161255, done.
> > remote: Counting objects: 100% (55884/55884), done.
> > remote: Compressing objects: 100% (5518/5518), done.
> > remote: Total 161255 (delta 54253), reused 50509 (delta 50364),
> > pack-reused 105371 (from 4)
> > Receiving objects: 100% (161255/161255), 309.90 MiB | 16.87 MiB/s, done.
> > Resolving deltas: 100% (118048/118048), completed with 13416 local objects.
> > From github.com:github/github
> >    97ab7ae3f3745..8fb2f9fa180ed  master
> >                     -> origin/master
> > [...snip many screenfuls of updates to origin remotes...]
> > BUG: refs/packed-backend.c:984: packed-refs backend yielded reference
> > preceding its prefix
> > error: fetch died of signal 6
> > 
> > I made a backup of the repo with rsync.
> 
> Thanks, I can indeed reproduce the issue rather easily:
> 
> 	test_expect_success 'list refs with unicode characters' '
> 		test_when_finished "rm -rf repo" &&
> 		git init repo &&
> 		(
> 			cd repo &&
> 			test_commit A &&
> 			git update-ref refs/heads/ HEAD &&
> 			git pack-refs --all &&
> 			git for-each-ref refs/heads/z
> 		)
> 	'
> 
> I'll investigate.
> 
> Patrick

Okay, below patch should fix the issue. The problem is that the sorting we use for refnames is done via `cmp_packed_refname()`, which does the same cast. And because the uppermost bit is set for the emoji character this causes us to compare diferently in `packed_ref_iterator_advance()` and thus causes the bug.

Could you please test whether this works for you? Once confirmed I'll send a proper patch.

Thanks!

Patrick

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index b4289a7d9ce..7e31904bd41 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -980,9 +980,9 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)
 			continue;
 
 		while (prefix && *prefix) {
-			if (*refname < *prefix)
+			if ((unsigned char)*refname < (unsigned char)*prefix)
 				BUG("packed-refs backend yielded reference preceding its prefix");
-			else if (*refname > *prefix)
+			else if ((unsigned char)*refname > (unsigned char)*prefix)
 				return ITER_DONE;
 			prefix++;
 			refname++;