From b477a79c4e67e90e828cdf3e82fad5bac644a85c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 27 Apr 2011 14:07:13 -0700 Subject: cgi: improve Location response handling 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 | 83 +++++++++++++++++++++++++++++++++----- src/pkg/http/cgi/host_test.go | 37 +++++++++++++++++ src/pkg/http/cgi/testdata/test.cgi | 5 +++ 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/src/pkg/http/cgi/host.go b/src/pkg/http/cgi/host.go index 35fbde705a..136d4e4ee2 100644 --- a/src/pkg/http/cgi/host.go +++ b/src/pkg/http/cgi/host.go @@ -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': diff --git a/src/pkg/http/cgi/host_test.go b/src/pkg/http/cgi/host_test.go index e8084b1134..9ac085f2f3 100644 --- a/src/pkg/http/cgi/host_test.go +++ b/src/pkg/http/cgi/host_test.go @@ -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) +} diff --git a/src/pkg/http/cgi/testdata/test.cgi b/src/pkg/http/cgi/testdata/test.cgi index 253589eed9..a1b2ff893d 100755 --- a/src/pkg/http/cgi/testdata/test.cgi +++ b/src/pkg/http/cgi/testdata/test.cgi @@ -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"; -- cgit v1.2.3-54-g00ecf