6263: Split git-specific stuff into git_handler.go, add tests for new env vars.

diff --git a/services/arv-git-httpd/auth_handler.go b/services/arv-git-httpd/auth_handler.go
index 39a9098c525b1830a1ffb688ed48d8fd03963725..fccb0c9576864634481a2e69b7237def54b6f0ec 100644 (file)
--- a/services/arv-git-httpd/auth_handler.go
+++ b/services/arv-git-httpd/auth_handler.go
@@ -3,7 +3,6 @@ package main
 import (
        "log"
        "net/http"
-       "net/http/cgi"
        "os"
        "strings"
        "time"
@@ -16,7 +15,7 @@ import (
 var clientPool = arvadosclient.MakeClientPool()
 
 type authHandler struct {
-       handler *cgi.Handler
+       handler http.Handler
 }
 
 func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
@@ -150,7 +149,5 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
        }
        r.URL.Path = rewrittenPath
 
-       handlerCopy := *h.handler
-       handlerCopy.Env = append(handlerCopy.Env, "REMOTE_USER="+r.RemoteAddr) // Should be username
-       handlerCopy.ServeHTTP(&w, r)
+       h.handler.ServeHTTP(&w, r)
 }

diff --git a/services/arv-git-httpd/git_handler.go b/services/arv-git-httpd/git_handler.go
new file mode 100644 (file)
index 0000000..96ab674
--- /dev/null
+++ b/services/arv-git-httpd/git_handler.go
@@ -0,0 +1,59 @@
+package main
+
+import (
+       "log"
+       "net"
+       "net/http"
+       "net/http/cgi"
+)
+
+// gitHandler is an http.Handler that invokes git-http-backend (or
+// whatever backend is configured) via CGI, with appropriate
+// environment variables in place for git-http-backend or
+// gitolite-shell.
+type gitHandler struct {
+       cgi.Handler
+}
+
+func newGitHandler() http.Handler {
+       return &gitHandler{
+               Handler: cgi.Handler{
+                       Path: theConfig.GitCommand,
+                       Dir:  theConfig.Root,
+                       Env: []string{
+                               "GIT_PROJECT_ROOT=" + theConfig.Root,
+                               "GIT_HTTP_EXPORT_ALL=",
+                               "SERVER_ADDR=" + theConfig.Addr,
+                       },
+                       InheritEnv: []string{
+                               "PATH",
+                               // Needed if GitCommand is gitolite-shell:
+                               "GITOLITE_HTTP_HOME",
+                               "GL_BYPASS_ACCESS_CHECKS",
+                       },
+                       Args:       []string{"http-backend"},
+               },
+       }
+}
+
+func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+       remoteHost, remotePort, err := net.SplitHostPort(r.RemoteAddr)
+       if err != nil {
+               log.Printf("Internal error: SplitHostPort(r.RemoteAddr==%q): %s", r.RemoteAddr, err)
+               w.WriteHeader(http.StatusInternalServerError)
+               return
+       }
+
+       // Copy the wrapped cgi.Handler, so these request-specific
+       // variables don't leak into the next request.
+       handlerCopy := h.Handler
+       handlerCopy.Env = append(handlerCopy.Env,
+               // In Go1.5 we can skip this, net/http/cgi will do it for us:
+               "REMOTE_HOST="+remoteHost,
+               "REMOTE_ADDR="+remoteHost,
+               "REMOTE_PORT="+remotePort,
+               // Ideally this would be a real username:
+               "REMOTE_USER="+r.RemoteAddr,
+       )
+       handlerCopy.ServeHTTP(w, r)
+}

diff --git a/services/arv-git-httpd/git_handler_test.go b/services/arv-git-httpd/git_handler_test.go
new file mode 100644 (file)
index 0000000..e24679f
--- /dev/null
+++ b/services/arv-git-httpd/git_handler_test.go
@@ -0,0 +1,56 @@
+package main
+
+import (
+       "net/http"
+       "net/http/httptest"
+       "net/url"
+       "os"
+       "regexp"
+
+       check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&GitHandlerSuite{})
+
+type GitHandlerSuite struct {}
+
+func (s *GitHandlerSuite) TestEnvVars(c *check.C) {
+       u, err := url.Parse("git.zzzzz.arvadosapi.com/test")
+       c.Check(err, check.Equals, nil)
+       resp := httptest.NewRecorder()
+       req := &http.Request{
+               Method: "GET",
+               URL: u,
+               RemoteAddr: "[::1]:12345",
+       }
+       h := newGitHandler()
+       h.(*gitHandler).Path = "/bin/sh"
+       h.(*gitHandler).Args = []string{"-c", "echo HTTP/1.1 200 OK; echo Content-Type: text/plain; echo; env"}
+       os.Setenv("GITOLITE_HTTP_HOME", "/test/ghh")
+       os.Setenv("GL_BYPASS_ACCESS_CHECKS", "yesplease")
+
+       h.ServeHTTP(resp, req)
+
+       c.Check(resp.Code, check.Equals, http.StatusOK)
+       body := resp.Body.String()
+       c.Check(body, check.Matches, `(?ms).*^GITOLITE_HTTP_HOME=/test/ghh$.*`)
+       c.Check(body, check.Matches, `(?ms).*^GL_BYPASS_ACCESS_CHECKS=yesplease$.*`)
+       c.Check(body, check.Matches, `(?ms).*^REMOTE_HOST=::1$.*`)
+       c.Check(body, check.Matches, `(?ms).*^REMOTE_PORT=12345$.*`)
+       c.Check(body, check.Matches, `(?ms).*^SERVER_ADDR=` + regexp.QuoteMeta(theConfig.Addr) + `$.*`)
+}
+
+func (s *GitHandlerSuite) TestCGIError(c *check.C) {
+       u, err := url.Parse("git.zzzzz.arvadosapi.com/test")
+       c.Check(err, check.Equals, nil)
+       resp := httptest.NewRecorder()
+       req := &http.Request{
+               Method: "GET",
+               URL: u,
+               RemoteAddr: "bogus",
+       }
+       h := newGitHandler()
+       h.ServeHTTP(resp, req)
+       c.Check(resp.Code, check.Equals, http.StatusInternalServerError)
+       c.Check(resp.Body.String(), check.Equals, "")
+}

diff --git a/services/arv-git-httpd/server.go b/services/arv-git-httpd/server.go
index e3cd58299bca3be27c59a3df3af607734c0399b9..40e77a812a6ff4c04a524ca76b819555b9e6e69e 100644 (file)
--- a/services/arv-git-httpd/server.go
+++ b/services/arv-git-httpd/server.go
@@ -2,7 +2,6 @@ package main
 
 import (
        "net/http"
-       "net/http/cgi"
 
        "git.curoverse.com/arvados.git/sdk/go/httpserver"
 )
@@ -12,24 +11,8 @@ type server struct {
 }
 
 func (srv *server) Start() error {
-       gitHandler := &cgi.Handler{
-               Path: theConfig.GitCommand,
-               Dir:  theConfig.Root,
-               Env: []string{
-                       "GIT_PROJECT_ROOT=" + theConfig.Root,
-                       "GIT_HTTP_EXPORT_ALL=",
-                       "SERVER_ADDR=" + theConfig.Addr,
-               },
-               InheritEnv: []string{
-                       "PATH",
-                       // Needed if GitCommand is gitolite-shell:
-                       "GITOLITE_HTTP_HOME",
-                       "GL_BYPASS_ACCESS_CHECKS",
-               },
-               Args:       []string{"http-backend"},
-       }
        mux := http.NewServeMux()
-       mux.Handle("/", &authHandler{gitHandler})
+       mux.Handle("/", &authHandler{newGitHandler()})
        srv.Handler = mux
        srv.Addr = theConfig.Addr
        return srv.Server.Start()