From 453c29a46521636c0b4b6d80e7bf8cc45077c042 Mon Sep 17 00:00:00 2001 From: Ashley Svetlik Date: Wed, 19 Jun 2019 11:38:20 -0500 Subject: [PATCH 1/7] Fixed a malformed bitmap bug in pilosa roaring --- roaring/fuzz_test.go | 35 ++++++++++++++++++++++++++++++----- roaring/roaring.go | 5 ++++- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/roaring/fuzz_test.go b/roaring/fuzz_test.go index 69362d947..abcac1c0b 100644 --- a/roaring/fuzz_test.go +++ b/roaring/fuzz_test.go @@ -23,15 +23,40 @@ func TestUnmarshalBinary(t *testing.T) { cr []byte expected string } { - { - cr : []byte(":0\x000\x01\x00\x00\x000000"), //":000000" - expected : "reading roaring header: malformed bitmap, key-cardinality slice overruns buffer at 12", - }, - { + { // Checks for int overflow cr : []byte("<0\x000\x00\x00\x00\x00000000000000" + "0"), //"<000000000000000" expected : "unmarshaling as pilosa roaring: Maximum operation size exceeded", }, + { // Checks for the zero containers situation + cr : []byte(":0\x000\x01\x00\x00\x000000"), //":000000" + expected : "reading roaring header: malformed bitmap, key-cardinality slice overruns buffer at 12", + }, + { // The next 5 check for malformed bitmaps + cr : []byte("<0\x0000000000000000000" + + "\x00\x00\xec\x00\x03\x00\x00\x00\xec000"), //"<000000000000000000ÏÏ000" + expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 67372036 containers", + }, + { + cr : []byte("<0\x00\x02\x00\x00\x00\\f\x01\xb5\x8d\x009\v\x01\x00\x00\x00\x00" + + "\x00\x00e\x04\x00\x00\x00\x04\xfd\x00\x01\x00"), //"<0\fµç9e˝" + expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 128625322 containers", + }, + { + cr : []byte("<0\x00\x02\x00\x00\x00&x.field safe"), //"<0&x.field safe" + expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 53127850 containers", + }, + { + cr : []byte("<0\x00\x00\x14\x00\x00\x00\x80\xffp\x05_ 4\x114089" + + "\x00\x00\xff\x000\x00\x02\x00\x00\x00\x00\xff\u007f\x00\x00\x01\x10\x00\x00j" + + "\x02\x00\x00$\x04_\x00\xff\u007f\xff062616163\x00" + //"<0ġp_ 44089ˇ0ˇj$_ˇˇ0626161630ø¸ad$j√" + "0\x00\x02\x00\x01\xbf\x00\x04\x00\xfcad$\x00\x00j\x10\x00\x00\xc3"), + expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 1 containers", + }, + { // 0 containers because the container is partially formed, but not fully (ie. 3/12 = 0) + cr : []byte("<0\x00\x02\x03\x00\x00\x00쳫\v\x00d9\v\x00\x009\v"), //<0쳫 d9 9 + expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 0 containers", + }, } for _, crash := range confirmedCrashers { diff --git a/roaring/roaring.go b/roaring/roaring.go index d7334be05..985ffa67e 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1095,7 +1095,7 @@ func (b *Bitmap) writeToUnoptimized(w io.Writer) (n int64, err error) { // unmarshalPilosaRoaring treats data as being encoded in Pilosa's 64 bit // roaring format and decodes it into b. func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error { - if len(data) < headerBaseSize { + if len(data) <= headerBaseSize { return errors.New("data too small") } @@ -1113,6 +1113,9 @@ func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error { // Read key count in bytes sizeof(cookie)+sizeof(flag):(sizeof(cookie)+sizeof(uint32)). keyN := binary.LittleEndian.Uint32(data[3+1 : 8]) + if len(data) < headerBaseSize+int(keyN)*12 { + return fmt.Errorf("malformed bitmap, key-cardinality not provided for %d containers", int(keyN)/12) + } headerSize := headerBaseSize b.Containers.Reset() From 656efb4ea629713159ca5cc4de2d4fbbd75fc0ae Mon Sep 17 00:00:00 2001 From: Ashley Svetlik Date: Wed, 19 Jun 2019 13:08:54 -0500 Subject: [PATCH 2/7] Fixed fuzz_test.go order to mergability --- roaring/fuzz_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/roaring/fuzz_test.go b/roaring/fuzz_test.go index abcac1c0b..78b4bb34a 100644 --- a/roaring/fuzz_test.go +++ b/roaring/fuzz_test.go @@ -23,14 +23,14 @@ func TestUnmarshalBinary(t *testing.T) { cr []byte expected string } { + { // Checks for the zero containers situation + cr : []byte(":0\x000\x01\x00\x00\x000000"), //":000000" + expected : "reading roaring header: malformed bitmap, key-cardinality slice overruns buffer at 12", + }, { // Checks for int overflow cr : []byte("<0\x000\x00\x00\x00\x00000000000000" + "0"), //"<000000000000000" expected : "unmarshaling as pilosa roaring: Maximum operation size exceeded", - }, - { // Checks for the zero containers situation - cr : []byte(":0\x000\x01\x00\x00\x000000"), //":000000" - expected : "reading roaring header: malformed bitmap, key-cardinality slice overruns buffer at 12", }, { // The next 5 check for malformed bitmaps cr : []byte("<0\x0000000000000000000" + From 873486bc1a3425eb350cde6a87d3914b3ae853b3 Mon Sep 17 00:00:00 2001 From: Ashley Svetlik Date: Thu, 20 Jun 2019 14:43:55 -0500 Subject: [PATCH 3/7] Removed no containers pilosa format fix --- roaring/roaring.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index fe69140c7..1adbc102e 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1102,7 +1102,7 @@ func (b *Bitmap) writeToUnoptimized(w io.Writer) (n int64, err error) { // unmarshalPilosaRoaring treats data as being encoded in Pilosa's 64 bit // roaring format and decodes it into b. func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error { - if len(data) <= headerBaseSize { + if len(data) < headerBaseSize { return errors.New("data too small") } @@ -3954,7 +3954,7 @@ func (op *op) WriteTo(w io.Writer) (n int64, err error) { } var minOpSize = 13 -var maxBatchSize = uint64(1<<59) +var maxBatchSize = uint64(1 << 59) // UnmarshalBinary decodes data into an op. func (op *op) UnmarshalBinary(data []byte) error { From 031e23cdeabeaffa9a7d64be08c13bd65dc19c0d Mon Sep 17 00:00:00 2001 From: Ashley Svetlik Date: Thu, 20 Jun 2019 15:02:18 -0500 Subject: [PATCH 4/7] Resolved int overflow --- roaring/roaring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 1adbc102e..b29e9f01f 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1120,7 +1120,7 @@ func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error { // Read key count in bytes sizeof(cookie)+sizeof(flag):(sizeof(cookie)+sizeof(uint32)). keyN := binary.LittleEndian.Uint32(data[3+1 : 8]) - if len(data) < headerBaseSize+int(keyN)*12 { + if uint32(len(data)) < headerBaseSize+keyN*12 { return fmt.Errorf("malformed bitmap, key-cardinality not provided for %d containers", int(keyN)/12) } From 481c85acae6dc3d3bdae92410a47a1d86d57c410 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Thu, 20 Jun 2019 17:32:57 -0500 Subject: [PATCH 5/7] more info if nodeleave confirmation queries fail --- cluster.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cluster.go b/cluster.go index cf0090c0f..4c32f6b42 100644 --- a/cluster.go +++ b/cluster.go @@ -1709,15 +1709,17 @@ func confirmNodeDown(uri URI, log logger.Logger) bool { log.Printf("bad request:%s %s", u.String(), err) return false } - for i := 0; i < confirmDownRetries; i++ { resp, err := http.DefaultClient.Do(req.WithContext(ctx)) + var bod []byte if err == nil { + bod, err = ioutil.ReadAll(resp.Body) if resp.StatusCode == 200 { return false } } - log.Printf("NodeLeave Timeout with %s %d", uri.HostPort(), i) + + log.Printf("NodeLeave confirm with %s %d. err: '%v' bod: '%s'", uri.HostPort(), i, err, bod) time.Sleep(confirmDownSleep * time.Second) } return true From 8fd23239a1eca199a2ea7c97106b185b66176a1c Mon Sep 17 00:00:00 2001 From: Ashley Svetlik Date: Fri, 21 Jun 2019 10:09:20 -0500 Subject: [PATCH 6/7] Formatted Fuzz_test.go --- roaring/fuzz_test.go | 68 ++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/roaring/fuzz_test.go b/roaring/fuzz_test.go index 94bd82982..232ebf2f6 100644 --- a/roaring/fuzz_test.go +++ b/roaring/fuzz_test.go @@ -13,49 +13,49 @@ // limitations under the License. package roaring -import ( +import ( "testing" ) func TestUnmarshalBinary(t *testing.T) { b := NewBitmap() confirmedCrashers := []struct { - cr []byte + cr []byte expected string - } { - { // Checks for the zero containers situation - cr : []byte(":0\x000\x01\x00\x00\x000000"), //":000000" - expected : "reading roaring header: malformed bitmap, key-cardinality slice overruns buffer at 12", - }, - { // Checks for int overflow - cr : []byte("<0\x000\x00\x00\x00\x00000000000000" + - "0"), //"<000000000000000" - expected : "unmarshaling as pilosa roaring: Maximum operation size exceeded", - }, - { // The next 5 check for malformed bitmaps - cr : []byte("<0\x0000000000000000000" + - "\x00\x00\xec\x00\x03\x00\x00\x00\xec000"), //"<000000000000000000ÏÏ000" - expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 67372036 containers", - }, - { - cr : []byte("<0\x00\x02\x00\x00\x00\\f\x01\xb5\x8d\x009\v\x01\x00\x00\x00\x00" + - "\x00\x00e\x04\x00\x00\x00\x04\xfd\x00\x01\x00"), //"<0\fµç9e˝" - expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 128625322 containers", + }{ + { // Checks for the zero containers situation + cr: []byte(":0\x000\x01\x00\x00\x000000"), //":000000" + expected: "reading roaring header: malformed bitmap, key-cardinality slice overruns buffer at 12", + }, + { // Checks for int overflow + cr: []byte("<0\x000\x00\x00\x00\x00000000000000" + + "0"), //"<000000000000000" + expected: "unmarshaling as pilosa roaring: Maximum operation size exceeded", + }, + { // The next 5 check for malformed bitmaps + cr: []byte("<0\x0000000000000000000" + + "\x00\x00\xec\x00\x03\x00\x00\x00\xec000"), //"<000000000000000000ÏÏ000" + expected: "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 67372036 containers", }, { - cr : []byte("<0\x00\x02\x00\x00\x00&x.field safe"), //"<0&x.field safe" - expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 53127850 containers", + cr: []byte("<0\x00\x02\x00\x00\x00\\f\x01\xb5\x8d\x009\v\x01\x00\x00\x00\x00" + + "\x00\x00e\x04\x00\x00\x00\x04\xfd\x00\x01\x00"), //"<0\fµç9e˝" + expected: "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 128625322 containers", }, { - cr : []byte("<0\x00\x00\x14\x00\x00\x00\x80\xffp\x05_ 4\x114089" + - "\x00\x00\xff\x000\x00\x02\x00\x00\x00\x00\xff\u007f\x00\x00\x01\x10\x00\x00j" + - "\x02\x00\x00$\x04_\x00\xff\u007f\xff062616163\x00" + //"<0ġp_ 44089ˇ0ˇj$_ˇˇ0626161630ø¸ad$j√" - "0\x00\x02\x00\x01\xbf\x00\x04\x00\xfcad$\x00\x00j\x10\x00\x00\xc3"), - expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 1 containers", + cr: []byte("<0\x00\x02\x00\x00\x00&x.field safe"), //"<0&x.field safe" + expected: "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 53127850 containers", }, - { // 0 containers because the container is partially formed, but not fully (ie. 3/12 = 0) - cr : []byte("<0\x00\x02\x03\x00\x00\x00쳫\v\x00d9\v\x00\x009\v"), //<0쳫 d9 9 - expected : "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 0 containers", + { + cr: []byte("<0\x00\x00\x14\x00\x00\x00\x80\xffp\x05_ 4\x114089" + + "\x00\x00\xff\x000\x00\x02\x00\x00\x00\x00\xff\u007f\x00\x00\x01\x10\x00\x00j" + + "\x02\x00\x00$\x04_\x00\xff\u007f\xff062616163\x00" + //"<0ġp_ 44089ˇ0ˇj$_ˇˇ0626161630ø¸ad$j√" + "0\x00\x02\x00\x01\xbf\x00\x04\x00\xfcad$\x00\x00j\x10\x00\x00\xc3"), + expected: "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 1 containers", + }, + { // 0 containers because the container is partially formed, but not fully (ie. 3/12 = 0) + cr: []byte("<0\x00\x02\x03\x00\x00\x00쳫\v\x00d9\v\x00\x009\v"), //<0쳫 d9 9 + expected: "unmarshaling as pilosa roaring: malformed bitmap, key-cardinality not provided for 0 containers", }, } @@ -63,7 +63,7 @@ func TestUnmarshalBinary(t *testing.T) { err := b.UnmarshalBinary(crash.cr) if err.Error() != crash.expected { t.Errorf("Expected: %s, Got: %s", crash.expected, err) - } + } } - -} \ No newline at end of file + +} From c0d067b7ee4ad937c13bf5c3c3a8b2228af22a8a Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 21 Jun 2019 12:15:58 -0500 Subject: [PATCH 7/7] move context timeout inside loop, so context gets a fresh deadline --- cluster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cluster.go b/cluster.go index 4c32f6b42..c972daadd 100644 --- a/cluster.go +++ b/cluster.go @@ -1702,14 +1702,14 @@ func confirmNodeDown(uri URI, log logger.Logger) bool { Host: uri.HostPort(), Path: "version", } - ctx, cancel := context.WithTimeout(context.Background(), confirmDownTimeout*time.Second) - defer cancel() req, err := http.NewRequest("GET", u.String(), nil) if err != nil { log.Printf("bad request:%s %s", u.String(), err) return false } for i := 0; i < confirmDownRetries; i++ { + ctx, cancel := context.WithTimeout(context.Background(), confirmDownTimeout*time.Second) + defer cancel() resp, err := http.DefaultClient.Do(req.WithContext(ctx)) var bod []byte if err == nil {