On Tue, Jun 11, 2024 at 11:16:03PM +0000, brian m. carlson wrote: > On 2024-06-11 at 11:58:47, Patrick Steinhardt wrote: > > The IPC socket used by the fsmonitor on Darwin is usually contained in > > the Git repository itself. When the repository is hosted on a networked > > filesystem though, we instead create the socket path in the user's home > > directory or the socket directory. In that case, we derive the path by > > hashing the repository path. > > > > The hashing implicitly depends on `the_repository` though via > > `hash_to_hex()`. For one, this is wrong because we really should be > > using the passed-in repository. But arguably, it is not sensible to > > derive the path hash from the repository's object hash in the first > > place -- they don't have anything to do with each other, and a > > repository that is hosted in the same path should always derive the same > > IPC socket path. > > > > Fix this by unconditionally using SHA1 to derive the path. > > Let's instead use SHA-256 to derive the path. I can imagine that there > might be a time when some users would like to drop support for SHA-1 > altogether (for FIPS compliance reasons, say) and we'll make our lives a > lot easier if we avoid more uses of SHA-1. > > It is also typically faster when compiled appropriately, although the > amount of data we're processing is very small. I only now realize that this actually a bug. The hash is already getting computed as SHA1 unconditionally like this: git_SHA1_Init(&sha1ctx); git_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree)); git_SHA1_Final(hash, &sha1ctx); And then we used to pass the computed hash to `hash_to_hex()`, which is of course the wrong thing to do in a SHA256 repository because we would end up printing `GIT_MAX_RAWSZ - GIT_SHA1_RAWSZ` uninitialized bytes. I agree that we want to convert this to SHA256 eventually. But I'd say we should keep such a backwards-incompatible change out of this patch series and handle it as a follow-up. I'll rephrase the commit message though. Patrick
Attachment:
signature.asc
Description: PGP signature
