Skip to content

Commit

Permalink
minor modification to tests involving ips
Browse files Browse the repository at this point in the history
  • Loading branch information
@karmanyaahm
karmanyaahm committed Oct 8, 2022
1 parent 511d3f6 commit 3b29294
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
14 changes: 9 additions & 5 deletions server/message_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import (
"github.com/stretchr/testify/require"
)

var (
exampleIP1234 = netip.MustParseAddr("1.2.3.4")
)

func TestSqliteCache_Messages(t *testing.T) {
testCacheMessages(t, newSqliteTestCache(t))
}
Expand Down Expand Up @@ -283,7 +287,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
expires1 := time.Now().Add(-4 * time.Hour).Unix()
m := newDefaultMessage("mytopic", "flower for you")
m.ID = "m1"
m.Sender = netip.MustParseAddr("1.2.3.4")
m.Sender = exampleIP1234
m.Attachment = &attachment{
Name: "flower.jpg",
Type: "image/jpeg",
Expand All @@ -296,7 +300,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
expires2 := time.Now().Add(2 * time.Hour).Unix() // Future
m = newDefaultMessage("mytopic", "sending you a car")
m.ID = "m2"
m.Sender = netip.MustParseAddr("1.2.3.4")
m.Sender = exampleIP1234
m.Attachment = &attachment{
Name: "car.jpg",
Type: "image/jpeg",
Expand All @@ -309,7 +313,7 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
expires3 := time.Now().Add(1 * time.Hour).Unix() // Future
m = newDefaultMessage("another-topic", "sending you another car")
m.ID = "m3"
m.Sender = netip.MustParseAddr("1.2.3.4")
m.Sender = exampleIP1234
m.Attachment = &attachment{
Name: "another-car.jpg",
Type: "image/jpeg",
Expand All @@ -329,15 +333,15 @@ func testCacheAttachments(t *testing.T, c *messageCache) {
require.Equal(t, int64(5000), messages[0].Attachment.Size)
require.Equal(t, expires1, messages[0].Attachment.Expires)
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, "car.jpg", messages[1].Attachment.Name)
require.Equal(t, "image/jpeg", messages[1].Attachment.Type)
require.Equal(t, int64(10000), messages[1].Attachment.Size)
require.Equal(t, expires2, messages[1].Attachment.Expires)
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")
require.Nil(t, err)
Expand Down
7 changes: 6 additions & 1 deletion server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,12 @@ func (s *Server) visitor(r *http.Request) *visitor {
ipport, err := netip.ParseAddrPort(remoteAddr)
ip := ipport.Addr()
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")) != "" {
// X-Forwarded-For can contain multiple addresses (see #328). If we are behind a proxy,
Expand Down
18 changes: 9 additions & 9 deletions server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,13 @@ func TestServer_PublishAt(t *testing.T) {
messages = toMessages(t, response.Body.String())
require.Equal(t, 1, len(messages))
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)
require.Nil(t, err)
require.Equal(t, 1, len(messages))
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) {
Expand Down Expand Up @@ -1134,7 +1134,7 @@ func TestServer_PublishAttachment(t *testing.T) {
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.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))

// GET
Expand Down Expand Up @@ -1170,7 +1170,7 @@ func TestServer_PublishAttachmentShortWithFilename(t *testing.T) {
require.Equal(t, int64(21), msg.Attachment.Size)
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.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))

path := strings.TrimPrefix(msg.Attachment.URL, "http://127.0.0.1:12345")
Expand All @@ -1197,7 +1197,7 @@ func TestServer_PublishAttachmentExternalWithoutFilename(t *testing.T) {
require.Equal(t, "", msg.Attachment.Type)
require.Equal(t, int64(0), msg.Attachment.Size)
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
size, err := s.messageCache.AttachmentBytesUsed("127.0.0.1")
Expand All @@ -1218,7 +1218,7 @@ func TestServer_PublishAttachmentExternalWithFilename(t *testing.T) {
require.Equal(t, "", msg.Attachment.Type)
require.Equal(t, int64(0), msg.Attachment.Size)
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) {
Expand Down Expand Up @@ -1393,7 +1393,7 @@ func TestServer_Visitor_XForwardedFor_None(t *testing.T) {
r.RemoteAddr = "8.9.10.11"
r.Header.Set("X-Forwarded-For", " ") // Spaces, not empty!
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) {
Expand All @@ -1404,7 +1404,7 @@ func TestServer_Visitor_XForwardedFor_Single(t *testing.T) {
r.RemoteAddr = "8.9.10.11"
r.Header.Set("X-Forwarded-For", "1.1.1.1")
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) {
Expand All @@ -1415,7 +1415,7 @@ func TestServer_Visitor_XForwardedFor_Multiple(t *testing.T) {
r.RemoteAddr = "8.9.10.11"
r.Header.Set("X-Forwarded-For", "1.2.3.4 , 2.4.4.2,234.5.2.1 ")
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) {
Expand Down

0 comments on commit 3b29294

Please sign in to comment.