fix a case when Limit is 0 when syncing mailservers (#29)

It also improves passing errors to the requester.
This commit is contained in:
Adam Babik 2019-06-26 11:11:20 +02:00 committed by GitHub
parent 0b74212950
commit 3e303b1d5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 26 additions and 13 deletions

View File

@ -2,7 +2,7 @@ notifications:
email: false
language: go
go:
- 1.10.x
- 1.12.x
sudo: false
jobs:
include:

View File

@ -122,8 +122,12 @@ type SyncMailRequest struct {
// Validate checks request's fields if they are valid.
func (r SyncMailRequest) Validate() error {
if r.Limit == 0 {
return errors.New("invalid 'Limit' value, expected value greater than 0")
}
if r.Limit > MaxLimitInSyncMailRequest {
return fmt.Errorf("invalid 'Limit' value, expected lower than %d", MaxLimitInSyncMailRequest)
return fmt.Errorf("invalid 'Limit' value, expected value lower than %d", MaxLimitInSyncMailRequest)
}
if r.Lower > r.Upper {

View File

@ -16,17 +16,18 @@ func TestSyncMailRequestValidate(t *testing.T) {
Error string
}{
{
Name: "empty request is valid",
Req: SyncMailRequest{},
Name: "invalid zero Limit",
Req: SyncMailRequest{},
Error: "invalid 'Limit' value, expected value greater than 0",
},
{
Name: "invalid Limit",
Name: "invalid large Limit",
Req: SyncMailRequest{Limit: 1e6},
Error: "invalid 'Limit' value, expected lower than 1000",
Error: "invalid 'Limit' value, expected value lower than 1000",
},
{
Name: "invalid Lower",
Req: SyncMailRequest{Lower: 10, Upper: 5},
Req: SyncMailRequest{Limit: 10, Lower: 10, Upper: 5},
Error: "invalid 'Lower' value, can't be greater than 'Upper'",
},
}

View File

@ -1058,7 +1058,15 @@ func (whisper *Whisper) runMessageLoop(p *Peer, rw p2p.MsgReadWriter) error {
}
if err := whisper.mailServer.SyncMail(p, request); err != nil {
log.Error("failed to sync envelopes", "peer", p.peer.ID().String())
log.Error(
"failed to sync envelopes",
"peer", p.peer.ID().String(),
)
_ = whisper.SendSyncResponse(
p,
SyncResponse{Error: err.Error()},
)
return err
}
} else {
log.Debug("requested to sync messages but mail servers is not registered", "peer", p.peer.ID().String())

View File

@ -1378,7 +1378,7 @@ func TestSyncMessages(t *testing.T) {
w.peers[newPeer(w, p, rw1)] = struct{}{}
err := w.SyncMessages(p.ID().Bytes(), SyncMailRequest{Lower: 10, Upper: 5})
err := w.SyncMessages(p.ID().Bytes(), SyncMailRequest{Limit: 10, Lower: 10, Upper: 5})
require.EqualError(t, err, "invalid 'Lower' value, can't be greater than 'Upper'")
})
@ -1392,7 +1392,7 @@ func TestSyncMessages(t *testing.T) {
w.peers[newPeer(w, p, rw1)] = struct{}{}
go func() {
err := w.SyncMessages(p.ID().Bytes(), SyncMailRequest{})
err := w.SyncMessages(p.ID().Bytes(), SyncMailRequest{Limit: 10})
require.NoError(t, err)
}()
@ -1425,14 +1425,14 @@ func TestHandleP2PSyncRequestCode(t *testing.T) {
w.RegisterServer(mailMock)
go func() {
err := p2p.Send(rw1, p2pSyncRequestCode, SyncMailRequest{})
err := p2p.Send(rw1, p2pSyncRequestCode, SyncMailRequest{Limit: 10})
require.NoError(t, err)
require.NoError(t, rw1.Close())
}()
err := w.runMessageLoop(peer, rw2)
if err != nil && err != p2p.ErrPipeClosed {
require.FailNow(t, "invalid err", err)
require.FailNow(t, "invalid err", err.Error())
}
mailMock.AssertNumberOfCalls(t, "SyncMail", 1)
@ -1449,7 +1449,7 @@ func TestHandleP2PSyncRequestCodeWithInvalidRequest(t *testing.T) {
w.RegisterServer(mailMock)
// create an invalid request
req := SyncMailRequest{Lower: 10, Upper: 5}
req := SyncMailRequest{Limit: 10, Lower: 10, Upper: 5}
require.Error(t, req.Validate())
go func() {