From: Meir Fischer Date: Sat, 8 Apr 2017 04:17:01 +0000 (-0400) Subject: net/http/fcgi: expose cgi env vars in request context X-Git-Tag: go1.9beta1~708 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=aaf4682171f1ffabd3673b82cb71fdc3d8e5317e;p=gostls13.git net/http/fcgi: expose cgi env vars in request context 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 53d129c23f..c197489cd7 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -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"}, diff --git a/src/net/http/fcgi/child.go b/src/net/http/fcgi/child.go index 88704245db..30a6b2ce2d 100644 --- a/src/net/http/fcgi/child.go +++ b/src/net/http/fcgi/child.go @@ -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 +} diff --git a/src/net/http/fcgi/fcgi.go b/src/net/http/fcgi/fcgi.go index 5057d70098..8f3449a991 100644 --- a/src/net/http/fcgi/fcgi.go +++ b/src/net/http/fcgi/fcgi.go @@ -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 ( diff --git a/src/net/http/fcgi/fcgi_test.go b/src/net/http/fcgi/fcgi_test.go index b6013bfdd5..e9d2b34023 100644 --- a/src/net/http/fcgi/fcgi_test.go +++ b/src/net/http/fcgi/fcgi_test.go @@ -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 + } +}