RealIP sets r.RemoteAddr without port

[Dirbaio]

The Go documentation states the following on http.Request.RemoteAddr:

The HTTP server in this package sets RemoteAddr to an "IP:port" address before invoking a handler.

Therefore, you could expect that a code like this is correct:

package main

import (
	"fmt"
	"net"
	"net/http"

	"github.com/go-chi/chi"
	"github.com/go-chi/chi/middleware"
)

func main() {
	r := chi.NewMux()
	r.Use(middleware.RealIP)
	r.Get("/test", func(rw http.ResponseWriter, r *http.Request) {
		host, port, err := net.SplitHostPort(r.RemoteAddr)
		if err != nil {
			fmt.Fprintf(rw, "Error: %v\n", err)
		} else {
			fmt.Fprintf(rw, "Host: %s\nPort: %s\n", host, port)
		}
	})
	http.ListenAndServe(":8000", r)
}

However, the RealIP middleware is just copying the X-Forwarded-For value into r.RemoteAddr, which usually does not contain a port, making the code fail:

[dirbaio@mars]$ curl localhost:8000/test
Host: ::1
Port: 40188
[dirbaio@mars]$ curl localhost:8000/test -H 'X-Forwarded-For: 1.2.3.4'
Error: address 1.2.3.4: missing port in address
[dirbaio@mars]$ curl localhost:8000/test -H 'X-Forwarded-For: ::1'
Error: address ::1: too many colons in address

Perhaps RealIP should try to parse X-Forwarded-For for a host:port, and if it isn't, add a port? Maybe 0 to denote the port is unknown, like 1.2.3.4 -> 1.2.3.4:0?

This is particularly frustrating with IPv6 addresses because they have colons but they're not the host:port colon, making it hard to parse the RemoteAddr in user code.

[rof20004]

@Dirbaio

From Go pkg net: https://golang.org/pkg/net/#SplitHostPort

func SplitHostPort(hostport string) (host, port string, err error)

SplitHostPort splits a network address of the form "host:port", "host%zone:port", "[host]:port" or "[host%zone]:port" into host or host%zone and port.

A literal IPv6 address in hostport must be enclosed in square brackets, as in "[::1]:80", "[::1%lo0]:80".

A literal IPv6 address in hostport must be enclosed in square brackets, as in "[::1]:80", "[::1%lo0]:80

Your IPv6 need to follow these recomendations.

[moorereason]

@Dirbaio is correct. The RealIP middleware is breaking the rules here. Three options:

1. Stop using the RealIP middleware since it's fairly trivial.
2. Fork a local copy.
3. Change the RealIP API to accept a port (see Hearbeat middleware as an example).

Maybe changing the RealIP API is just what we need to force a v5.0.0 (#462). :smile:

[Dirbaio]

I'm using this in my code in place of Chi's RealIP. It sets the port to 0. Not pretty but at least the code consuming RemoteAddr doesn't need hacks to accept both ip and ip:port values.

func RealIP(h http.Handler) http.Handler {
	fn := func(w http.ResponseWriter, r *http.Request) {
		if rip := realIP(r); rip != "" {
			r.RemoteAddr = net.JoinHostPort(rip, "0")
		}
		h.ServeHTTP(w, r)
	}

	return http.HandlerFunc(fn)
}

This would be a breaking change, of course.

[pkieltyka]

this is an interesting point. any thoughts on the ideal solution? r.RemoteAddr = net.JoinHostPort(rip, "0") might work, but it provides a confusing port, but at least would be valid to parse

[lrstanley]

Maybe you could take the port from the existing r.RemoteAddr, and essentially just replace the IP?

[pkieltyka]

alternatively, RealIP can change so it doesn't override r.RemoteAddr at all, and instead it will set a "RealIP" on the request context, I think that would be cleaner