add request validation for mail sync (#14)

whisperv6/doc.go

@@ -33,6 +33,8 @@ particularly the notion of singular endpoints.
 package whisperv6
 
 import (
+	"errors"
+	"fmt"
 	"time"
 )
 
@@ -80,6 +82,8 @@ const (
 
 	DefaultTTL           = 50 // seconds
 	DefaultSyncAllowance = 10 // seconds
+
+	MaxLimitInSyncMailRequest = 1000
 )
 
 // MailServer represents a mail server, capable of
@@ -109,6 +113,19 @@ type SyncMailRequest struct {
 	Cursor []byte
 }
 
+// Validate checks request's fields if they are valid.
+func (r SyncMailRequest) Validate() error {
+	if r.Limit > MaxLimitInSyncMailRequest {
+		return fmt.Errorf("invalid 'Limit' value, expected lower than %d", MaxLimitInSyncMailRequest)
+	}
+
+	if r.Lower > r.Upper {
+		return errors.New("invalid 'Lower' value, can't be greater than 'Upper'")
+	}
+
+	return nil
+}
+
 // SyncResponse is a struct representing a response sent to the peer
 // asking for syncing archived envelopes.
 type SyncResponse struct {
@@ -117,10 +134,3 @@ type SyncResponse struct {
 	Final     bool // if true it means all envelopes were processed
 	Error     string
 }
-
-// IsFinal returns true if it's the final response for the request.
-// It might be a successful final response (r.Final being true)
-// or an error occured (r.Error being not empty).
-func (r SyncResponse) IsFinal() bool {
-	return r.Final || r.Error != ""
-}

whisperv6/doc_test.go

@@ -0,0 +1,41 @@
+package whisperv6
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSyncMailRequestValidate(t *testing.T) {
+	testCases := []struct {
+		Name  string
+		Req   SyncMailRequest
+		Error string
+	}{
+		{
+			Name: "empty request is valid",
+			Req:  SyncMailRequest{},
+		},
+		{
+			Name:  "invalid Limit",
+			Req:   SyncMailRequest{Limit: 1e6},
+			Error: "invalid 'Limit' value, expected lower than 1000",
+		},
+		{
+			Name:  "invalid Lower",
+			Req:   SyncMailRequest{Lower: 10, Upper: 5},
+			Error: "invalid 'Lower' value, can't be greater than 'Upper'",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			err := tc.Req.Validate()
+			if tc.Error != "" {
+				assert.EqualError(t, err, tc.Error)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

whisperv6/whisper.go

@@ -463,6 +463,10 @@ func (whisper *Whisper) SyncMessages(peerID []byte, req SyncMailRequest) error {
 		return err
 	}
 
+	if err := req.Validate(); err != nil {
+		return err
+	}
+
 	return p2p.Send(p.ws, p2pSyncRequestCode, req)
 }
 
@@ -975,10 +979,14 @@ func (whisper *Whisper) runMessageLoop(p *Peer, rw p2p.MsgReadWriter) error {
 			// TODO(adam): should we limit who can send this request?
 			if whisper.mailServer != nil {
 				var request SyncMailRequest
-				if err = packet.Decode(&request); err != nil {
+				if err := packet.Decode(&request); err != nil {
 					return fmt.Errorf("failed to decode p2pSyncRequestCode payload: %v", err)
 				}
 
+				if err := request.Validate(); err != nil {
+					return fmt.Errorf("sync mail request was invalid: %v", err)
+				}
+
 				if err := whisper.mailServer.SyncMail(p, request); err != nil {
 					log.Error("failed to sync envelopes", "peer", p.peer.ID().String())
 				}

whisperv6/whisper_test.go

@@ -1257,6 +1257,19 @@ func TestSyncMessages(t *testing.T) {
 		require.EqualError(t, err, "Could not find peer with ID: 0102")
 	})
 
+	t.Run("WithInvalidRequest", func(t *testing.T) {
+		w := New(nil)
+		w.RegisterServer(&stubMailServer{})
+
+		p := p2p.NewPeer(enode.ID{0x01}, "peer01", nil)
+		rw1, _ := p2p.MsgPipe()
+
+		w.peers[newPeer(w, p, rw1)] = struct{}{}
+
+		err := w.SyncMessages(p.ID().Bytes(), SyncMailRequest{Lower: 10, Upper: 5})
+		require.EqualError(t, err, "invalid 'Lower' value, can't be greater than 'Upper'")
+	})
+
 	t.Run("AllGood", func(t *testing.T) {
 		w := New(nil)
 		w.RegisterServer(&stubMailServer{})
@@ -1313,6 +1326,32 @@ func TestHandleP2PSyncRequestCode(t *testing.T) {
 	mailMock.AssertNumberOfCalls(t, "SyncMail", 1)
 }
 
+func TestHandleP2PSyncRequestCodeWithInvalidRequest(t *testing.T) {
+	rw1, rw2 := p2p.MsgPipe()
+	peer := newPeer(nil, p2p.NewPeer(enode.ID{}, "test", nil), nil)
+
+	mailMock := &mockMailServer{}
+	mailMock.On("SyncMail", peer, mock.Anything).Return(nil)
+
+	w := New(nil)
+	w.RegisterServer(mailMock)
+
+	// create an invalid request
+	req := SyncMailRequest{Lower: 10, Upper: 5}
+	require.Error(t, req.Validate())
+
+	go func() {
+		err := p2p.Send(rw1, p2pSyncRequestCode, req)
+		require.NoError(t, err)
+		require.NoError(t, rw1.Close())
+	}()
+
+	err := w.runMessageLoop(peer, rw2)
+	require.EqualError(t, err, "sync mail request was invalid: invalid 'Lower' value, can't be greater than 'Upper'")
+
+	mailMock.AssertNumberOfCalls(t, "SyncMail", 0)
+}
+
 func TestHandleP2PSyncResponseCode(t *testing.T) {
 	rw1, rw2 := p2p.MsgPipe()
 	peer := newPeer(nil, p2p.NewPeer(enode.ID{}, "test", nil), nil)