diff --git a/whisperv6/doc.go b/whisperv6/doc.go index f911d26..eb1da06 100644 --- a/whisperv6/doc.go +++ b/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 != "" -} diff --git a/whisperv6/doc_test.go b/whisperv6/doc_test.go new file mode 100644 index 0000000..8ff251e --- /dev/null +++ b/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) + } + }) + } +} diff --git a/whisperv6/whisper.go b/whisperv6/whisper.go index 2b92d58..ad41f85 100644 --- a/whisperv6/whisper.go +++ b/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()) } diff --git a/whisperv6/whisper_test.go b/whisperv6/whisper_test.go index a842b72..b277a60 100644 --- a/whisperv6/whisper_test.go +++ b/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)