http-backend: respect existing GIT_COMMITTER_* variables

The http-backend program sets default GIT_COMMITTER_NAME and
GIT_COMMITTER_EMAIL variables based on the REMOTE_USER and
REMOTE_ADDR variables provided by the webserver. However, it
unconditionally overwrites any existing GIT_COMMITTER
variables, which may have been customized by site-specific
code in the webserver (or in a script wrapping http-backend).

Let's leave those variables intact if they already exist,
assuming that any such configuration was intentional. There
is a slight chance of a regression if somebody has set
GIT_COMMITTER_* for the entire webserver, not intending it
to leak through http-backend. We could protect against this
by passing the information in alternate variables.  However,
it seems unlikely that anyone will care about that
regression, and there is value in the simplicity of using
the common variable names that are used elsewhere in git.

While we're tweaking the environment-handling in
http-backend, let's switch it to use argv_array to handle
the list of variables. That makes the memory management much
simpler.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http-backend.c b/http-backend.c
index 869d515383b5fc9c562ea7987b1cd485cce12032..f50e77fb2890207bd7385b59bdd561133991c910 100644 (file)
--- a/http-backend.c
+++ b/http-backend.c
@@ -7,6 +7,7 @@
 #include "run-command.h"
 #include "string-list.h"
 #include "url.h"
+#include "argv-array.h"
 
 static const char content_type[] = "Content-Type";
 static const char content_length[] = "Content-Length";
@@ -317,8 +318,7 @@ static void run_service(const char **argv)
        const char *encoding = getenv("HTTP_CONTENT_ENCODING");
        const char *user = getenv("REMOTE_USER");
        const char *host = getenv("REMOTE_ADDR");
-       char *env[3];
-       struct strbuf buf = STRBUF_INIT;
+       struct argv_array env = ARGV_ARRAY_INIT;
        int gzipped_request = 0;
        struct child_process cld;
 
@@ -332,17 +332,15 @@ static void run_service(const char **argv)
        if (!host || !*host)
                host = "(none)";
 
-       memset(&env, 0, sizeof(env));
-       strbuf_addf(&buf, "GIT_COMMITTER_NAME=%s", user);
-       env[0] = strbuf_detach(&buf, NULL);
-
-       strbuf_addf(&buf, "GIT_COMMITTER_EMAIL=%s@http.%s", user, host);
-       env[1] = strbuf_detach(&buf, NULL);
-       env[2] = NULL;
+       if (!getenv("GIT_COMMITTER_NAME"))
+               argv_array_pushf(&env, "GIT_COMMITTER_NAME=%s", user);
+       if (!getenv("GIT_COMMITTER_EMAIL"))
+               argv_array_pushf(&env, "GIT_COMMITTER_EMAIL=%s@http.%s",
+                                user, host);
 
        memset(&cld, 0, sizeof(cld));
        cld.argv = argv;
-       cld.env = (const char *const *)env;
+       cld.env = env.argv;
        if (gzipped_request)
                cld.in = -1;
        cld.git_cmd = 1;
@@ -357,9 +355,7 @@ static void run_service(const char **argv)
 
        if (finish_command(&cld))
                exit(1);
-       free(env[0]);
-       free(env[1]);
-       strbuf_release(&buf);
+       argv_array_clear(&env);
 }
 
 static int show_text_ref(const char *name, const unsigned char *sha1,

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 3c12b05d60849b4f3063527338140c717b720c5d..de3762e24762692733f7a42f58b139e279b29f3d 100644 (file)
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -52,8 +52,15 @@ Alias /auth/ www/auth/
 <Location /smart_noexport/>
        SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
 </Location>
+<Location /smart_custom_env/>
+       SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+       SetEnv GIT_HTTP_EXPORT_ALL
+       SetEnv GIT_COMMITTER_NAME "Custom User"
+       SetEnv GIT_COMMITTER_EMAIL custom@example.com
+</Location>
 ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/
 ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
+ScriptAlias /smart_custom_env/ ${GIT_EXEC_PATH}/git-http-backend/
 <Directory ${GIT_EXEC_PATH}>
        Options None
 </Directory>

diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index cc6f081711002b42bcf6b2cb26287dcc56852a06..d7964c7eb523ce95944d559e6123df4c2b6d3934 100755 (executable)
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -30,6 +30,7 @@ test_expect_success 'setup remote repository' '
        git clone --bare test_repo test_repo.git &&
        cd test_repo.git &&
        git config http.receivepack true &&
+       git config core.logallrefupdates true &&
        ORIG_HEAD=$(git rev-parse --verify HEAD) &&
        cd - &&
        mv test_repo.git "$HTTPD_DOCUMENT_ROOT_PATH"
@@ -222,5 +223,25 @@ test_expect_success TTY 'quiet push' '
        test_cmp /dev/null output
 '
 
+test_expect_success 'http push gives sane defaults to reflog' '
+       cd "$ROOT_PATH"/test_repo_clone &&
+       test_commit reflog-test &&
+       git push "$HTTPD_URL"/smart/test_repo.git &&
+       git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
+               log -g -1 --format="%gn <%ge>" >actual &&
+       echo "anonymous <anonymous@http.127.0.0.1>" >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'http push respects GIT_COMMITTER_* in reflog' '
+       cd "$ROOT_PATH"/test_repo_clone &&
+       test_commit custom-reflog-test &&
+       git push "$HTTPD_URL"/smart_custom_env/test_repo.git &&
+       git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
+               log -g -1 --format="%gn <%ge>" >actual &&
+       echo "Custom User <custom@example.com>" >expect &&
+       test_cmp expect actual
+'
+
 stop_httpd
 test_done