| Message ID | 20190212012256.1005924-31-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Hash function transition part 16 | expand |
On Tue, Feb 12 2019, brian m. carlson wrote: > The cat_blob function was matching on exactly 40 hex characters. This > won't work with SHA-256, which uses 64-character hex object IDs. While > it should be fine to simply match any number of hex characters since the > output is space delimited, be extra safe by matching either exactly 40 > or exactly 64 hex characters. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > perl/Git.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/perl/Git.pm b/perl/Git.pm > index d856930b2e..62c472e0ce 100644 > --- a/perl/Git.pm > +++ b/perl/Git.pm > @@ -980,7 +980,7 @@ sub cat_blob { > return -1; > } > > - if ($description !~ /^[0-9a-fA-F]{40} \S+ (\d+)$/) { > + if ($description !~ /^[0-9a-fA-F]{40}(?:[0-9a-fA-F]{24})? \S+ (\d+)$/) { > carp "Unexpected result returned from git cat-file"; > return -1; > } The gitweb code doesn't load Git.pm now, but does anyone know a reason for why we'd avoid any perl/* dependency in the gitweb code? If not the regex here & in gitweb could be factored into e.g. a tiny Git::OID library which would either just expose a $GIT::OID_REGEX, or something like the sort of interface (might not be worth it) that I suggested in my feedback to 31/31.
On Tue, Feb 12, 2019 at 11:59:27AM +0100, Ævar Arnfjörð Bjarmason wrote: > The gitweb code doesn't load Git.pm now, but does anyone know a reason > for why we'd avoid any perl/* dependency in the gitweb code? If not the > regex here & in gitweb could be factored into e.g. a tiny Git::OID > library which would either just expose a $GIT::OID_REGEX, or something > like the sort of interface (might not be worth it) that I suggested in > my feedback to 31/31. I think one potential issue here is that some distributors bundle the Perl modules with git send-email and not gitweb, and they're packaged independently. I'm not opposed to seeing a patch to do that, but it probably belongs in its own series.
On Mon, Feb 18 2019, brian m. carlson wrote: > On Tue, Feb 12, 2019 at 11:59:27AM +0100, Ævar Arnfjörð Bjarmason wrote: >> The gitweb code doesn't load Git.pm now, but does anyone know a reason >> for why we'd avoid any perl/* dependency in the gitweb code? If not the >> regex here & in gitweb could be factored into e.g. a tiny Git::OID >> library which would either just expose a $GIT::OID_REGEX, or something >> like the sort of interface (might not be worth it) that I suggested in >> my feedback to 31/31. > > I think one potential issue here is that some distributors bundle the > Perl modules with git send-email and not gitweb, and they're packaged > independently. I'm not opposed to seeing a patch to do that, but it > probably belongs in its own series. In packaging terms that one's easy to juggle, FWIW I'm just aware of e.g. RedHat packaging perl-Git as its own thing depended on by git-svn, git-email & friends, not anyone who packages it with git-send-email specifically. But according to gitweb/README it's also meant to be used as a stand-alone script. So: * You can wget it from the git.git repo & run it * You can run it with any (reasonable) version of git * If you're missing perl modules, those are all on CPAN (just CGI.pm should be missing these days) Making it depend on any module we ship would break all those things. I don't know if anyone cares, but avoiding this very minor copy/pasting seems like a bad reason to wake that particular dragon.
diff --git a/perl/Git.pm b/perl/Git.pm index d856930b2e..62c472e0ce 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -980,7 +980,7 @@ sub cat_blob { return -1; } - if ($description !~ /^[0-9a-fA-F]{40} \S+ (\d+)$/) { + if ($description !~ /^[0-9a-fA-F]{40}(?:[0-9a-fA-F]{24})? \S+ (\d+)$/) { carp "Unexpected result returned from git cat-file"; return -1; }
The cat_blob function was matching on exactly 40 hex characters. This won't work with SHA-256, which uses 64-character hex object IDs. While it should be fine to simply match any number of hex characters since the output is space delimited, be extra safe by matching either exactly 40 or exactly 64 hex characters. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- perl/Git.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)