Rewrite RealIP() to avoid returning an empty string

11 files changed, 117 insertions, 255 deletions

diff --git a/Gopkg.lock b/Gopkg.lock
index 8cfabcf..398956e 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -71,12 +71,6 @@
 
 [[projects]]
   branch = "master"
-  name = "github.com/tomasen/realip"
-  packages = ["."]
-  revision = "b5850897b7b539a1c9f22cdaa3b547d1bd453db8"
-
-[[projects]]
-  branch = "master"
   name = "golang.org/x/crypto"
   packages = [
     "acme",
@@ -158,6 +152,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "0cb64ea4a8054f26b80103c60380d800d74bdd2faec4ef53453874922e7de748"
+  inputs-digest = "f030af166f90ac96d133b7ddf9f1f80785508462cac8cae4c65abbdb7c20474b"
   solver-name = "gps-cdcl"
   solver-version = 1
diff --git a/Gopkg.toml b/Gopkg.toml
index b8ae7d7..42d2567 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -47,10 +47,6 @@
 
 [[constraint]]
   branch = "master"
-  name = "github.com/tomasen/realip"
-
-[[constraint]]
-  branch = "master"
   name = "golang.org/x/crypto"
 
 [[constraint]]
diff --git a/http/request/request.go b/http/request/request.go
index da79a3f..1de6f73 100644
--- a/http/request/request.go
+++ b/http/request/request.go
@@ -6,8 +6,10 @@ package request
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"strconv"
+	"strings"
 
 	"github.com/gorilla/mux"
 )
@@ -88,3 +90,30 @@ func HasQueryParam(r *http.Request, param string) bool {
 	_, ok := values[param]
 	return ok
 }
+
+// RealIP returns client's real IP address.
+func RealIP(r *http.Request) string {
+	headers := []string{"X-Forwarded-For", "X-Real-Ip"}
+	for _, header := range headers {
+		value := r.Header.Get(header)
+
+		if value != "" {
+			addresses := strings.Split(value, ",")
+			address := strings.TrimSpace(addresses[0])
+
+			if net.ParseIP(address) != nil {
+				return address
+			}
+		}
+	}
+
+	// Fallback to TCP/IP source IP address.
+	var remoteIP string
+	if strings.ContainsRune(r.RemoteAddr, ':') {
+		remoteIP, _, _ = net.SplitHostPort(r.RemoteAddr)
+	} else {
+		remoteIP = r.RemoteAddr
+	}
+
+	return remoteIP
+}
diff --git a/http/request/request_test.go b/http/request/request_test.go
new file mode 100644
index 0000000..a4aaabf
--- /dev/null
+++ b/http/request/request_test.go
@@ -0,0 +1,82 @@
+// Copyright 2018 Frédéric Guillot. All rights reserved.
+// Use of this source code is governed by the Apache 2.0
+// license that can be found in the LICENSE file.
+
+package request
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestRealIPWithoutHeaders(t *testing.T) {
+	r := &http.Request{RemoteAddr: "192.168.0.1:4242"}
+	if ip := RealIP(r); ip != "192.168.0.1" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+
+	r = &http.Request{RemoteAddr: "192.168.0.1"}
+	if ip := RealIP(r); ip != "192.168.0.1" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+}
+
+func TestRealIPWithXFFHeader(t *testing.T) {
+	// Test with multiple IPv4 addresses.
+	headers := http.Header{}
+	headers.Set("X-Forwarded-For", "203.0.113.195, 70.41.3.18, 150.172.238.178")
+	r := &http.Request{RemoteAddr: "192.168.0.1:4242", Header: headers}
+
+	if ip := RealIP(r); ip != "203.0.113.195" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+
+	// Test with single IPv6 address.
+	headers = http.Header{}
+	headers.Set("X-Forwarded-For", "2001:db8:85a3:8d3:1319:8a2e:370:7348")
+	r = &http.Request{RemoteAddr: "192.168.0.1:4242", Header: headers}
+
+	if ip := RealIP(r); ip != "2001:db8:85a3:8d3:1319:8a2e:370:7348" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+
+	// Test with single IPv4 address.
+	headers = http.Header{}
+	headers.Set("X-Forwarded-For", "70.41.3.18")
+	r = &http.Request{RemoteAddr: "192.168.0.1:4242", Header: headers}
+
+	if ip := RealIP(r); ip != "70.41.3.18" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+
+	// Test with invalid IP address.
+	headers = http.Header{}
+	headers.Set("X-Forwarded-For", "fake IP")
+	r = &http.Request{RemoteAddr: "192.168.0.1:4242", Header: headers}
+
+	if ip := RealIP(r); ip != "192.168.0.1" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+}
+
+func TestRealIPWithXRealIPHeader(t *testing.T) {
+	headers := http.Header{}
+	headers.Set("X-Real-Ip", "192.168.122.1")
+	r := &http.Request{RemoteAddr: "192.168.0.1:4242", Header: headers}
+
+	if ip := RealIP(r); ip != "192.168.122.1" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+}
+
+func TestRealIPWithBothHeaders(t *testing.T) {
+	headers := http.Header{}
+	headers.Set("X-Forwarded-For", "203.0.113.195, 70.41.3.18, 150.172.238.178")
+	headers.Set("X-Real-Ip", "192.168.122.1")
+
+	r := &http.Request{RemoteAddr: "192.168.0.1:4242", Header: headers}
+
+	if ip := RealIP(r); ip != "203.0.113.195" {
+		t.Fatalf(`Unexpected result, got: %q`, ip)
+	}
+}
diff --git a/middleware/logging.go b/middleware/logging.go
index a6c141b..48158b4 100644
--- a/middleware/logging.go
+++ b/middleware/logging.go
@@ -7,15 +7,14 @@ package middleware
 import (
 	"net/http"
 
+	"github.com/miniflux/miniflux/http/request"
 	"github.com/miniflux/miniflux/logger"
-
-	"github.com/tomasen/realip"
 )
 
 // Logging logs the HTTP request.
 func (m *Middleware) Logging(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		logger.Debug("[HTTP] %s %s %s", realip.RealIP(r), r.Method, r.RequestURI)
+		logger.Debug("[HTTP] %s %s %s", request.RealIP(r), r.Method, r.RequestURI)
 		next.ServeHTTP(w, r)
 	})
 }
diff --git a/ui/login_check.go b/ui/login_check.go
index bc49c8d..7cd63b7 100644
--- a/ui/login_check.go
+++ b/ui/login_check.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/miniflux/miniflux/http/context"
 	"github.com/miniflux/miniflux/http/cookie"
+	"github.com/miniflux/miniflux/http/request"
 	"github.com/miniflux/miniflux/http/response"
 	"github.com/miniflux/miniflux/http/response/html"
 	"github.com/miniflux/miniflux/http/route"
@@ -12,7 +13,6 @@ import (
 	"github.com/miniflux/miniflux/ui/form"
 	"github.com/miniflux/miniflux/ui/session"
 	"github.com/miniflux/miniflux/ui/view"
-	"github.com/tomasen/realip"
 )
 
 // CheckLogin validates the username/password and redirects the user to the unread page.
@@ -38,7 +38,7 @@ func (c *Controller) CheckLogin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	sessionToken, userID, err := c.store.CreateUserSession(authForm.Username, r.UserAgent(), realip.RealIP(r))
+	sessionToken, userID, err := c.store.CreateUserSession(authForm.Username, r.UserAgent(), request.RealIP(r))
 	if err != nil {
 		html.ServerError(w, err)
 		return
diff --git a/ui/oauth2_callback.go b/ui/oauth2_callback.go
index f564c7e..23e379b 100644
--- a/ui/oauth2_callback.go
+++ b/ui/oauth2_callback.go
@@ -16,8 +16,6 @@ import (
 	"github.com/miniflux/miniflux/logger"
 	"github.com/miniflux/miniflux/model"
 	"github.com/miniflux/miniflux/ui/session"
-
-	"github.com/tomasen/realip"
 )
 
 // OAuth2Callback receives the authorization code and create a new session.
@@ -107,7 +105,7 @@ func (c *Controller) OAuth2Callback(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	sessionToken, _, err := c.store.CreateUserSession(user.Username, r.UserAgent(), realip.RealIP(r))
+	sessionToken, _, err := c.store.CreateUserSession(user.Username, r.UserAgent(), request.RealIP(r))
 	if err != nil {
 		html.ServerError(w, err)
 		return
diff --git a/vendor/github.com/tomasen/realip/.travis.yml b/vendor/github.com/tomasen/realip/.travis.yml
deleted file mode 100644
index fdfbf87..0000000
--- a/vendor/github.com/tomasen/realip/.travis.yml
+++ /dev/null
@@ -1,19 +0,0 @@
-language: go
-
-go:
-  - tip
-
-before_install:
-  # lint
-  - go get github.com/golang/lint/golint
-
-  # code complexity
-  - go get github.com/on99/gocyclo
-
-  # test
-  - go get github.com/smartystreets/goconvey/convey
-
-script:
-  - golint ./...
-  - go vet ./...
-  - gocyclo -skip-godeps=true -top 10 -over 10 . # over 10 is bad code
diff --git a/vendor/github.com/tomasen/realip/README.md b/vendor/github.com/tomasen/realip/README.md
deleted file mode 100644
index 085f182..0000000
--- a/vendor/github.com/tomasen/realip/README.md
+++ /dev/null
@@ -1,33 +0,0 @@
-# RealIP
-
-[![GoDoc](https://godoc.org/github.com/Tomasen/realip?status.svg)](http://godoc.org/github.com/Tomasen/realip)
-
-Go package that can be used to get client's real public IP, which usually useful for logging HTTP server.
-
-### Feature
-
-* Follows the rule of X-Real-IP
-* Follows the rule of X-Forwarded-For
-* Exclude local or private address
-
-## Example
-
-```go
-package main
-
-import "github.com/Tomasen/realip"
-
-func (h *Handler) ServeIndexPage(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
-	clientIP := realip.FromRequest(r)
-	log.Println("GET / from", clientIP)
-}
-```
-
-## Developing
-
-Commited code must pass:
-
-* [golint](https://github.com/golang/lint)
-* [go vet](https://godoc.org/golang.org/x/tools/cmd/vet)
-* [gofmt](https://golang.org/cmd/gofmt)
-* [go test](https://golang.org/cmd/go/#hdr-Test_packages):
diff --git a/vendor/github.com/tomasen/realip/realip.go b/vendor/github.com/tomasen/realip/realip.go
deleted file mode 100644
index e2803a2..0000000
--- a/vendor/github.com/tomasen/realip/realip.go
+++ /dev/null
@@ -1,89 +0,0 @@
-package realip
-
-import (
-	"errors"
-	"net"
-	"net/http"
-	"strings"
-)
-
-var cidrs []*net.IPNet
-
-func init() {
-	maxCidrBlocks := []string{
-		"127.0.0.1/8",    // localhost
-		"10.0.0.0/8",     // 24-bit block
-		"172.16.0.0/12",  // 20-bit block
-		"192.168.0.0/16", // 16-bit block
-		"169.254.0.0/16", // link local address
-		"::1/128",        // localhost IPv6
-		"fc00::/7",       // unique local address IPv6
-		"fe80::/10",      // link local address IPv6
-	}
-
-	cidrs = make([]*net.IPNet, len(maxCidrBlocks))
-	for i, maxCidrBlock := range maxCidrBlocks {
-		_, cidr, _ := net.ParseCIDR(maxCidrBlock)
-		cidrs[i] = cidr
-	}
-}
-
-// isLocalAddress works by checking if the address is under private CIDR blocks.
-// List of private CIDR blocks can be seen on :
-//
-// https://en.wikipedia.org/wiki/Private_network
-//
-// https://en.wikipedia.org/wiki/Link-local_address
-func isPrivateAddress(address string) (bool, error) {
-	ipAddress := net.ParseIP(address)
-	if ipAddress == nil {
-		return false, errors.New("address is not valid")
-	}
-
-	for i := range cidrs {
-		if cidrs[i].Contains(ipAddress) {
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
-// FromRequest return client's real public IP address from http request headers.
-func FromRequest(r *http.Request) string {
-	// Fetch header value
-	xRealIP := r.Header.Get("X-Real-Ip")
-	xForwardedFor := r.Header.Get("X-Forwarded-For")
-
-	// If both empty, return IP from remote address
-	if xRealIP == "" && xForwardedFor == "" {
-		var remoteIP string
-
-		// If there are colon in remote address, remove the port number
-		// otherwise, return remote address as is
-		if strings.ContainsRune(r.RemoteAddr, ':') {
-			remoteIP, _, _ = net.SplitHostPort(r.RemoteAddr)
-		} else {
-			remoteIP = r.RemoteAddr
-		}
-
-		return remoteIP
-	}
-
-	// Check list of IP in X-Forwarded-For and return the first global address
-	for _, address := range strings.Split(xForwardedFor, ",") {
-		address = strings.TrimSpace(address)
-		isPrivate, err := isPrivateAddress(address)
-		if !isPrivate && err == nil {
-			return address
-		}
-	}
-
-	// If nothing succeed, return X-Real-IP
-	return xRealIP
-}
-
-// RealIP is depreciated, use FromRequest instead
-func RealIP(r *http.Request) string {
-	return FromRequest(r)
-}
diff --git a/vendor/github.com/tomasen/realip/realip_test.go b/vendor/github.com/tomasen/realip/realip_test.go
deleted file mode 100644
index e80efe0..0000000
--- a/vendor/github.com/tomasen/realip/realip_test.go
+++ /dev/null
@@ -1,95 +0,0 @@
-package realip
-
-import (
-	"net/http"
-	"testing"
-)
-
-func TestIsPrivateAddr(t *testing.T) {
-	testData := map[string]bool{
-		"127.0.0.0":   true,
-		"10.0.0.0":    true,
-		"169.254.0.0": true,
-		"192.168.0.0": true,
-		"::1":         true,
-		"fc00::":      true,
-
-		"172.15.0.0": false,
-		"172.16.0.0": true,
-		"172.31.0.0": true,
-		"172.32.0.0": false,
-
-		"147.12.56.11": false,
-	}
-
-	for addr, isLocal := range testData {
-		isPrivate, err := isPrivateAddress(addr)
-		if err != nil {
-			t.Errorf("fail processing %s: %v", addr, err)
-		}
-
-		if isPrivate != isLocal {
-			format := "%s should "
-			if !isLocal {
-				format += "not "
-			}
-			format += "be local address"
-
-			t.Errorf(format, addr)
-		}
-	}
-}
-
-func TestRealIP(t *testing.T) {
-	// Create type and function for testing
-	type testIP struct {
-		name     string
-		request  *http.Request
-		expected string
-	}
-
-	newRequest := func(remoteAddr, xRealIP string, xForwardedFor ...string) *http.Request {
-		h := http.Header{}
-		h.Set("X-Real-IP", xRealIP)
-		for _, address := range xForwardedFor {
-			h.Set("X-Forwarded-For", address)
-		}
-
-		return &http.Request{
-			RemoteAddr: remoteAddr,
-			Header:     h,
-		}
-	}
-
-	// Create test data
-	publicAddr1 := "144.12.54.87"
-	publicAddr2 := "119.14.55.11"
-	localAddr := "127.0.0.0"
-
-	testData := []testIP{
-		{
-			name:     "No header",
-			request:  newRequest(publicAddr1, ""),
-			expected: publicAddr1,
-		}, {
-			name:     "Has X-Forwarded-For",
-			request:  newRequest("", "", publicAddr1),
-			expected: publicAddr1,
-		}, {
-			name:     "Has multiple X-Forwarded-For",
-			request:  newRequest("", "", localAddr, publicAddr1, publicAddr2),
-			expected: publicAddr2,
-		}, {
-			name:     "Has X-Real-IP",
-			request:  newRequest("", publicAddr1),
-			expected: publicAddr1,
-		},
-	}
-
-	// Run test
-	for _, v := range testData {
-		if actual := FromRequest(v.request); v.expected != actual {
-			t.Errorf("%s: expected %s but get %s", v.name, v.expected, actual)
-		}
-	}
-}