]> Cypherpunks repositories - gostls13.git/commitdiff
cgi: improve Location response handling
authorBrad Fitzpatrick <bradfitz@golang.org>
Wed, 27 Apr 2011 21:07:13 +0000 (14:07 -0700)
committerBrad Fitzpatrick <bradfitz@golang.org>
Wed, 27 Apr 2011 21:07:13 +0000 (14:07 -0700)
Add local URI path support, which isn't as fringe
as I originally thought. (it's supported by Apache)

Send an implicit 302 status on redirects (not 200).

Fixes #1597

R=rsc, r
CC=golang-dev
https://golang.org/cl/4442089

src/pkg/http/cgi/host.go
src/pkg/http/cgi/host_test.go
src/pkg/http/cgi/testdata/test.cgi

index 35fbde705a03d258fc0f106590f11cc571afdb8a..136d4e4ee250ee367e1aee6513eed6d9c403b0b1 100644 (file)
@@ -49,6 +49,16 @@ type Handler struct {
        InheritEnv []string    // environment variables to inherit from host, as "key"
        Logger     *log.Logger // optional log for errors or nil to use log.Print
        Args       []string    // optional arguments to pass to child process
+
+       // PathLocationHandler specifies the root http Handler that
+       // should handle internal redirects when the CGI process
+       // returns a Location header value starting with a "/", as
+       // specified in RFC 3875 ยง 6.3.2. This will likely be
+       // http.DefaultServeMux.
+       //
+       // If nil, a CGI response with a local URI path is instead sent
+       // back to the client and not redirected internally.
+       PathLocationHandler http.Handler
 }
 
 func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
@@ -171,13 +181,13 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
        }
 
        linebody, _ := bufio.NewReaderSize(cmd.Stdout, 1024)
-       headers := rw.Header()
-       statusCode := http.StatusOK
+       headers := make(http.Header)
+       statusCode := 0
        for {
                line, isPrefix, err := linebody.ReadLine()
                if isPrefix {
                        rw.WriteHeader(http.StatusInternalServerError)
-                       h.printf("CGI: long header line from subprocess.")
+                       h.printf("cgi: long header line from subprocess.")
                        return
                }
                if err == os.EOF {
@@ -185,7 +195,7 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                }
                if err != nil {
                        rw.WriteHeader(http.StatusInternalServerError)
-                       h.printf("CGI: error reading headers: %v", err)
+                       h.printf("cgi: error reading headers: %v", err)
                        return
                }
                if len(line) == 0 {
@@ -193,7 +203,7 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                }
                parts := strings.Split(string(line), ":", 2)
                if len(parts) < 2 {
-                       h.printf("CGI: bogus header line: %s", string(line))
+                       h.printf("cgi: bogus header line: %s", string(line))
                        continue
                }
                header, val := parts[0], parts[1]
@@ -202,13 +212,13 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                switch {
                case header == "Status":
                        if len(val) < 3 {
-                               h.printf("CGI: bogus status (short): %q", val)
+                               h.printf("cgi: bogus status (short): %q", val)
                                return
                        }
                        code, err := strconv.Atoi(val[0:3])
                        if err != nil {
-                               h.printf("CGI: bogus status: %q", val)
-                               h.printf("CGI: line was %q", line)
+                               h.printf("cgi: bogus status: %q", val)
+                               h.printf("cgi: line was %q", line)
                                return
                        }
                        statusCode = code
@@ -216,11 +226,35 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                        headers.Add(header, val)
                }
        }
+
+       if loc := headers.Get("Location"); loc != "" {
+               if strings.HasPrefix(loc, "/") && h.PathLocationHandler != nil {
+                       h.handleInternalRedirect(rw, req, loc)
+                       return
+               }
+               if statusCode == 0 {
+                       statusCode = http.StatusFound
+               }
+       }
+
+       if statusCode == 0 {
+               statusCode = http.StatusOK
+       }
+
+       // Copy headers to rw's headers, after we've decided not to
+       // go into handleInternalRedirect, which won't want its rw
+       // headers to have been touched.
+       for k, vv := range headers {
+               for _, v := range vv {
+                       rw.Header().Add(k, v)
+               }
+       }
+
        rw.WriteHeader(statusCode)
 
        _, err = io.Copy(rw, linebody)
        if err != nil {
-               h.printf("CGI: copy error: %v", err)
+               h.printf("cgi: copy error: %v", err)
        }
 }
 
@@ -232,6 +266,37 @@ func (h *Handler) printf(format string, v ...interface{}) {
        }
 }
 
+func (h *Handler) handleInternalRedirect(rw http.ResponseWriter, req *http.Request, path string) {
+       url, err := req.URL.ParseURL(path)
+       if err != nil {
+               rw.WriteHeader(http.StatusInternalServerError)
+               h.printf("cgi: error resolving local URI path %q: %v", path, err)
+               return
+       }
+       // TODO: RFC 3875 isn't clear if only GET is supported, but it
+       // suggests so: "Note that any message-body attached to the
+       // request (such as for a POST request) may not be available
+       // to the resource that is the target of the redirect."  We
+       // should do some tests against Apache to see how it handles
+       // POST, HEAD, etc. Does the internal redirect get the same
+       // method or just GET? What about incoming headers?
+       // (e.g. Cookies) Which headers, if any, are copied into the
+       // second request?
+       newReq := &http.Request{
+               Method:     "GET",
+               URL:        url,
+               RawURL:     path,
+               Proto:      "HTTP/1.1",
+               ProtoMajor: 1,
+               ProtoMinor: 1,
+               Header:     make(http.Header),
+               Host:       url.Host,
+               RemoteAddr: req.RemoteAddr,
+               TLS:        req.TLS,
+       }
+       h.PathLocationHandler.ServeHTTP(rw, newReq)
+}
+
 func upperCaseAndUnderscore(rune int) int {
        switch {
        case rune >= 'a' && rune <= 'z':
index e8084b1134e3e8f8235933b106238c525e376e22..9ac085f2f3aba7542d7d1d7d01b9f7609abe6965 100644 (file)
@@ -271,3 +271,40 @@ Transfer-Encoding: chunked
                        expected, got)
        }
 }
+
+func TestRedirect(t *testing.T) {
+       if skipTest(t) {
+               return
+       }
+       h := &Handler{
+               Path: "testdata/test.cgi",
+               Root: "/test.cgi",
+       }
+       rec := runCgiTest(t, h, "GET /test.cgi?loc=http://foo.com/ HTTP/1.0\nHost: example.com\n\n", nil)
+       if e, g := 302, rec.Code; e != g {
+               t.Errorf("expected status code %d; got %d", e, g)
+       }
+       if e, g := "http://foo.com/", rec.Header().Get("Location"); e != g {
+               t.Errorf("expected Location header of %q; got %q", e, g)
+       }
+}
+
+func TestInternalRedirect(t *testing.T) {
+       if skipTest(t) {
+               return
+       }
+       baseHandler := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+               fmt.Fprintf(rw, "basepath=%s\n", req.URL.Path)
+               fmt.Fprintf(rw, "remoteaddr=%s\n", req.RemoteAddr)
+       })
+       h := &Handler{
+               Path:                "testdata/test.cgi",
+               Root:                "/test.cgi",
+               PathLocationHandler: baseHandler,
+       }
+       expectedMap := map[string]string{
+               "basepath":   "/foo",
+               "remoteaddr": "1.2.3.4",
+       }
+       runCgiTest(t, h, "GET /test.cgi?loc=/foo HTTP/1.0\nHost: example.com\n\n", expectedMap)
+}
index 253589eed96d0c8385fb07c7a7c4e2a9efc00f55..a1b2ff893dc5579144ba82f196c8a668ddd81795 100755 (executable)
@@ -11,6 +11,11 @@ use CGI;
 my $q = CGI->new;
 my $params = $q->Vars;
 
+if ($params->{"loc"}) {
+    print "Location: $params->{loc}\r\n\r\n";
+    exit(0);
+}
+
 my $NL = "\r\n";
 $NL = "\n" if $params->{mode} eq "NL";