diff --git a/ctl/check_test.go b/ctl/check_test.go index 144347685..ab5ef9279 100644 --- a/ctl/check_test.go +++ b/ctl/check_test.go @@ -93,7 +93,7 @@ func TestCheckCommand_Run(t *testing.T) { t.Fatalf("copy: %v", err) } - expectedPrefix := "checking bitmap: unmarshalling: unknown roaring magic number 12849" + expectedPrefix := "checking bitmap: unmarshalling: " if !strings.HasPrefix(err.Error(), expectedPrefix) { t.Fatalf("expect error: '%s...', actual: '%s'", expectedPrefix, err) } diff --git a/ctl/inspect_test.go b/ctl/inspect_test.go index d661aa1ab..1ff852712 100644 --- a/ctl/inspect_test.go +++ b/ctl/inspect_test.go @@ -41,8 +41,8 @@ func TestInspectCommand_Run(t *testing.T) { file.Close() cm.Path = file.Name() err = cm.Run(context.Background()) - expectedError := "unmarshalling: unknown roaring magic number 12849" - if err != nil && err.Error() != expectedError { + expectedError := "unmarshalling: " + if !strings.Contains(err.Error(), expectedError) { t.Fatalf("expected error '%s', got '%v'", expectedError, err) } diff --git a/roaring/fuzz_test.go b/roaring/fuzz_test.go index 8d934c209..86f99faa0 100644 --- a/roaring/fuzz_test.go +++ b/roaring/fuzz_test.go @@ -14,6 +14,7 @@ package roaring import ( + "strings" "testing" ) @@ -25,41 +26,41 @@ func TestUnmarshalBinary(t *testing.T) { }{ { // Checks for the zero containers situation cr: []byte(":0\x00\x00\x01\x00\x00\x000000"), //":000000" - expected: "reading official header: malformed bitmap, key-cardinality slice overruns buffer at 12", + expected: "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: "insufficient data for header + offsets: want 12935430920 bytes, got 32", + expected: "insufficient data for header + offsets:", }, { 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: "insufficient data for header + offsets: want 24696061960 bytes, got 32", + expected: "insufficient data for header + offsets:", }, { cr: []byte("<0\x00\x02\x00\x00\x00&x.field safe"), //"<0&x.field safe" - expected: "insufficient data for header + offsets: want 10200547336 bytes, got 20", + expected: "insufficient data for header + offsets:", }, { 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: "insufficient data for header + offsets: want 328 bytes, got 80", + expected: "insufficient data for header + offsets:", }, { // 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: "insufficient data for header + offsets: want 56 bytes, got 20", + expected: "insufficient data for header + offsets:", }, { // Checks for incomplete offset in readWithRuns cr: []byte(";0\x00\x00\v00000"), //";00 00000" - expected: "container 0/1, expect run length at 9/10 bytes", + expected: "insufficient data for offsets", }, { // Checks for incomplete offset in readOffsets cr: []byte(":0\x00\x00\x03\x00\x00\x00000000000000" + "\x00"), //:0000000000000 - expected: "insufficient data for offsets (need 12 bytes, found 1)", + expected: "insufficient data for offsets", }, } @@ -67,7 +68,7 @@ func TestUnmarshalBinary(t *testing.T) { err := b.UnmarshalBinary(crash.cr) if err == nil { t.Errorf("expected: %s, got: no error", crash.expected) - } else if err.Error() != crash.expected { + } else if !strings.Contains(err.Error(), crash.expected) { t.Errorf("expected: %s, got: %s", crash.expected, err) } } diff --git a/roaring/roaring.go b/roaring/roaring.go index 56c60d21a..86ee64fc5 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1364,7 +1364,7 @@ func (r *officialRoaringIterator) Next() (key uint64, cType byte, n int, length var runCount uint16 if r.currentType == containerRun { if int(r.currentDataOffset)+2 > len(r.data) { - r.Done(fmt.Errorf("container %d/%d, expect run length at %d/%d bytes", + r.Done(fmt.Errorf("insufficient data for offsets container %d/%d, expect run length at %d/%d bytes", r.currentIdx, r.keys, r.currentDataOffset, len(r.data))) return r.Current() } @@ -1595,81 +1595,6 @@ func (b *Bitmap) PreferMapping(preferred bool) { b.preferMapping = preferred } -// unmarshalPilosaRoaring treats data as being encoded in Pilosa's 64 bit -// roaring format and decodes it into b. -func (b *Bitmap) UnmarshalBinary(data []byte) (err error) { - if data == nil { - return errors.New("no roaring bitmap provided") - } - var itr roaringIterator - var itrKey uint64 - var itrCType byte - var itrN int - var itrLen int - var itrPointer *uint16 - var itrErr error - - itr, err = newRoaringIterator(data) - if err != nil { - return err - } - if itr == nil { - return errors.New("failed to create roaring iterator, but don't know why") - } - - b.Containers.Reset() - - itrKey, itrCType, itrN, itrLen, itrPointer, itrErr = itr.Next() - for itrErr == nil { - newC := &Container{ - typeID: itrCType, - n: int32(itrN), - len: int32(itrLen), - cap: int32(itrLen), - pointer: itrPointer, - flags: flagMapped, - } - if !b.preferMapping { - newC.unmapOrClone() - } - b.Containers.Put(itrKey, newC) - itrKey, itrCType, itrN, itrLen, itrPointer, itrErr = itr.Next() - } - // note: if we get a non-EOF err, it's possible that we made SOME - // changes but didn't log them. I don't have a good solution to this. - if itrErr != io.EOF { - return itrErr - } - - // Read ops log until the end of the file. - b.ops = 0 - b.opN = 0 - buf := itr.Remaining() - for { - // Exit when there are no more ops to parse. - if len(buf) == 0 { - break - } - - // Unmarshal the op and apply it. - var opr op - if err := opr.UnmarshalBinary(buf); err != nil { - // FIXME(benbjohnson): return error with position so file can be trimmed. - return err - } - - opr.apply(b) - - // Increase the op count. - b.ops++ - b.opN += opr.count() - - // Move the buffer forward. - buf = buf[opr.size():] - } - return nil -} - // writeOp writes op to the OpWriter, if available. func (b *Bitmap) writeOp(op *op) error { if b.OpWriter == nil { diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index f581f4bff..0ecd73021 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -3409,11 +3409,11 @@ func TestUnmarshalRoaringWithErrors(t *testing.T) { }{ { // Runs a bitmap without runs and no containers through the official roaring hexString: "3A30000000000000", - expectedError: "reading official header: malformed bitmap, key-cardinality slice overruns buffer at 8", + expectedError: "header: malformed bitmap, key-cardinality slice overruns buffer at 8", }, { // Runs a bitmap with runs and no containers through the official roaring hexString: "3B30000000000000", - expectedError: "reading official header: malformed bitmap, key-cardinality slice overruns buffer at 9", + expectedError: "header: malformed bitmap, key-cardinality slice overruns buffer at 9", }, { // Runs a bitmap in the Pilosa format through the Pilosa roaring hexString: "3C30000000000000", @@ -3427,7 +3427,7 @@ func TestUnmarshalRoaringWithErrors(t *testing.T) { bm := NewBitmap() err = bm.UnmarshalBinary(zeroContainers) if err != nil { - if err.Error() != loopContainers.expectedError { + if !strings.Contains(err.Error(), loopContainers.expectedError) { t.Fatalf("Expected: %s, Got: %s", loopContainers.expectedError, err) } } diff --git a/roaring/roaring_unmarshal_binary.go b/roaring/roaring_unmarshal_binary.go new file mode 100644 index 000000000..e7e0fc101 --- /dev/null +++ b/roaring/roaring_unmarshal_binary.go @@ -0,0 +1,208 @@ +// +build !enterprise + +package roaring + +import ( + "encoding/binary" + "fmt" + "unsafe" + + "github.com/pkg/errors" +) + +// UnmarshalBinary decodes b from a binary-encoded byte slice. data can be in +// either official roaring format or Pilosa's roaring format. +func (b *Bitmap) UnmarshalBinary(data []byte) error { + if data == nil { + // Nothing to unmarshal + return nil + } + statsHit("Bitmap/UnmarshalBinary") + b.opN = 0 // reset opN since we're reading new data. + fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2])) + if fileMagic == MagicNumber { // if pilosa roaring + return errors.Wrap(b.unmarshalPilosaRoaring(data), "unmarshaling as pilosa roaring") + } + + keyN, containerTyper, header, pos, haveRuns, err := readOfficialHeader(data) + if err != nil { + return errors.Wrap(err, "reading roaring header") + } + // Only the Pilosa roaring format has flags. The official Roaring format + // hasn't got space in its header for flags. + b.Flags = 0 + + b.Containers.ResetN(int(keyN)) + // Descriptive header section: Read container keys and cardinalities. + for i, buf := uint(0), data[header:]; i < uint(keyN); i, buf = i+1, buf[4:] { + card := int(binary.LittleEndian.Uint16(buf[2:4])) + 1 + b.Containers.PutContainerValues( + uint64(binary.LittleEndian.Uint16(buf[0:2])), + containerTyper(i, card), /// container type voodo with isRunBitmap + card, + true) + } + + // Read container offsets and attach data. + if haveRuns { + err := readWithRuns(b, data, pos, keyN) + if err != nil { + return errors.Wrap(err, "reading offsets from official roaring format") + } + } else { + err := readOffsets(b, data, pos, keyN) + if err != nil { + return errors.Wrap(err, "reading official roaring format") + } + } + return nil +} + +func readOffsets(b *Bitmap, data []byte, pos int, keyN uint32) error { + + citer, _ := b.Containers.Iterator(0) + for i, buf := 0, data[pos:]; i < int(keyN); i, buf = i+1, buf[4:] { + // Verify the offset is fully formed + if len(buf) < 4 { + return fmt.Errorf("insufficient data for offsets: len=%d", len(buf)) + } + offset := binary.LittleEndian.Uint32(buf[0:4]) + // Verify the offset is within the bounds of the input data. + if int(offset) >= len(data) { + return fmt.Errorf("offset out of bounds: off=%d, len=%d", offset, len(data)) + } + + // Map byte slice directly to the container data. + citer.Next() + _, c := citer.Value() + switch c.typ() { + case containerArray: + c.setArray((*[0xFFFFFFF]uint16)(unsafe.Pointer(&data[offset]))[:c.N():c.N()]) + case containerBitmap: + c.setBitmap((*[0xFFFFFFF]uint64)(unsafe.Pointer(&data[offset]))[:bitmapN:bitmapN]) + default: + return fmt.Errorf("unsupported container type %d", c.typ()) + } + } + return nil +} + +func readWithRuns(b *Bitmap, data []byte, pos int, keyN uint32) error { + if len(data) < pos+runCountHeaderSize { + return fmt.Errorf("insufficient data for offsets(run): len=%d", len(data)) + } + citer, _ := b.Containers.Iterator(0) + for i := 0; i < int(keyN); i++ { + citer.Next() + _, c := citer.Value() + switch c.typ() { + case containerRun: + runCount := binary.LittleEndian.Uint16(data[pos : pos+runCountHeaderSize]) + c.setRuns((*[0xFFFFFFF]interval16)(unsafe.Pointer(&data[pos+runCountHeaderSize]))[:runCount:runCount]) + runs := c.runs() + + for o := range runs { // must convert from start:length to start:end :( + runs[o].last = runs[o].start + runs[o].last + } + pos += int((runCount * interval16Size) + runCountHeaderSize) + case containerArray: + c.setArray((*[0xFFFFFFF]uint16)(unsafe.Pointer(&data[pos]))[:c.N():c.N()]) + pos += int(c.N() * 2) + case containerBitmap: + c.setBitmap((*[0xFFFFFFF]uint64)(unsafe.Pointer(&data[pos]))[:bitmapN:bitmapN]) + pos += bitmapN * 8 + } + } + return nil +} + +func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error { + if len(data) < headerBaseSize { + return errors.New("data too small") + } + + // Verify the first two bytes are a valid MagicNumber, and second two bytes match current storageVersion. + fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2])) + fileVersion := uint32(data[2]) + b.Flags = data[3] + if fileMagic != MagicNumber { + return fmt.Errorf("invalid roaring file, magic number %v is incorrect", fileMagic) + } + + if fileVersion != storageVersion { + return fmt.Errorf("wrong roaring version, file is v%d, server requires v%d", fileVersion, storageVersion) + } + + // Read key count in bytes sizeof(cookie)+sizeof(flag):(sizeof(cookie)+sizeof(uint32)). + keyN := binary.LittleEndian.Uint32(data[3+1 : 8]) + if uint32(len(data)) < headerBaseSize+keyN*12 { + return fmt.Errorf("insufficient data for header + offsets: key-cardinality not provided for %d containers", int(keyN)/12) + } + + headerSize := headerBaseSize + b.Containers.ResetN(int(keyN)) + // Descriptive header section: Read container keys and cardinalities. + for i, buf := 0, data[headerSize:]; i < int(keyN); i, buf = i+1, buf[12:] { + b.Containers.PutContainerValues( + binary.LittleEndian.Uint64(buf[0:8]), + byte(binary.LittleEndian.Uint16(buf[8:10])), + int(binary.LittleEndian.Uint16(buf[10:12]))+1, + true) + } + opsOffset := headerSize + int(keyN)*12 + + // Read container offsets and attach data. + citer, _ := b.Containers.Iterator(0) + for i, buf := 0, data[opsOffset:]; i < int(keyN); i, buf = i+1, buf[4:] { + offset := binary.LittleEndian.Uint32(buf[0:4]) + // Verify the offset is within the bounds of the input data. + if int(offset) >= len(data) { + return fmt.Errorf("offset out of bounds: off=%d, len=%d", offset, len(data)) + } + + // Map byte slice directly to the container data. + citer.Next() + _, c := citer.Value() + + // this shouldn't happen, since we don't normally store nils. + if c == nil { + continue + } + switch c.typ() { + case containerRun: + runCount := binary.LittleEndian.Uint16(data[offset : offset+runCountHeaderSize]) + c.setRuns((*[0xFFFFFFF]interval16)(unsafe.Pointer(&data[offset+runCountHeaderSize]))[:runCount:runCount]) + opsOffset = int(offset) + runCountHeaderSize + len(c.runs())*interval16Size + case containerArray: + c.setArray((*[0xFFFFFFF]uint16)(unsafe.Pointer(&data[offset]))[:c.N():c.N()]) + opsOffset = int(offset) + len(c.array())*2 // sizeof(uint32) + case containerBitmap: + c.setBitmap((*[0xFFFFFFF]uint64)(unsafe.Pointer(&data[offset]))[:bitmapN:bitmapN]) + opsOffset = int(offset) + len(c.bitmap())*8 // sizeof(uint64) + } + } + + // Read ops log until the end of the file. + buf := data[opsOffset:] + + for { + // Exit when there are no more ops to parse. + if len(buf) == 0 { + break + } + // Unmarshal the op and apply it. + var opr op + if err := opr.UnmarshalBinary(buf); err != nil { + // FIXME(benbjohnson): return error with position so file can be trimmed. + return err + } + opr.apply(b) + // Increase the op count. + b.ops++ + b.opN += opr.count() + // Move the buffer forward. + buf = buf[opr.size():] + } + + return nil +}