{"thread":{"id":"39776","subject":"[PATCH] pager: do not leak \"GIT_PAGER_IN_USE\" to the pager","startedAt":"2015-07-03T17:18:45Z","lastAt":"2015-07-03T17:56:27Z","messageCount":2,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"265464","messageId":"xmqqk2uh5fd6.fsf@gitster.dls.corp.google.com","threadId":"39776","inReplyTo":null,"subject":"[PATCH] pager: do not leak \"GIT_PAGER_IN_USE\" to the pager","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-03T17:18:45Z","receivedAt":"2015-07-03T17:18:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that\nbecomes the upstream of the spawned pager can still tell that we\nhave spawned the pager and decide to do colored output even when\nits output no longer goes to a terminal (i.e. isatty(1)).\n\nBut we forgot to clear it from the enviornment of the spawned pager.\nThis is not a problem in a sane world, but if you have a handful of\nthousands Git users in your organization, somebody is bound to do\nstrange things, e.g. typing \"!<ENTER>\" instead of 'q' to get control\nback from $LESS.  GIT_PAGER_IN_USE is still set in that subshell\nspawned by \"less\", and all sorts of interesting things starts\nhappening, e.g. \"git diff | cat\" starts coloring its output.\n\nWe can clear the environment variable in the half of the fork that\nruns the pager to avoid the confusion.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Arguably, \"git diff | cat\" that is colored is much minor problem\n   than that the user keep using that subshell from pager without\n   realizing.  The user may run more git commands that spawn a pager\n   and do the \"!<ENTER>\" infinite times creating a deep process tree\n   and then \"logout\" many number of times.\n\n pager.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/pager.c b/pager.c\nindex 98b2682..070dc11 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -78,6 +78,7 @@ void setup_pager(void)\n \t\targv_array_push(&pager_process.env_array, \"LESS=FRX\");\n \tif (!getenv(\"LV\"))\n \t\targv_array_push(&pager_process.env_array, \"LV=-c\");\n+\targv_array_push(&pager_process.env_array, \"GIT_PAGER_IN_USE\");\n \tif (start_command(&pager_process))\n \t\treturn;\n \n"},{"id":"265472","messageId":"20150703175627.GA9223@peff.net","threadId":"39776","inReplyTo":"xmqqk2uh5fd6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] pager: do not leak \"GIT_PAGER_IN_USE\" to the pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-03T17:56:27Z","receivedAt":"2015-07-03T17:56:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 03, 2015 at 10:18:45AM -0700, Junio C Hamano wrote:\n\n> Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that\n\nI imagine you mean 2e6c012e here.\n\n> becomes the upstream of the spawned pager can still tell that we\n> have spawned the pager and decide to do colored output even when\n> its output no longer goes to a terminal (i.e. isatty(1)).\n> \n> But we forgot to clear it from the enviornment of the spawned pager.\n> This is not a problem in a sane world, but if you have a handful of\n> thousands Git users in your organization, somebody is bound to do\n> strange things, e.g. typing \"!<ENTER>\" instead of 'q' to get control\n> back from $LESS.  GIT_PAGER_IN_USE is still set in that subshell\n> spawned by \"less\", and all sorts of interesting things starts\n> happening, e.g. \"git diff | cat\" starts coloring its output.\n> \n> We can clear the environment variable in the half of the fork that\n> runs the pager to avoid the confusion.\n\nI think this is a reasonable fix. I guess it would be possible that some\npager would want to react differently depending on the variable, but I\ncould not think of a useful case. And certainly your pager, being the\npager itself, can assume that the pager is in use. ;) At the very worst,\nsomebody can set GIT_PAGER=\"GIT_PAGER_IN_USE=1 my-pager\" if they truly\nwant to do something bizarre.\n\nSo,\n\nAcked-by: Jeff King <peff@peff.net>\n\n-Peff\n"}]}