diff --git a/context/http.go b/context/http.go index 357f0dc3..01600c35 100644 --- a/context/http.go +++ b/context/http.go @@ -17,11 +17,34 @@ var ( ErrNoRequestContext = errors.New("no http request in context") ) +// http.Request.RemoteAddr is not the originating IP if the request +// came through a reverse proxy. +// X-Forwarded-For and X-Real-IP provide this information with decreased +// support, so check in that order. There may be multiple 'X-Forwarded-For' +// headers, but go maps don't retain order so just pick one. +func RemoteAddr(r *http.Request) string { + remoteAddr := r.RemoteAddr + + if prior, ok := r.Header["X-Forwarded-For"]; ok { + proxies := strings.Split(prior[0], ",") + if len(proxies) > 0 { + remoteAddr = strings.Trim(proxies[0], " ") + } + } else if realip, ok := r.Header["X-Real-Ip"]; ok { + if len(realip) > 0 { + remoteAddr = realip[0] + } + } + + return remoteAddr +} + // WithRequest places the request on the context. The context of the request // is assigned a unique id, available at "http.request.id". The request itself // is available at "http.request". Other common attributes are available under // the prefix "http.request.". If a request is already present on the context, // this method will panic. + func WithRequest(ctx context.Context, r *http.Request) context.Context { if ctx.Value("http.request") != nil { // NOTE(stevvooe): This needs to be considered a programming error. It @@ -147,7 +170,7 @@ func (ctx *httpRequestContext) Value(key interface{}) interface{} { case "uri": return ctx.r.RequestURI case "remoteaddr": - return ctx.r.RemoteAddr + return RemoteAddr(ctx.r) case "method": return ctx.r.Method case "host": diff --git a/context/http_test.go b/context/http_test.go index df3734e8..1fee1761 100644 --- a/context/http_test.go +++ b/context/http_test.go @@ -2,6 +2,9 @@ package context import ( "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" "reflect" "testing" "time" @@ -205,3 +208,47 @@ func TestWithVars(t *testing.T) { } } } + +// SingleHostReverseProxy will insert an X-Forwarded-For header, and can be used to test +// RemoteAddr(). A fake RemoteAddr cannot be set on the HTTP request - it is overwritten +// at the transport layer to 127.0.0.1: . However, as the X-Forwarded-For header +// just contains the IP address, it is different enough for testing. +func TestRemoteAddr(t *testing.T) { + expectedRemote := "127.0.0.1" + var actualRemote string + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + + if r.RemoteAddr == expectedRemote { + t.Errorf("Unexpected matching remote addresses") + } + actualRemote = RemoteAddr(r) + + if expectedRemote != actualRemote { + t.Errorf("Mismatching remote hosts: %v != %v", expectedRemote, actualRemote) + } + + w.WriteHeader(200) + })) + + defer backend.Close() + backendURL, err := url.Parse(backend.URL) + if err != nil { + t.Fatal(err) + } + + proxy := httputil.NewSingleHostReverseProxy(backendURL) + frontend := httptest.NewServer(proxy) + defer frontend.Close() + + getReq, err := http.NewRequest("GET", frontend.URL, nil) + if err != nil { + t.Fatal(err) + } + + _, err = http.DefaultClient.Do(getReq) + if err != nil { + t.Fatal(err) + } + +} diff --git a/notifications/bridge.go b/notifications/bridge.go index 21d2105d..48a2bd5b 100644 --- a/notifications/bridge.go +++ b/notifications/bridge.go @@ -6,6 +6,7 @@ import ( "code.google.com/p/go-uuid/uuid" "github.com/docker/distribution" + "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" ) @@ -42,10 +43,11 @@ func NewBridge(ub URLBuilder, source SourceRecord, actor ActorRecord, request Re // NewRequestRecord builds a RequestRecord for use in NewBridge from an // http.Request, associating it with a request id. + func NewRequestRecord(id string, r *http.Request) RequestRecord { return RequestRecord{ ID: id, - Addr: r.RemoteAddr, + Addr: context.RemoteAddr(r), Host: r.Host, Method: r.Method, UserAgent: r.UserAgent(),