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

Re: [PATCH] git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Mar 22, 2016, 21:43 UTC
Message-ID
<20160322214355.GI28749@google.com>
In-Reply-To
<20160322195051.GA20563@sigill.intra.peff.net>
Jeff King wrote[1]:
Show 37 quoted lines
> Subject: git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS
>
> The "git -c var=value" option stuffs the config value into
> $GIT_CONFIG_PARAMETERS, so that sub-processes can see it.
> When the config is later read via git_config() or similar,
> we parse it back out of that variable.  The parsing end is a
> little bit picky; it assumes that each entry was generated
> with sq_quote_buf(), and that there is no extraneous
> whitespace.
>
> On the generating end, we are careful to append to an
> existing $GIT_CONFIG_PARAMETERS variable if it exists.
> However, our test for "should we add a space separator" is
> too liberal: it will add one even if the environment
> variable exists but is empty. As a result, you might end up
> with:
>
>    GIT_CONFIG_PARAMETERS=" 'core.foo=bar'"
>
> which the parser will choke on.
>
> This was hard to trigger in older versions of git, since we
> only set the variable when we had something to put into it
> (though you could certainly trigger it manually). But since
> 14111fc (git: submodule honor -c credential.* from command
> line, 2016-02-29), the submodule code will unconditionally
> put the $GIT_CONFIG_PARAMETERS variable into the environment
> of any operation in the submodule, whether it is empty or
> not. So any of those operations which themselves use "git
> -c" will generate the unparseable value and fail.
>
> We can easily fix it by catching this case on the generating
> side. While we're adding a test, let's also check that
> multiple layers of "git -c" work, which was previously not
> tested at all.
>
> Reported-by: Jonathan Nieder <jrnieder@gmail.com>
I should have mentioned this is
Reported-by: Shin Fan <shinfan@google.com>
Show 9 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I just did this on master, and it is standalone. But for the reasons
> above I think it would also be fine to stick on the tip of the
> jk/submodule-c-credential topic.
>
>  config.c               |  2 +-
>  t/t1300-repo-config.sh | 14 ++++++++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Tested-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for the quick fix.

Sincerely, Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/287928/focus=289551
Previous: Junio C HamanoNext: Jacob Keller
Message 25 of 27 in “submodule--helper credential.* pass into submodule”
  1. 0/7 submodule--helper credential.* pass into submoduleJacob Keller, Feb 29, 2016
  2. 1/7 t/lib-httpd: load mod_unixdJacob Keller, Feb 29, 2016
  3. 2/7 submodule: don't pass empty string arguments to submodule--helper cloneJacob Keller, Feb 29, 2016
  4. Stefan BellerFeb 29, 2016
  5. 3/7 submodule: check argc count for git submodule--helper cloneJacob Keller, Feb 29, 2016
  6. Stefan BellerFeb 29, 2016
  7. 4/7 submodule: fix submodule--helper clone usageJacob Keller, Feb 29, 2016
  8. Stefan BellerFeb 29, 2016
  9. 5/7 submodule: fix segmentation fault in submodule--helper cloneJacob Keller, Feb 29, 2016
  10. Stefan BellerFeb 29, 2016
  11. Jacob KellerFeb 29, 2016
  12. 6/7 quote: implement sq_quotef()Jacob Keller, Feb 29, 2016
  13. 7/7 git: submodule honor -c credential.* from command lineJacob Keller, Feb 29, 2016
  14. Stefan BellerFeb 29, 2016
  15. Jacob KellerFeb 29, 2016
  16. Jeff KingMar 1, 2016
  17. Junio C HamanoMar 1, 2016
  18. Jacob KellerMar 1, 2016
  19. Junio C HamanoMar 1, 2016
  20. Jacob KellerMar 1, 2016
  21. Jonathan NiederMar 22, 2016
  22. Jeff KingMar 22, 2016
  23. git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERSJeff King, Mar 22, 2016
  24. Junio C HamanoMar 22, 2016
  25. Jonathan NiederMar 22, 2016
  26. Jacob KellerMar 23, 2016
  27. Junio C HamanoMar 23, 2016

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.