RemoteAddr fixes

pkg/models/forum_recent.go

@@ -148,7 +148,7 @@ func PaginateRecentPosts(user *User, categories []string, pager *Pagination) ([]
 			rc.Thread = thr
 			thrs = append(thrs, thr)
 		} else {
-			log.Error("RecentPosts: didn't find thread ID %d in map!")
+			log.Error("RecentPosts: didn't find thread ID %d in map!", rc.ThreadID)
 		}
 
 		if f, ok := forums[rc.ForumID]; ok {

pkg/models/subscription.go

@@ -41,7 +41,7 @@ func GetSubscribers(tableName string, tableID uint64) []uint64 {
 	).Scan(&userIDs)
 
 	if result.Error != nil {
-		log.Error("GetSubscribers(%s, %d): couldn't get user IDs: %s", tableName, tableID)
+		log.Error("GetSubscribers(%s, %d): couldn't get user IDs: %s", tableName, tableID, result.Error)
 	}
 
 	return userIDs

pkg/models/thread.go

@@ -238,7 +238,7 @@ func MapThreadStatistics(threads []*Thread) ThreadStatsMap {
 	).Group("table_id").Scan(&groups)
 
 	if err != nil {
-		log.Error("MapThreadStatistics: SQL error: %s")
+		log.Error("MapThreadStatistics: SQL error: %s", err)
 	}
 
 	// Map the results in.

pkg/photo/filenames.go

@@ -27,7 +27,7 @@ func NewFilename(ext string) string {
 	basename := uuid.New().String()
 	first2 := basename[:2]
 	next2 := basename[2:4]
-	log.Debug("photo.NewFilename: UUID %s first2 %d next2 %d", basename, first2, next2)
+	log.Debug("photo.NewFilename: UUID %s first2 %s next2 %s", basename, first2, next2)
 	return fmt.Sprintf(
 		"%s/%s/%s%s",
 		first2, next2, basename, ext,

pkg/photo/upload.go

@@ -85,7 +85,7 @@ func UploadPhoto(cfg UploadConfig) (string, string, error) {
 			newHeight := config.MaxPhotoWidth
 			width = int((float64(width) / float64(height)) * float64(newHeight))
 			height = newHeight
-			log.Debug("Its longest is height, scale to %sx%s", width, height)
+			log.Debug("Its longest is height, scale to %dx%d", width, height)
 		}
 	}
 

pkg/redis/redis.go

@@ -81,7 +81,7 @@ func Exists(key string) bool {
 	if err != nil {
 		return false
 	}
-	log.Debug("redis.Exists(%s): %s", key, val)
+	log.Debug("redis.Exists(%s): %d", key, val)
 	return val == 1
 }
 

pkg/session/session.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"regexp"
 	"strings"
 	"time"
 
@@ -59,7 +60,7 @@ func LoadOrNew(r *http.Request) *Session {
 	err = redis.Get(key, sess)
 	// log.Error("LoadOrNew: raw from Redis: %+v", sess)
 	if err != nil {
-		log.Error("session.LoadOrNew: didn't find %s in Redis: %s", err)
+		log.Error("session.LoadOrNew: didn't find %s in Redis: %s", key, err)
 	}
 
 	return sess
@@ -113,17 +114,21 @@ func Get(r *http.Request) *Session {
 	return nil
 }
 
+var portSuffixRegexp = regexp.MustCompile(`:(\d+)$`)
+
 // RemoteAddr returns the user's remote IP address. If UseXForwardedFor is enabled in settings.json,
 // the HTTP header X-Forwarded-For may be returned here or otherwise the request RemoteAddr is returned.
 func RemoteAddr(r *http.Request) string {
+	var remoteAddr = r.RemoteAddr // Usually "ip:port" format
 	if config.Current.UseXForwardedFor {
 		xff := r.Header.Get("X-Forwarded-For")
 		if len(xff) > 0 {
-			return strings.SplitN(xff, ",", 1)[0]
+			remoteAddr = strings.SplitN(xff, ",", 2)[0]
 		}
 	}
 
-	return strings.SplitN(r.RemoteAddr, ":", 1)[0]
+	// Return just the IP and not the port suffix.
+	return portSuffixRegexp.ReplaceAllString(remoteAddr, "")
 }
 
 // ReadFlashes returns and clears the Flashes and Errors for this session.

pkg/session/session_test.go

@@ -0,0 +1,85 @@
+package session_test
+
+import (
+	"net/http"
+	"testing"
+
+	"code.nonshy.com/nonshy/website/pkg/config"
+	"code.nonshy.com/nonshy/website/pkg/session"
+)
+
+func TestRemoteAddr(t *testing.T) {
+	var tests = []struct {
+		RemoteAddr    string
+		XForwardedFor string
+		Expect        string
+	}{
+		{
+			"127.0.0.1:12345",
+			"",
+			"127.0.0.1",
+		},
+		{
+			"127.0.0.1:22022",
+			"8.8.4.4:12345",
+			"8.8.4.4",
+		},
+		{
+			"127.0.0.1:11223",
+			"8.8.4.4:12345, 8.8.1.1, 1.1.1.1",
+			"8.8.4.4",
+		},
+		{
+			"127.0.0.1",
+			"8.8.8.8, 8.8.4.4, 1.1.1.1",
+			"8.8.8.8",
+		},
+		{
+			"127.0.0.1",
+			"2001:db8:85a3:8d3:1319:8a2e:370:7348",
+			"2001:db8:85a3:8d3:1319:8a2e:370", // acceptable bug
+		},
+		{
+			"127.0.0.1",
+			"2001:db8:85a3:8d3:1319:8a2e:370:7bee",
+			"2001:db8:85a3:8d3:1319:8a2e:370:7bee",
+		},
+		{
+			"127.0.0.1",
+			"2001:db8:85a3:8d3:1319:8a2e:370:7bee, 127.0.0.7",
+			"2001:db8:85a3:8d3:1319:8a2e:370:7bee",
+		},
+	}
+
+	// Test all cases with X-Forwarded-For enabled.
+	config.Current.UseXForwardedFor = true
+	for _, test := range tests {
+		r, _ := http.NewRequest("GET", "/", nil)
+		r.RemoteAddr = test.RemoteAddr
+		if test.XForwardedFor != "" {
+			r.Header.Set("X-Forwarded-For", test.XForwardedFor)
+		}
+
+		actual := session.RemoteAddr(r)
+		if actual != test.Expect {
+			t.Errorf("RemoteAddr expected %s but got %s for (RemoteAddr=%s  XForwardedFor=%s)",
+				test.Expect, actual, test.RemoteAddr, test.XForwardedFor,
+			)
+		}
+	}
+
+	// Test them without X-Forwarded-For -- the expect should always be the RemoteAddr.
+	config.Current.UseXForwardedFor = false
+	for _, test := range tests {
+		r, _ := http.NewRequest("GET", "/", nil)
+		r.RemoteAddr = test.RemoteAddr
+		if test.XForwardedFor != "" {
+			r.Header.Set("X-Forwarded-For", test.XForwardedFor)
+		}
+
+		actual := session.RemoteAddr(r)
+		if actual != "127.0.0.1" {
+			t.Errorf("Without X-Forwarded-For %+v did not return 127.0.0.1", test)
+		}
+	}
+}