net/http/cgi: fix REMOTE_ADDR, REMOTE_HOST, add REMOTE_PORT

Env vars were incorrectly copying whole value of http.RemoteAddr
to REMOTE_ADDR and REMOTE_HOST. They contained IP:port pair which
instead should only have IP (RFC 3875, other sources).

Module also was not setting REMOTE_PORT variable which become de-facto
standard for passing TCP client port to CGI scripts (Apache mod_cgi,
IIS, and probably others)

Fixes #9861

Change-Id: Ia73e664c48539e3c7db4997d09d957884e98d8a5
Reviewed-on: https://go-review.googlesource.com/4933
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/cgi/host.go b/src/net/http/cgi/host.go
index ec95a972c1a425b860c78d6456f320ec5d748d26..4efbe7abeec001395c6cec764a081299b4a0031f 100644 (file)
--- a/src/net/http/cgi/host.go
+++ b/src/net/http/cgi/host.go
@@ -19,6 +19,7 @@ import (
        "fmt"
        "io"
        "log"
+       "net"
        "net/http"
        "os"
        "os/exec"
@@ -128,11 +129,16 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                "PATH_INFO=" + pathInfo,
                "SCRIPT_NAME=" + root,
                "SCRIPT_FILENAME=" + h.Path,
-               "REMOTE_ADDR=" + req.RemoteAddr,
-               "REMOTE_HOST=" + req.RemoteAddr,
                "SERVER_PORT=" + port,
        }
 
+       if remoteIP, remotePort, err := net.SplitHostPort(req.RemoteAddr); err == nil {
+               env = append(env, "REMOTE_ADDR="+remoteIP, "REMOTE_HOST="+remoteIP, "REMOTE_PORT="+remotePort)
+       } else {
+               // could not parse ip:port, let's use whole RemoteAddr and leave REMOTE_PORT undefined
+               env = append(env, "REMOTE_ADDR="+req.RemoteAddr, "REMOTE_HOST="+req.RemoteAddr)
+       }
+
        if req.TLS != nil {
                env = append(env, "HTTPS=on")
        }

diff --git a/src/net/http/cgi/host_test.go b/src/net/http/cgi/host_test.go
index 8c16e6897f27e15ef2557a909a373781dac341f3..4aa67e4e5f3835881b6af37ffddaac6c9678bb29 100644 (file)
--- a/src/net/http/cgi/host_test.go
+++ b/src/net/http/cgi/host_test.go
@@ -29,7 +29,7 @@ func newRequest(httpreq string) *http.Request {
        if err != nil {
                panic("cgi: bogus http request in test: " + httpreq)
        }
-       req.RemoteAddr = "1.2.3.4"
+       req.RemoteAddr = "1.2.3.4:1234"
        return req
 }
 
@@ -37,7 +37,11 @@ func runCgiTest(t *testing.T, h *Handler, httpreq string, expectedMap map[string
        rw := httptest.NewRecorder()
        req := newRequest(httpreq)
        h.ServeHTTP(rw, req)
+       runResponseChecks(t, rw, expectedMap)
+       return rw
+}
 
+func runResponseChecks(t *testing.T, rw *httptest.ResponseRecorder, expectedMap map[string]string) {
        // Make a map to hold the test map that the CGI returns.
        m := make(map[string]string)
        m["_body"] = rw.Body.String()
@@ -75,7 +79,6 @@ readlines:
                        t.Errorf("for key %q got %q; expected %q", key, got, expected)
                }
        }
-       return rw
 }
 
 var cgiTested, cgiWorks bool
@@ -108,6 +111,7 @@ func TestCGIBasicGet(t *testing.T) {
                "env-QUERY_STRING":      "foo=bar&a=b",
                "env-REMOTE_ADDR":       "1.2.3.4",
                "env-REMOTE_HOST":       "1.2.3.4",
+               "env-REMOTE_PORT":       "1234",
                "env-REQUEST_METHOD":    "GET",
                "env-REQUEST_URI":       "/test.cgi?foo=bar&a=b",
                "env-SCRIPT_FILENAME":   "testdata/test.cgi",
@@ -126,6 +130,39 @@ func TestCGIBasicGet(t *testing.T) {
        }
 }
 
+func TestCGIEnvIPv6(t *testing.T) {
+       check(t)
+       h := &Handler{
+               Path: "testdata/test.cgi",
+               Root: "/test.cgi",
+       }
+       expectedMap := map[string]string{
+               "test":                  "Hello CGI",
+               "param-a":               "b",
+               "param-foo":             "bar",
+               "env-GATEWAY_INTERFACE": "CGI/1.1",
+               "env-HTTP_HOST":         "example.com",
+               "env-PATH_INFO":         "",
+               "env-QUERY_STRING":      "foo=bar&a=b",
+               "env-REMOTE_ADDR":       "2000::3000",
+               "env-REMOTE_HOST":       "2000::3000",
+               "env-REMOTE_PORT":       "12345",
+               "env-REQUEST_METHOD":    "GET",
+               "env-REQUEST_URI":       "/test.cgi?foo=bar&a=b",
+               "env-SCRIPT_FILENAME":   "testdata/test.cgi",
+               "env-SCRIPT_NAME":       "/test.cgi",
+               "env-SERVER_NAME":       "example.com",
+               "env-SERVER_PORT":       "80",
+               "env-SERVER_SOFTWARE":   "go",
+       }
+
+       rw := httptest.NewRecorder()
+       req := newRequest("GET /test.cgi?foo=bar&a=b HTTP/1.0\nHost: example.com\n\n")
+       req.RemoteAddr = "[2000::3000]:12345"
+       h.ServeHTTP(rw, req)
+       runResponseChecks(t, rw, expectedMap)
+}
+
 func TestCGIBasicGetAbsPath(t *testing.T) {
        check(t)
        pwd, err := os.Getwd()
@@ -289,7 +326,7 @@ func TestInternalRedirect(t *testing.T) {
        }
        expectedMap := map[string]string{
                "basepath":   "/foo",
-               "remoteaddr": "1.2.3.4",
+               "remoteaddr": "1.2.3.4:1234",
        }
        runCgiTest(t, h, "GET /test.cgi?loc=/foo HTTP/1.0\nHost: example.com\n\n", expectedMap)
 }

diff --git a/src/net/http/cgi/matryoshka_test.go b/src/net/http/cgi/matryoshka_test.go
index 18c4803e71b55374881674429d8e49d4adef3a9b..244acf1e8c02babf31af4e02f0476da13ebbdc3d 100644 (file)
--- a/src/net/http/cgi/matryoshka_test.go
+++ b/src/net/http/cgi/matryoshka_test.go
@@ -43,6 +43,7 @@ func TestHostingOurselves(t *testing.T) {
                "env-QUERY_STRING":      "foo=bar&a=b",
                "env-REMOTE_ADDR":       "1.2.3.4",
                "env-REMOTE_HOST":       "1.2.3.4",
+               "env-REMOTE_PORT":       "1234",
                "env-REQUEST_METHOD":    "GET",
                "env-REQUEST_URI":       "/test.go?foo=bar&a=b",
                "env-SCRIPT_FILENAME":   os.Args[0],