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>

3 files changed, 49 insertions, 5 deletions

diff --git a/src/net/http/cgi/host.go b/src/net/http/cgi/host.go
index ec95a972c1..4efbe7abee 100644
--- 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 8c16e6897f..4aa67e4e5f 100644
--- 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 18c4803e71..244acf1e8c 100644
--- 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],