]> Cypherpunks repositories - gostls13.git/commitdiff
net/http/fcgi: expose cgi env vars in request context
authorMeir Fischer <meirfischer@gmail.com>
Sat, 8 Apr 2017 04:17:01 +0000 (00:17 -0400)
committerBrad Fitzpatrick <bradfitz@golang.org>
Thu, 13 Apr 2017 04:38:23 +0000 (04:38 +0000)
The current interface can't access all environment
variables directly or via cgi.RequestFromMap, which
only reads variables on its "white list" to be set on
the http.Request it returns. If an fcgi variable is
not on the "white list" - e.g. REMOTE_USER - the old
code has no access to its value.

This passes variables in the Request context that aren't
used to add data to the Request itself and adds a method
that parses those env vars from the Request's context.

Fixes #16546

Change-Id: Ibf933a768b677ece1bb93d7bf99a14cef36ec671
Reviewed-on: https://go-review.googlesource.com/40012
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
src/go/build/deps_test.go
src/net/http/fcgi/child.go
src/net/http/fcgi/fcgi.go
src/net/http/fcgi/fcgi_test.go

index 53d129c23fae9b4fb40785c79cc26e21028ec976..c197489cd74acbcc2823c8abdcd593a26f41ee06 100644 (file)
@@ -410,7 +410,7 @@ var pkgDeps = map[string][]string{
        "expvar":             {"L4", "OS", "encoding/json", "net/http"},
        "net/http/cgi":       {"L4", "NET", "OS", "crypto/tls", "net/http", "regexp"},
        "net/http/cookiejar": {"L4", "NET", "net/http"},
-       "net/http/fcgi":      {"L4", "NET", "OS", "net/http", "net/http/cgi"},
+       "net/http/fcgi":      {"L4", "NET", "OS", "context", "net/http", "net/http/cgi"},
        "net/http/httptest":  {"L4", "NET", "OS", "crypto/tls", "flag", "net/http", "net/http/internal", "crypto/x509"},
        "net/http/httputil":  {"L4", "NET", "OS", "context", "net/http", "net/http/internal"},
        "net/http/pprof":     {"L4", "OS", "html/template", "net/http", "runtime/pprof", "runtime/trace"},
index 88704245db8742325c426fcad49ecd37e1c8606e..30a6b2ce2dfe8335efce24b38c274bc0476fd8ed 100644 (file)
@@ -7,6 +7,7 @@ package fcgi
 // This file implements FastCGI from the perspective of a child process.
 
 import (
+       "context"
        "errors"
        "fmt"
        "io"
@@ -31,6 +32,10 @@ type request struct {
        keepConn  bool
 }
 
+// envVarsContextKey uniquely identifies a mapping of CGI
+// environment variables to their values in a request context
+type envVarsContextKey struct{}
+
 func newRequest(reqId uint16, flags uint8) *request {
        r := &request{
                reqId:    reqId,
@@ -259,6 +264,18 @@ func (c *child) handleRecord(rec *record) error {
        }
 }
 
+// filterOutUsedEnvVars returns a new map of env vars without the
+// variables in the given envVars map that are read for creating each http.Request
+func filterOutUsedEnvVars(envVars map[string]string) map[string]string {
+       withoutUsedEnvVars := make(map[string]string)
+       for k, v := range envVars {
+               if addFastCGIEnvToContext(k) {
+                       withoutUsedEnvVars[k] = v
+               }
+       }
+       return withoutUsedEnvVars
+}
+
 func (c *child) serveRequest(req *request, body io.ReadCloser) {
        r := newResponse(c, req)
        httpReq, err := cgi.RequestFromMap(req.params)
@@ -268,6 +285,9 @@ func (c *child) serveRequest(req *request, body io.ReadCloser) {
                c.conn.writeRecord(typeStderr, req.reqId, []byte(err.Error()))
        } else {
                httpReq.Body = body
+               withoutUsedEnvVars := filterOutUsedEnvVars(req.params)
+               envVarCtx := context.WithValue(httpReq.Context(), envVarsContextKey{}, withoutUsedEnvVars)
+               httpReq = httpReq.WithContext(envVarCtx)
                c.handler.ServeHTTP(r, httpReq)
        }
        r.Close()
@@ -329,3 +349,39 @@ func Serve(l net.Listener, handler http.Handler) error {
                go c.serve()
        }
 }
+
+// ProcessEnv returns FastCGI environment variables associated with the request r
+// for which no effort was made to be included in the request itself - the data
+// is hidden in the request's context. As an example, if REMOTE_USER is set for a
+// request, it will not be found anywhere in r, but it will be included in
+// ProcessEnv's response (via r's context).
+func ProcessEnv(r *http.Request) map[string]string {
+       env, _ := r.Context().Value(envVarsContextKey{}).(map[string]string)
+       return env
+}
+
+// addFastCGIEnvToContext reports whether to include the FastCGI environment variable s
+// in the http.Request.Context, accessible via ProcessEnv.
+func addFastCGIEnvToContext(s string) bool {
+       // Exclude things supported by net/http natively:
+       switch s {
+       case "CONTENT_LENGTH", "CONTENT_TYPE", "HTTPS",
+               "PATH_INFO", "QUERY_STRING", "REMOTE_ADDR",
+               "REMOTE_HOST", "REMOTE_PORT", "REQUEST_METHOD",
+               "REQUEST_URI", "SCRIPT_NAME", "SERVER_PROTOCOL":
+               return false
+       }
+       if strings.HasPrefix(s, "HTTP_") {
+               return false
+       }
+       // Explicitly include FastCGI-specific things.
+       // This list is redundant with the default "return true" below.
+       // Consider this documentation of the sorts of things we expect
+       // to maybe see.
+       switch s {
+       case "REMOTE_USER":
+               return true
+       }
+       // Unknown, so include it to be safe.
+       return true
+}
index 5057d700981bad46c66eb2822820df726e0065a2..8f3449a991a863097e3a62a6028f45472c9a3a4d 100644 (file)
@@ -24,7 +24,7 @@ import (
 )
 
 // recType is a record type, as defined by
-// http://www.fastcgi.com/devkit/doc/fcgi-spec.html#S8
+// https://web.archive.org/web/20150420080736/http://www.fastcgi.com/drupal/node/6?q=node/22#S8
 type recType uint8
 
 const (
index b6013bfdd519362f71e91f9c79d67991dc4a3f9e..e9d2b34023c81a392c6b1662eaff5a162f20cbfb 100644 (file)
@@ -278,3 +278,69 @@ func TestMalformedParams(t *testing.T) {
        c := newChild(rw, http.DefaultServeMux)
        c.serve()
 }
+
+// a series of FastCGI records that start and end a request
+var streamFullRequestStdin = bytes.Join([][]byte{
+       // set up request
+       makeRecord(typeBeginRequest, 1,
+               []byte{0, byte(roleResponder), 0, 0, 0, 0, 0, 0}),
+       // add required parameters
+       makeRecord(typeParams, 1, nameValuePair11("REQUEST_METHOD", "GET")),
+       makeRecord(typeParams, 1, nameValuePair11("SERVER_PROTOCOL", "HTTP/1.1")),
+       // set optional parameters
+       makeRecord(typeParams, 1, nameValuePair11("REMOTE_USER", "jane.doe")),
+       makeRecord(typeParams, 1, nameValuePair11("QUERY_STRING", "/foo/bar")),
+       makeRecord(typeParams, 1, nil),
+       // begin sending body of request
+       makeRecord(typeStdin, 1, []byte("0123456789abcdef")),
+       // end request
+       makeRecord(typeEndRequest, 1, nil),
+},
+       nil)
+
+var envVarTests = []struct {
+       input               []byte
+       envVar              string
+       expectedVal         string
+       expectedFilteredOut bool
+}{
+       {
+               streamFullRequestStdin,
+               "REMOTE_USER",
+               "jane.doe",
+               false,
+       },
+       {
+               streamFullRequestStdin,
+               "QUERY_STRING",
+               "",
+               true,
+       },
+}
+
+// Test that environment variables set for a request can be
+// read by a handler. Ensures that variables not set will not
+// be exposed to a handler.
+func TestChildServeReadsEnvVars(t *testing.T) {
+       for _, tt := range envVarTests {
+               input := make([]byte, len(tt.input))
+               copy(input, tt.input)
+               rc := nopWriteCloser{bytes.NewBuffer(input)}
+               done := make(chan bool)
+               c := newChild(rc, http.HandlerFunc(func(
+                       w http.ResponseWriter,
+                       r *http.Request,
+               ) {
+                       env := ProcessEnv(r)
+                       if _, ok := env[tt.envVar]; ok && tt.expectedFilteredOut {
+                               t.Errorf("Expected environment variable %s to not be set, but set to %s",
+                                       tt.envVar, env[tt.envVar])
+                       } else if env[tt.envVar] != tt.expectedVal {
+                               t.Errorf("Expected %s, got %s", tt.expectedVal, env[tt.envVar])
+                       }
+                       done <- true
+               }))
+               go c.serve()
+               <-done
+       }
+}