Skip to content

Commit

Permalink
S2: limit max repeat length (#188)
Browse files Browse the repository at this point in the history
On single, extremely large single block encodes with high extremely long repeats, we could store the wrong length.

Does not affect streaming mode.
  • Loading branch information
klauspost authored Nov 28, 2019
1 parent 0ebbf25 commit 0583f81
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
9 changes: 9 additions & 0 deletions s2/encode_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,21 @@ func emitRepeat(dst []byte, offset, length int) int {
dst[0] = 6<<2 | tagCopy1
return 4
}
const maxRepeat = (1 << 24) - 1
length -= 1 << 16
left := 0
if length > maxRepeat {
left = length - maxRepeat
length = maxRepeat
}
dst[4] = uint8(length >> 16)
dst[3] = uint8(length >> 8)
dst[2] = uint8(length >> 0)
dst[1] = 0
dst[0] = 7<<2 | tagCopy1
if left > 0 {
return 5 + emitRepeat(dst[5:], offset, left)
}
return 5
}

Expand Down
26 changes: 26 additions & 0 deletions s2/s2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,32 @@ func testFile(t *testing.T, i, repeat int) {
})
}

func TestDataRoundtrips(t *testing.T) {
test := func(t *testing.T, data []byte) {
t.Run("s2", func(t *testing.T) {
testWriterRoundtrip(t, data)
})
t.Run("s2-better", func(t *testing.T) {
testWriterRoundtrip(t, data, WriterBetterCompression())
})
t.Run("block", func(t *testing.T) {
d := data
testBlockRoundtrip(t, d)
})
t.Run("block-better", func(t *testing.T) {
d := data
testBetterBlockRoundtrip(t, d)
})
t.Run("snappy", func(t *testing.T) {
testSnappyDecode(t, data)
})
}
t.Run("longblock", func(t *testing.T) {
data := make([]byte, 1<<25)
test(t, data)
})
}

// Naming convention is kept similar to what snappy's C++ implementation uses.
func Benchmark_UFlat0(b *testing.B) { benchFile(b, 0, true) }
func Benchmark_UFlat1(b *testing.B) { benchFile(b, 1, true) }
Expand Down

0 comments on commit 0583f81

Please sign in to comment.