[PATCH] config --get --path: check for unset $HOME

[PATCH] config --get --path: check for unset $HOME

[Jonathan Nieder]

If $HOME is unset (as in some automated build situations),
currently

	git config --path path.home "~"
	git config --path --get path.home

segfaults.  Error out with

	Failed to expand user dir in: '~/'

instead.

Reported-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Julien Cristau wrote:

> * expecting success: 
>         git config --get --path path.home > result &&
>         git config --get --path path.normal >> result &&
>         git config --get --path path.trailingtilde >> result &&
>         test_cmp expect result
> 
> Segmentation fault

Good catch.  

 path.c                 |    2 ++
 t/t1300-repo-config.sh |   24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/path.c b/path.c
index b4c8d91..6b23023 100644
--- a/path.c
+++ b/path.c
@@ -316,6 +316,8 @@ char *expand_user_path(const char *path)
 		size_t username_len = first_slash - username;
 		if (username_len == 0) {
 			const char *home = getenv("HOME");
+			if (!home)
+				goto return_null;
 			strbuf_add(&user_path, home, strlen(home));
 		} else {
 			struct passwd *pw = getpw_str(username, username_len);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index f11f98c..7ddab5f 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -707,19 +707,41 @@ test_expect_success 'set --path' '
 	git config --path path.trailingtilde "foo~" &&
 	test_cmp expect .git/config'
 
+if test "${HOME+set}"
+then
+	test_set_prereq HOMEVAR
+fi
+
 cat >expect <<EOF
 $HOME/
 /dev/null
 foo~
 EOF
 
-test_expect_success 'get --path' '
+test_expect_success HOMEVAR 'get --path' '
 	git config --get --path path.home > result &&
 	git config --get --path path.normal >> result &&
 	git config --get --path path.trailingtilde >> result &&
 	test_cmp expect result
 '
 
+cat >expect <<\EOF
+/dev/null
+foo~
+EOF
+
+test_expect_success 'get --path copes with unset $HOME' '
+	(
+		unset HOME;
+		test_must_fail git config --get --path path.home \
+			>result 2>msg &&
+		git config --get --path path.normal >>result &&
+		git config --get --path path.trailingtilde >>result
+	) &&
+	grep "[Ff]ailed to expand.*~/" msg &&
+	test_cmp expect result
+'
+
 rm .git/config
 
 git config quote.leading " test"
-- 
1.7.1

Re: [PATCH] config --get --path: check for unset $HOME

[Julien Cristau]

On Mon, Jul 26, 2010 at 10:06:51 -0500, Jonathan Nieder wrote:

> If $HOME is unset (as in some automated build situations),
> currently
> 
> 	git config --path path.home "~"
> 	git config --path --get path.home
> 
> segfaults.  Error out with
> 
> 	Failed to expand user dir in: '~/'
> 
> instead.
> 
> Reported-by: Julien Cristau <jcristau@debian.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Tested-by: Julien Cristau <jcristau@debian.org>

Thanks!

Cheers,
Julien

Re: [PATCH] config --get --path: check for unset $HOME

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 26, 2010 at 15:06, Jonathan Nieder <jrnieder@gmail.com> wrote:
>                size_t username_len = first_slash - username;
>                if (username_len == 0) {
>                        const char *home = getenv("HOME");
> +                       if (!home)
> +                               goto return_null;
>                        strbuf_add(&user_path, home, strlen(home));
>                } else {
>                        struct passwd *pw = getpw_str(username, username_len);

Should we maybe fall back on checking pw_dir in getpwuid() if $HOME isn't set?

Re: [PATCH] config --get --path: check for unset $HOME

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Should we maybe fall back on checking pw_dir in getpwuid() if $HOME isn't set?

Maybe.  Why?  On Unix I suspect it is such a rare case as to not
justify complicating the rules, but I do not know if it would help on
other platforms.

Jonathan

Re: [PATCH] config --get --path: check for unset $HOME

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 26, 2010 at 17:08, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ęvar Arnfjörš Bjarmason wrote:
>
>> Should we maybe fall back on checking pw_dir in getpwuid() if $HOME isn't set?
>
> Maybe.  Why?  On Unix I suspect it is such a rare case as to not
> justify complicating the rules, but I do not know if it would help on
> other platforms.

I don't know if this is worth it, but in the Perl world this is a
fairly common idiom:

    my $home = $ENV{HOME} || (getpwuid($<))[7];

Sometimes you're executing under some buggy wrapper that's scrubbing
your environment. I recall running into this issue e.g. on some odd
Solaris webserver, which was executing CGI scripts under my user, but
$HOME wasn't set.