Re: [PATCH] pager: do not leak GIT_PAGER_IN_USE to the pager
On Fri, Jul 03, 2015 at 10:18:45AM -0700, Junio C Hamano wrote: Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that I imagine you mean 2e6c012e here. becomes the upstream of the spawned pager can still tell that we have spawned the pager and decide to do colored output even when its output no longer goes to a terminal (i.e. isatty(1)). But we forgot to clear it from the enviornment of the spawned pager. This is not a problem in a sane world, but if you have a handful of thousands Git users in your organization, somebody is bound to do strange things, e.g. typing !ENTER instead of 'q' to get control back from $LESS. GIT_PAGER_IN_USE is still set in that subshell spawned by less, and all sorts of interesting things starts happening, e.g. git diff | cat starts coloring its output. We can clear the environment variable in the half of the fork that runs the pager to avoid the confusion. I think this is a reasonable fix. I guess it would be possible that some pager would want to react differently depending on the variable, but I could not think of a useful case. And certainly your pager, being the pager itself, can assume that the pager is in use. ;) At the very worst, somebody can set GIT_PAGER=GIT_PAGER_IN_USE=1 my-pager if they truly want to do something bizarre. So, Acked-by: Jeff King p...@peff.net -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pager: do not leak GIT_PAGER_IN_USE to the pager
Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that becomes the upstream of the spawned pager can still tell that we have spawned the pager and decide to do colored output even when its output no longer goes to a terminal (i.e. isatty(1)). But we forgot to clear it from the enviornment of the spawned pager. This is not a problem in a sane world, but if you have a handful of thousands Git users in your organization, somebody is bound to do strange things, e.g. typing !ENTER instead of 'q' to get control back from $LESS. GIT_PAGER_IN_USE is still set in that subshell spawned by less, and all sorts of interesting things starts happening, e.g. git diff | cat starts coloring its output. We can clear the environment variable in the half of the fork that runs the pager to avoid the confusion. Signed-off-by: Junio C Hamano gits...@pobox.com --- * Arguably, git diff | cat that is colored is much minor problem than that the user keep using that subshell from pager without realizing. The user may run more git commands that spawn a pager and do the !ENTER infinite times creating a deep process tree and then logout many number of times. pager.c | 1 + 1 file changed, 1 insertion(+) diff --git a/pager.c b/pager.c index 98b2682..070dc11 100644 --- a/pager.c +++ b/pager.c @@ -78,6 +78,7 @@ void setup_pager(void) argv_array_push(pager_process.env_array, LESS=FRX); if (!getenv(LV)) argv_array_push(pager_process.env_array, LV=-c); + argv_array_push(pager_process.env_array, GIT_PAGER_IN_USE); if (start_command(pager_process)) return; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
