From b6d1e16b4ea4dfbe562a12a9399167c62d6eeb3d Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 17 Jan 2022 18:52:43 +0100 Subject: [PATCH] lib/protocol: Switch to a newer lz4 package (#8122) --- go.mod | 2 +- go.sum | 4 +-- .../syncthing/core/aboutModalView.html | 2 +- lib/protocol/protocol.go | 32 +++++++++-------- lib/protocol/protocol_test.go | 34 +++++++++++++++++-- 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index b40c10ac0..23bb4dc83 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ require ( github.com/AudriusButkevicius/pfilter v0.0.10 github.com/AudriusButkevicius/recli v0.0.6 github.com/alecthomas/kong v0.2.17 - github.com/bkaradzic/go-lz4 v0.0.0-20160924222819-7224d8d8f27e github.com/calmh/xdr v1.1.0 github.com/ccding/go-stun v0.1.3 github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect @@ -36,6 +35,7 @@ require ( github.com/minio/sha256-simd v1.0.0 github.com/miscreant/miscreant.go v0.0.0-20200214223636-26d376326b75 github.com/oschwald/geoip2-golang v1.5.0 + github.com/pierrec/lz4/v4 v4.1.12 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.0 github.com/prometheus/common v0.30.0 // indirect diff --git a/go.sum b/go.sum index 7d28a9f4f..0b74c3e2e 100644 --- a/go.sum +++ b/go.sum @@ -60,8 +60,6 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/bkaradzic/go-lz4 v0.0.0-20160924222819-7224d8d8f27e h1:2augTYh6E+XoNrrivZJBadpThP/dsvYKj0nzqfQ8tM4= -github.com/bkaradzic/go-lz4 v0.0.0-20160924222819-7224d8d8f27e/go.mod h1:0YdlkowM3VswSROI7qDxhRvJ3sLhlFrRRwjwegp5jy4= github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625/go.mod h1:HYsPBTaaSFSlLx/70C2HPIMNZpVV8+vt/A+FMnYP11g= github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7hK1yFx9hf58LP0zeX7UjIGs20ufpu3evjr+s= github.com/calmh/xdr v1.1.0 h1:U/Dd4CXNLoo8EiQ4ulJUXkgO1/EyQLgDKLgpY1SOoJE= @@ -299,6 +297,8 @@ github.com/oschwald/maxminddb-golang v1.8.0 h1:Uh/DSnGoxsyp/KYbY1AuP0tYEwfs0sCph github.com/oschwald/maxminddb-golang v1.8.0/go.mod h1:RXZtst0N6+FY/3qCNmZMBApR19cdQj43/NM9VkrNAis= github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 h1:q2e307iGHPdTGp0hoxKjt1H5pDo6utceo3dQVK3I5XQ= github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5/go.mod h1:jvVRKCrJTQWu0XVbaOlby/2lO20uSCHEMzzplHXte1o= +github.com/pierrec/lz4/v4 v4.1.12 h1:44l88ehTZAUGW4VlO1QC4zkilL99M6Y9MXNwEs0uzP8= +github.com/pierrec/lz4/v4 v4.1.12/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= diff --git a/gui/default/syncthing/core/aboutModalView.html b/gui/default/syncthing/core/aboutModalView.html index b2623a0ce..2a6fc1136 100644 --- a/gui/default/syncthing/core/aboutModalView.html +++ b/gui/default/syncthing/core/aboutModalView.html @@ -37,7 +37,7 @@ Aaron Bieber, Adam Piggott, Adel Qalieh, Alan Pope, Alberto Donato, Alessandro G
  • AudriusButkevicius/go-nat-pmp, Copyright © 2013 John Howard Palevich.
  • AudriusButkevicius/recli, Copyright © 2019 Audrius Butkevicius.
  • beorn7/perks, Copyright © 2013 Blake Mizerany.
    • -
  • bkaradzic/go-lz4, Copyright © 2011-2012 Branimir Karadzic, 2013 Damian Gryski.
    • +
  • pierrec/lz4, Copyright © 2015 Pierre Curto.
  • calmh/du, Public domain.
  • calmh/xdr, Copyright © 2014 Jakob Borg.
  • chmduquesne/rollinghash, Copyright © 2015 Christophe-Marie Duquesne.
    • diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go index 7afca5a1e..bed05c3a9 100644 --- a/lib/protocol/protocol.go +++ b/lib/protocol/protocol.go @@ -23,7 +23,7 @@ import ( "sync" "time" - lz4 "github.com/bkaradzic/go-lz4" + lz4 "github.com/pierrec/lz4/v4" "github.com/pkg/errors" ) @@ -63,6 +63,10 @@ var sha256OfEmptyBlock = map[int][sha256.Size]byte{ 16 << MiB: {0x8, 0xa, 0xcf, 0x35, 0xa5, 0x7, 0xac, 0x98, 0x49, 0xcf, 0xcb, 0xa4, 0x7d, 0xc2, 0xad, 0x83, 0xe0, 0x1b, 0x75, 0x66, 0x3a, 0x51, 0x62, 0x79, 0xc8, 0xb9, 0xd2, 0x43, 0xb7, 0x19, 0x64, 0x3e}, } +var ( + errNotCompressible = errors.New("not compressible") +) + func init() { for blockSize := MinBlockSize; blockSize <= MaxBlockSize; blockSize *= 2 { BlockSizes = append(BlockSizes, blockSize) @@ -800,7 +804,7 @@ func (c *rawConnection) writeCompressedMessage(msg message, marshaled []byte, ov } cOverhead := 2 + hdrSize + 4 - maxCompressed := cOverhead + lz4.CompressBound(len(marshaled)) + maxCompressed := cOverhead + lz4.CompressBlockBound(len(marshaled)) buf := BufferPool.Get(maxCompressed) defer BufferPool.Put(buf) @@ -1010,32 +1014,30 @@ func (c *rawConnection) Statistics() Statistics { } func lz4Compress(src, buf []byte) (int, error) { - compressed, err := lz4.Encode(buf, src) + // The compressed block is prefixed by the size of the uncompressed data. + binary.BigEndian.PutUint32(buf, uint32(len(src))) + + n, err := lz4.CompressBlock(src, buf[4:], nil) if err != nil { return -1, err - } - if &compressed[0] != &buf[0] { - panic("bug: lz4.Compress allocated, which it must not (should use buffer pool)") + } else if len(src) > 0 && n == 0 { + return -1, errNotCompressible } - binary.BigEndian.PutUint32(compressed, binary.LittleEndian.Uint32(compressed)) - return len(compressed), nil + return n + 4, nil } func lz4Decompress(src []byte) ([]byte, error) { size := binary.BigEndian.Uint32(src) - binary.LittleEndian.PutUint32(src, size) - var err error buf := BufferPool.Get(int(size)) - decoded, err := lz4.Decode(buf, src) + + n, err := lz4.UncompressBlock(src[4:], buf) if err != nil { BufferPool.Put(buf) return nil, err } - if &decoded[0] != &buf[0] { - panic("bug: lz4.Decode allocated, which it must not (should use buffer pool)") - } - return decoded, nil + + return buf[:n], nil } func newProtocolError(err error, msgContext string) error { diff --git a/lib/protocol/protocol_test.go b/lib/protocol/protocol_test.go index 81a74bb9f..85e73011e 100644 --- a/lib/protocol/protocol_test.go +++ b/lib/protocol/protocol_test.go @@ -17,7 +17,7 @@ import ( "testing/quick" "time" - lz4 "github.com/bkaradzic/go-lz4" + lz4 "github.com/pierrec/lz4/v4" "github.com/syncthing/syncthing/lib/rand" "github.com/syncthing/syncthing/lib/testutils" ) @@ -484,7 +484,7 @@ func TestLZ4Compression(t *testing.T) { t.Fatal(err) } - comp := make([]byte, lz4.CompressBound(dataLen)) + comp := make([]byte, lz4.CompressBlockBound(dataLen)) compLen, err := lz4Compress(data, comp) if err != nil { t.Errorf("compressing %d bytes: %v", dataLen, err) @@ -506,6 +506,36 @@ func TestLZ4Compression(t *testing.T) { } } +func TestLZ4CompressionUpdate(t *testing.T) { + uncompressed := []byte("this is some arbitrary yet fairly compressible data") + + // Compressed, as created by the LZ4 implementation in Syncthing 1.18.6 and earlier. + oldCompressed, _ := hex.DecodeString("00000033f0247468697320697320736f6d65206172626974726172792079657420666169726c7920636f6d707265737369626c652064617461") + + // Verify that we can decompress + + res, err := lz4Decompress(oldCompressed) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(uncompressed, res) { + t.Fatal("result does not match") + } + + // Verify that our current compression is equivalent + + buf := make([]byte, 128) + n, err := lz4Compress(uncompressed, buf) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(oldCompressed, buf[:n]) { + t.Logf("%x", oldCompressed) + t.Logf("%x", buf[:n]) + t.Fatal("compression does not match") + } +} + func TestCheckFilename(t *testing.T) { cases := []struct { name string