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>

Jeff King committed Mar 30, 2012 at 03:01 UTC e32a4581bcbf1cf43cd5069a0d19df07542d612a
3 files changed +37 -13
http-backend.c
+9 -13
@@ -7,6 +7,7 @@
7 #include "run-command.h"
8 #include "string-list.h"
9 #include "url.h"
10 +#include "argv-array.h"
11
12 static const char content_type[] = "Content-Type";
13 static const char content_length[] = "Content-Length";
@@ -317,8 +318,7 @@ static void run_service(const char **argv)
318 const char *encoding = getenv("HTTP_CONTENT_ENCODING");
319 const char *user = getenv("REMOTE_USER");
320 const char *host = getenv("REMOTE_ADDR");
320 - char *env[3];
321 - struct strbuf buf = STRBUF_INIT;
321 + struct argv_array env = ARGV_ARRAY_INIT;
322 int gzipped_request = 0;
323 struct child_process cld;
324
@@ -332,17 +332,15 @@ static void run_service(const char **argv)
332 if (!host || !*host)
333 host = "(none)";
334
335 - memset(&env, 0, sizeof(env));
336 - strbuf_addf(&buf, "GIT_COMMITTER_NAME=%s", user);
337 - env[0] = strbuf_detach(&buf, NULL);
338 -
339 - strbuf_addf(&buf, "GIT_COMMITTER_EMAIL=%s@http.%s", user, host);
340 - env[1] = strbuf_detach(&buf, NULL);
341 - env[2] = NULL;
335 + if (!getenv("GIT_COMMITTER_NAME"))
336 + argv_array_pushf(&env, "GIT_COMMITTER_NAME=%s", user);
337 + if (!getenv("GIT_COMMITTER_EMAIL"))
338 + argv_array_pushf(&env, "GIT_COMMITTER_EMAIL=%s@http.%s",
339 + user, host);
340
341 memset(&cld, 0, sizeof(cld));
342 cld.argv = argv;
345 - cld.env = (const char *const *)env;
343 + cld.env = env.argv;
344 if (gzipped_request)
345 cld.in = -1;
346 cld.git_cmd = 1;
@@ -357,9 +355,7 @@ static void run_service(const char **argv)
355
356 if (finish_command(&cld))
357 exit(1);
360 - free(env[0]);
361 - free(env[1]);
362 - strbuf_release(&buf);
358 + argv_array_clear(&env);
359 }
360
361 static int show_text_ref(const char *name, const unsigned char *sha1,
t/lib-httpd/apache.conf
+7
@@ -52,8 +52,15 @@ Alias /auth/ www/auth/
52 <Location /smart_noexport/>
53 SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
54 </Location>
55 +<Location /smart_custom_env/>
56 + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
57 + SetEnv GIT_HTTP_EXPORT_ALL
58 + SetEnv GIT_COMMITTER_NAME "Custom User"
59 + SetEnv GIT_COMMITTER_EMAIL custom@example.com
60 +</Location>
61 ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/
62 ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
63 +ScriptAlias /smart_custom_env/ ${GIT_EXEC_PATH}/git-http-backend/
64 <Directory ${GIT_EXEC_PATH}>
65 Options None
66 </Directory>
t/t5541-http-push.sh
+21
@@ -30,6 +30,7 @@ test_expect_success 'setup remote repository' '
30 git clone --bare test_repo test_repo.git &&
31 cd test_repo.git &&
32 git config http.receivepack true &&
33 + git config core.logallrefupdates true &&
34 ORIG_HEAD=$(git rev-parse --verify HEAD) &&
35 cd - &&
36 mv test_repo.git "$HTTPD_DOCUMENT_ROOT_PATH"
@@ -222,5 +223,25 @@ test_expect_success TTY 'quiet push' '
223 test_cmp /dev/null output
224 '
225
226 +test_expect_success 'http push gives sane defaults to reflog' '
227 + cd "$ROOT_PATH"/test_repo_clone &&
228 + test_commit reflog-test &&
229 + git push "$HTTPD_URL"/smart/test_repo.git &&
230 + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
231 + log -g -1 --format="%gn <%ge>" >actual &&
232 + echo "anonymous <anonymous@http.127.0.0.1>" >expect &&
233 + test_cmp expect actual
234 +'
235 +
236 +test_expect_success 'http push respects GIT_COMMITTER_* in reflog' '
237 + cd "$ROOT_PATH"/test_repo_clone &&
238 + test_commit custom-reflog-test &&
239 + git push "$HTTPD_URL"/smart_custom_env/test_repo.git &&
240 + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
241 + log -g -1 --format="%gn <%ge>" >actual &&
242 + echo "Custom User <custom@example.com>" >expect &&
243 + test_cmp expect actual
244 +'
245 +
246 stop_httpd
247 test_done