git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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 
Previous: Jeff KingNext: Jeff King
Message 3 of 31 in “archive-tar: fix sanity check in config parsing”
  1. archive-tar: fix sanity check in config parsingRené Scharfe, Jan 13, 2013
  2. Jeff KingJan 13, 2013
  3. Joachim SchmitzJan 14, 2013
  4. Jeff KingJan 14, 2013
  5. Jeff KingJan 14, 2013
  6. 1/6 config: add helper function for parsing key namesJeff King, Jan 14, 2013
  7. Junio C HamanoJan 14, 2013
  8. Jeff KingJan 15, 2013
  9. Junio C HamanoJan 15, 2013
  10. Junio C HamanoJan 18, 2013
  11. 0/8 config key-parsing cleanupsJeff King, Jan 23, 2013
  12. 1/8 config: add helper function for parsing key namesJeff King, Jan 23, 2013
  13. 2/8 archive-tar: use parse_config_key when parsing configJeff King, Jan 23, 2013
  14. 3/8 convert some config callbacks to parse_config_keyJeff King, Jan 23, 2013
  15. 4/8 userdiff: drop parse_driver functionJeff King, Jan 23, 2013
  16. 5/8 submodule: use parse_config_key when parsing configJeff King, Jan 23, 2013
  17. Jens LehmannJan 23, 2013
  18. 6/8 submodule: simplify memory handling in config parsingJeff King, Jan 23, 2013
  19. Jens LehmannJan 23, 2013
  20. 7/8 help: use parse_config_key for man configJeff King, Jan 23, 2013
  21. 8/8 reflog: use parse_config_key in config callbackJeff King, Jan 23, 2013
  22. Junio C HamanoJan 23, 2013
  23. Jonathan NiederJan 23, 2013
  24. 2/6 archive-tar: use match_config_key when parsing configJeff King, Jan 14, 2013
  25. 3/6 convert some config callbacks to match_config_keyJeff King, Jan 14, 2013
  26. Jonathan NiederJan 14, 2013
  27. Jeff KingJan 14, 2013
  28. Jeff KingJan 14, 2013
  29. 4/6 userdiff: drop parse_driver functionJeff King, Jan 14, 2013
  30. 5/6 submodule: use match_config_key when parsing configJeff King, Jan 14, 2013
  31. 6/6 submodule: simplify memory handling in config parsingJeff King, Jan 14, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.