Re: [PATCH 00/24] SHA-256 test fixes, part 8
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jan 13, 2020, 23:17 UTC
- Message-ID
- <20200113231705.GU6570@camp.crustytoothpaste.net>
- In-Reply-To
- <CAPig+cReDPs8D1L3+Z4sMn__JuU-Ezs+fOSF00=_tNxa0DotFQ@mail.gmail.com>
On 2020-01-13 at 13:41:44, Eric Sunshine wrote:
Show 25 quoted lines
> On Mon, Jan 13, 2020 at 7:40 AM brian m. carlson > <sandals@crustytoothpaste.net> wrote: > > I suspect that t3404 also has a bug, since the object IDs that are > > supposed to collide do not, according to my instrumentation of the test. > > I'm unsure what the intended collision was and consequently haven't > > fixed it. However, it does work with SHA-256 as it stands and is no > > more or less functional than with SHA-1, so I've removed the > > prerequisite. > > The test itself is fine, but it is one of those unfortunate cases of > checking for absence of something (which is a wide net). As explained > by the commit message[1] of the patch which added the test, the > collision occurred only between short OID's. The patch[2] which fixed > the problem did so by avoiding short OID's in the scripted > implementation of `git rebase -i` (and also flipped the test from > `text_expect_failure` to `test_expect_success`). > > The test, as currently implemented, is very much specific to SHA-1 > since the FAKE_COMMIT_MESSAGE="collide2 ac4f2ee" it uses only produces > a collision with short OID's when SHA-1 is the hashing function, so > the prerequisite is correct and serves as documentation (even if it > doesn't affect the outcome of the test). Removing that prerequisite > should only be done if the test is updated with a different > FAKE_COMMIT_MESSAGE which causes a short OID collision when SHA-256 is > used.
I'll take another look. When I looked at the output, it looked like they didn't collide anymore even under SHA-1, but perhaps I instrumented the test wrong and therefore got the wrong result. Thanks for double checking.
-- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204