Skip to content

Commit cb83489

Browse files
authored
fix file IO flaw + CI fixes (#7)
There was a suble bug in reading files, due to not using io.ReadFull, but instead doing partial reads. This wasn't found in the tests, due to them only handling small files. The root issue has been fixed, aswell as better tests which hit the issue. * ci: fixes for circle-ci config on v2 * fix error with file reading/writing, where non-full reads caused corruption
1 parent 16f9758 commit cb83489

File tree

5 files changed

+14
-19
lines changed

5 files changed

+14
-19
lines changed

.circleci/config.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414

1515
# specify any bash command here prefixed with `run: `
1616
- run: go get -v -t -d ./...
17-
- run: go test -v ./... -coverprofile=coverage.txt -covermode=count
17+
- run: (cd v2 && go test -v ./... -coverprofile=coverage.txt -covermode=count )
1818
- run:
1919
name: "Codecov upload"
2020
command: bash <(curl -s https://codecov.io/bash)
@@ -23,5 +23,5 @@ jobs:
2323
command: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.23.8
2424
- run:
2525
name: "Lint"
26-
command: golangci-lint run
26+
command: (cd v2 && golangci-lint run)
2727

v2/binarymarshaler.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,16 @@ func (f *Filter) MarshallToWriter(out io.Writer) (int, [sha512.Size384]byte, err
7474
if chunkSize < 512 {
7575
chunkSize = 512 // Min 4K bytes (512 uint64s)
7676
}
77-
bs := make([]byte, chunkSize*8)
77+
buf := make([]byte, chunkSize*8)
7878
for start := 0; start < len(f.bits); {
7979
end := start + chunkSize
8080
if end > len(f.bits) {
8181
end = len(f.bits)
8282
}
8383
for i, x := range f.bits[start:end] {
84-
binary.LittleEndian.PutUint64(bs[8*i:], x)
84+
binary.LittleEndian.PutUint64(buf[8*i:], x)
8585
}
86-
if _, err := mw.Write(bs[0 : (end-start)*8]); err != nil {
86+
if _, err := mw.Write(buf[0 : (end-start)*8]); err != nil {
8787
return c.bytes, hash, err
8888
}
8989
start = end

v2/binaryunmarshaler.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121

2222
func unmarshalBinaryHeader(r io.Reader) (k, n, m uint64, err error) {
2323
magic := make([]byte, len(headerMagic))
24-
if _, err := r.Read(magic); err != nil {
24+
if _, err := io.ReadFull(r, magic); err != nil {
2525
return 0, 0, 0, err
2626
}
2727
if !bytes.Equal(magic, headerMagic) {
@@ -41,7 +41,6 @@ func unmarshalBinaryHeader(r io.Reader) (k, n, m uint64, err error) {
4141
if m < MMin {
4242
return 0, 0, 0, fmt.Errorf("number of bits in the filter must be >= %d (was %d)", MMin, m)
4343
}
44-
4544
return k, n, m, err
4645
}
4746

@@ -52,7 +51,7 @@ func unmarshalBinaryBits(r io.Reader, m uint64) (bits []uint64, err error) {
5251
}
5352
bs := make([]byte, 8)
5453
for i := 0; i < len(bits) && err == nil; i++ {
55-
_, err = r.Read(bs)
54+
_, err = io.ReadFull(r, bs)
5655
bits[i] = binary.LittleEndian.Uint64(bs)
5756
}
5857
if err != nil {
@@ -81,7 +80,7 @@ func (h *hashingReader) Read(p []byte) (n int, err error) {
8180
if err != nil {
8281
return n, err
8382
}
84-
_, _ = h.hasher.Write(p)
83+
_, _ = h.hasher.Write(p[:n])
8584
return n, err
8685
}
8786

v2/fileio.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ func (f *Filter) ReadFrom(r io.Reader) (n int64, err error) {
3636

3737
// ReadFrom Reader r into a lossless-compressed Bloom filter f
3838
func ReadFrom(r io.Reader) (f *Filter, n int64, err error) {
39+
f = new(Filter)
3940
rawR, err := gzip.NewReader(r)
4041
if err != nil {
4142
return nil, -1, err
4243
}
4344
defer rawR.Close()
44-
f = new(Filter)
4545
n, err = f.UnmarshalFromReader(rawR)
4646
if err != nil {
4747
return nil, -1, err
@@ -67,11 +67,10 @@ func (f *Filter) WriteTo(w io.Writer) (n int64, err error) {
6767
defer f.lock.RUnlock()
6868

6969
rawW := gzip.NewWriter(w)
70-
defer func() {
71-
err = rawW.Close()
72-
}()
70+
defer rawW.Close()
7371

7472
intN, _, err := f.MarshallToWriter(rawW)
73+
//intN, _, err := f.MarshallToWriter(w)
7574
n = int64(intN)
7675
return n, err
7776
}
@@ -83,9 +82,7 @@ func (f *Filter) WriteFile(filename string) (n int64, err error) {
8382
if err != nil {
8483
return -1, err
8584
}
86-
defer func() {
87-
err = w.Close()
88-
}()
85+
defer w.Close()
8986

9087
return f.WriteTo(w)
9188
}

v2/fileio_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (d devnull) Write(p []byte) (n int, err error) {
3030

3131
func TestWriteRead(t *testing.T) {
3232
// minimal filter
33-
f, _ := New(8*32, 5)
33+
f, _ := New(8*1024*100, 5)
3434
// Add some content
3535
var tests = make([]hashableUint64, 20)
3636
for i := 0; i < 20; i++ {
@@ -89,7 +89,7 @@ func TestWriteRead(t *testing.T) {
8989
verify(t, &f2)
9090
})
9191
t.Run("file", func(t *testing.T) {
92-
fName := filepath.Join(os.TempDir(), "temp.deleteme")
92+
fName := filepath.Join(os.TempDir(), "temp.deleteme.gz")
9393
if _, err := f.WriteFile(fName); err != nil {
9494
t.Fatal(err)
9595
}
@@ -100,7 +100,6 @@ func TestWriteRead(t *testing.T) {
100100
verify(t, f2)
101101
}
102102
})
103-
104103
}
105104

106105
func TestCorruption(t *testing.T) {

0 commit comments

Comments
 (0)