Re: [PATCH] archive-tar: fix sanity check in config parsing
- From
Joachim Schmitz <jojo@schmitz-digital.de>
- Date
- Jan 14, 2013, 08:17 UTC
- Message-ID
- <kd0evl$ac0$1@ger.gmane.org>
- In-Reply-To
- <20130113200044.GA3979@sigill.intra.peff.net>
Jeff King wrote:
Show 35 quoted lines
> On Sun, Jan 13, 2013 at 06:42:01PM +0100, René Scharfe wrote: > >> When parsing these config variable names, we currently check that >> the second dot is found nine characters into the name, disallowing >> filter names with a length of five characters. Additionally, >> git archive crashes when the second dot is omitted: >> >> $ ./git -c tar.foo=bar archive HEAD >/dev/null >> fatal: Data too large to fit into virtual memory space. >> >> Instead we should check if the second dot exists at all, or if >> we only found the first one. > > Eek. Thanks for finding it. Your fix is obviously correct. > >> --- a/archive-tar.c >> +++ b/archive-tar.c >> @@ -335,7 +335,7 @@ static int tar_filter_config(const char *var, >> const char *value, void *data) if (prefixcmp(var, "tar.")) >> return 0; >> dot = strrchr(var, '.'); >> - if (dot == var + 9) >> + if (dot == var + 3) >> return 0; > > For the curious, the original version of the patch[1] read: > > + if (prefixcmp(var, "tarfilter.")) > + return 0; > + dot = strrchr(var, '.'); > + if (dot == var + 9) > + return 0; > > and when I shortened the config section to "tar" in a re-roll of the > series, I missed the corresponding change to the offset.
Wouldn't it then be better ti use strlen("tar") rather than a 3? Or at least a comment?
Bye, Jojo