minor modification to tests involving ips

pull/429/head
Karmanyaah Malhotra 2022-10-07 20:27:22 -05:00
parent 511d3f6aaf
commit 3b29294679
3 changed files with 24 additions and 15 deletions

View File

@ -12,6 +12,10 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
var (
exampleIP1234 = netip.MustParseAddr("1.2.3.4")
)
func TestSqliteCache_Messages(t *testing.T) { func TestSqliteCache_Messages(t *testing.T) {
testCacheMessages(t, newSqliteTestCache(t)) testCacheMessages(t, newSqliteTestCache(t))
} }
@ -283,7 +287,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
expires1 := time.Now().Add(-4 * time.Hour).Unix() expires1 := time.Now().Add(-4 * time.Hour).Unix()
m := newDefaultMessage("mytopic", "flower for you") m := newDefaultMessage("mytopic", "flower for you")
m.ID = "m1" m.ID = "m1"
m.Sender = netip.MustParseAddr("1.2.3.4") m.Sender = exampleIP1234
m.Attachment = &attachment{ m.Attachment = &attachment{
Name: "flower.jpg", Name: "flower.jpg",
Type: "image/jpeg", Type: "image/jpeg",
@ -296,7 +300,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
expires2 := time.Now().Add(2 * time.Hour).Unix() // Future expires2 := time.Now().Add(2 * time.Hour).Unix() // Future
m = newDefaultMessage("mytopic", "sending you a car") m = newDefaultMessage("mytopic", "sending you a car")
m.ID = "m2" m.ID = "m2"
m.Sender = netip.MustParseAddr("1.2.3.4") m.Sender = exampleIP1234
m.Attachment = &attachment{ m.Attachment = &attachment{
Name: "car.jpg", Name: "car.jpg",
Type: "image/jpeg", Type: "image/jpeg",
@ -309,7 +313,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
expires3 := time.Now().Add(1 * time.Hour).Unix() // Future expires3 := time.Now().Add(1 * time.Hour).Unix() // Future
m = newDefaultMessage("another-topic", "sending you another car") m = newDefaultMessage("another-topic", "sending you another car")
m.ID = "m3" m.ID = "m3"
m.Sender = netip.MustParseAddr("1.2.3.4") m.Sender = exampleIP1234
m.Attachment = &attachment{ m.Attachment = &attachment{
Name: "another-car.jpg", Name: "another-car.jpg",
Type: "image/jpeg", Type: "image/jpeg",
@ -329,7 +333,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
require.Equal(t, int64(5000), messages[0].Attachment.Size) require.Equal(t, int64(5000), messages[0].Attachment.Size)
require.Equal(t, expires1, messages[0].Attachment.Expires) require.Equal(t, expires1, messages[0].Attachment.Expires)
require.Equal(t, "https://ntfy.sh/file/AbDeFgJhal.jpg", messages[0].Attachment.URL) require.Equal(t, "https://ntfy.sh/file/AbDeFgJhal.jpg", messages[0].Attachment.URL)
require.Equal(t, "1.2.3.4", messages[0].Sender) require.Equal(t, "1.2.3.4", messages[0].Sender.String())
require.Equal(t, "sending you a car", messages[1].Message) require.Equal(t, "sending you a car", messages[1].Message)
require.Equal(t, "car.jpg", messages[1].Attachment.Name) require.Equal(t, "car.jpg", messages[1].Attachment.Name)
@ -337,7 +341,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
require.Equal(t, int64(10000), messages[1].Attachment.Size) require.Equal(t, int64(10000), messages[1].Attachment.Size)
require.Equal(t, expires2, messages[1].Attachment.Expires) require.Equal(t, expires2, messages[1].Attachment.Expires)
require.Equal(t, "https://ntfy.sh/file/aCaRURL.jpg", messages[1].Attachment.URL) require.Equal(t, "https://ntfy.sh/file/aCaRURL.jpg", messages[1].Attachment.URL)
require.Equal(t, "1.2.3.4", messages[1].Sender) require.Equal(t, "1.2.3.4", messages[1].Sender.String())
size, err := c.AttachmentBytesUsed("1.2.3.4") size, err := c.AttachmentBytesUsed("1.2.3.4")
require.Nil(t, err) require.Nil(t, err)

View File

@ -1440,7 +1440,12 @@ func (s *Server) visitor(r *http.Request) *visitor {
ipport, err := netip.ParseAddrPort(remoteAddr) ipport, err := netip.ParseAddrPort(remoteAddr)
ip := ipport.Addr() ip := ipport.Addr()
if err != nil { if err != nil {
ip = netip.MustParseAddr(remoteAddr) // This should not happen in real life; only in tests. So, using MustParse, which panics on error. // This should not happen in real life; only in tests. So, using falling back to 0.0.0.0 if address unspecified
ip, err = netip.ParseAddr(remoteAddr)
if err != nil {
ip = netip.IPv4Unspecified()
log.Error("Unable to parse IP (%s), new visitor with unspecified IP (0.0.0.0) created %s", remoteAddr, err)
}
} }
if s.config.BehindProxy && strings.TrimSpace(r.Header.Get("X-Forwarded-For")) != "" { if s.config.BehindProxy && strings.TrimSpace(r.Header.Get("X-Forwarded-For")) != "" {
// X-Forwarded-For can contain multiple addresses (see #328). If we are behind a proxy, // X-Forwarded-For can contain multiple addresses (see #328). If we are behind a proxy,

View File

@ -294,13 +294,13 @@ func TestServer_PublishAt(t *testing.T) {
messages = toMessages(t, response.Body.String()) messages = toMessages(t, response.Body.String())
require.Equal(t, 1, len(messages)) require.Equal(t, 1, len(messages))
require.Equal(t, "a message", messages[0].Message) require.Equal(t, "a message", messages[0].Message)
require.Equal(t, "", messages[0].Sender) // Never return the sender! require.Equal(t, netip.Addr{}, messages[0].Sender) // Never return the sender!
messages, err := s.messageCache.Messages("mytopic", sinceAllMessages, true) messages, err := s.messageCache.Messages("mytopic", sinceAllMessages, true)
require.Nil(t, err) require.Nil(t, err)
require.Equal(t, 1, len(messages)) require.Equal(t, 1, len(messages))
require.Equal(t, "a message", messages[0].Message) require.Equal(t, "a message", messages[0].Message)
require.Equal(t, "9.9.9.9", messages[0].Sender) // It's stored in the DB though! require.Equal(t, "9.9.9.9", messages[0].Sender.String()) // It's stored in the DB though!
} }
func TestServer_PublishAtWithCacheError(t *testing.T) { func TestServer_PublishAtWithCacheError(t *testing.T) {
@ -1134,7 +1134,7 @@ func TestServer_PublishAttachment(t *testing.T) {
require.Equal(t, int64(5000), msg.Attachment.Size) require.Equal(t, int64(5000), msg.Attachment.Size)
require.GreaterOrEqual(t, msg.Attachment.Expires, time.Now().Add(179*time.Minute).Unix()) // Almost 3 hours require.GreaterOrEqual(t, msg.Attachment.Expires, time.Now().Add(179*time.Minute).Unix()) // Almost 3 hours
require.Contains(t, msg.Attachment.URL, "http://127.0.0.1:12345/file/") require.Contains(t, msg.Attachment.URL, "http://127.0.0.1:12345/file/")
require.Equal(t, "", msg.Sender) // Should never be returned require.Equal(t, netip.Addr{}, msg.Sender) // Should never be returned
require.FileExists(t, filepath.Join(s.config.AttachmentCacheDir, msg.ID)) require.FileExists(t, filepath.Join(s.config.AttachmentCacheDir, msg.ID))
// GET // GET
@ -1170,7 +1170,7 @@ func TestServer_PublishAttachmentShortWithFilename(t *testing.T) {
require.Equal(t, int64(21), msg.Attachment.Size) require.Equal(t, int64(21), msg.Attachment.Size)
require.GreaterOrEqual(t, msg.Attachment.Expires, time.Now().Add(3*time.Hour).Unix()) require.GreaterOrEqual(t, msg.Attachment.Expires, time.Now().Add(3*time.Hour).Unix())
require.Contains(t, msg.Attachment.URL, "http://127.0.0.1:12345/file/") require.Contains(t, msg.Attachment.URL, "http://127.0.0.1:12345/file/")
require.Equal(t, "", msg.Sender) // Should never be returned require.Equal(t, netip.Addr{}, msg.Sender) // Should never be returned
require.FileExists(t, filepath.Join(s.config.AttachmentCacheDir, msg.ID)) require.FileExists(t, filepath.Join(s.config.AttachmentCacheDir, msg.ID))
path := strings.TrimPrefix(msg.Attachment.URL, "http://127.0.0.1:12345") path := strings.TrimPrefix(msg.Attachment.URL, "http://127.0.0.1:12345")
@ -1197,7 +1197,7 @@ func TestServer_PublishAttachmentExternalWithoutFilename(t *testing.T) {
require.Equal(t, "", msg.Attachment.Type) require.Equal(t, "", msg.Attachment.Type)
require.Equal(t, int64(0), msg.Attachment.Size) require.Equal(t, int64(0), msg.Attachment.Size)
require.Equal(t, int64(0), msg.Attachment.Expires) require.Equal(t, int64(0), msg.Attachment.Expires)
require.Equal(t, "", msg.Sender) require.Equal(t, netip.Addr{}, msg.Sender)
// Slightly unrelated cross-test: make sure we don't add an owner for external attachments // Slightly unrelated cross-test: make sure we don't add an owner for external attachments
size, err := s.messageCache.AttachmentBytesUsed("127.0.0.1") size, err := s.messageCache.AttachmentBytesUsed("127.0.0.1")
@ -1218,7 +1218,7 @@ func TestServer_PublishAttachmentExternalWithFilename(t *testing.T) {
require.Equal(t, "", msg.Attachment.Type) require.Equal(t, "", msg.Attachment.Type)
require.Equal(t, int64(0), msg.Attachment.Size) require.Equal(t, int64(0), msg.Attachment.Size)
require.Equal(t, int64(0), msg.Attachment.Expires) require.Equal(t, int64(0), msg.Attachment.Expires)
require.Equal(t, "", msg.Sender) require.Equal(t, netip.Addr{}, msg.Sender)
} }
func TestServer_PublishAttachmentBadURL(t *testing.T) { func TestServer_PublishAttachmentBadURL(t *testing.T) {
@ -1393,7 +1393,7 @@ func TestServer_Visitor_XForwardedFor_None(t *testing.T) {
r.RemoteAddr = "8.9.10.11" r.RemoteAddr = "8.9.10.11"
r.Header.Set("X-Forwarded-For", " ") // Spaces, not empty! r.Header.Set("X-Forwarded-For", " ") // Spaces, not empty!
v := s.visitor(r) v := s.visitor(r)
require.Equal(t, "8.9.10.11", v.ip) require.Equal(t, "8.9.10.11", v.ip.String())
} }
func TestServer_Visitor_XForwardedFor_Single(t *testing.T) { func TestServer_Visitor_XForwardedFor_Single(t *testing.T) {
@ -1404,7 +1404,7 @@ func TestServer_Visitor_XForwardedFor_Single(t *testing.T) {
r.RemoteAddr = "8.9.10.11" r.RemoteAddr = "8.9.10.11"
r.Header.Set("X-Forwarded-For", "1.1.1.1") r.Header.Set("X-Forwarded-For", "1.1.1.1")
v := s.visitor(r) v := s.visitor(r)
require.Equal(t, "1.1.1.1", v.ip) require.Equal(t, "1.1.1.1", v.ip.String())
} }
func TestServer_Visitor_XForwardedFor_Multiple(t *testing.T) { func TestServer_Visitor_XForwardedFor_Multiple(t *testing.T) {
@ -1415,7 +1415,7 @@ func TestServer_Visitor_XForwardedFor_Multiple(t *testing.T) {
r.RemoteAddr = "8.9.10.11" r.RemoteAddr = "8.9.10.11"
r.Header.Set("X-Forwarded-For", "1.2.3.4 , 2.4.4.2,234.5.2.1 ") r.Header.Set("X-Forwarded-For", "1.2.3.4 , 2.4.4.2,234.5.2.1 ")
v := s.visitor(r) v := s.visitor(r)
require.Equal(t, "234.5.2.1", v.ip) require.Equal(t, "234.5.2.1", v.ip.String())
} }
func TestServer_PublishWhileUpdatingStatsWithLotsOfMessages(t *testing.T) { func TestServer_PublishWhileUpdatingStatsWithLotsOfMessages(t *testing.T) {