From 6809d38cdeef7c0e8f6eb2c880fbd5cf7f4e8559 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sun, 1 Jan 2017 17:19:00 +0000 Subject: [PATCH] lib/protocol: Revert protobuf encoder changes in v0.14.17 (fixes #3855) The protobuf encoder now produces packed arrays for things like []int32, which is actually correct according to the proto3 spec. However Syncthing v0.14.16 and earlier doesn't support this. This reverts the encoding change, but keeps the updated decoder so that we are both more compatible with other proto3 implementations and can move to the updated encoder in the future. GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3856 --- .gitattributes | 1 - lib/protocol/bep.pb.go | 25 ++++++------------------- lib/protocol/protocol_test.go | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/.gitattributes b/.gitattributes index 2ee232be3..1c25f9553 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,4 +6,3 @@ vendor/** -text=auto # Diffs on these files are meaningless *.svg -diff -*.pb.go -diff diff --git a/lib/protocol/bep.pb.go b/lib/protocol/bep.pb.go index 3efd29fa7..53f3f6e7e 100644 --- a/lib/protocol/bep.pb.go +++ b/lib/protocol/bep.pb.go @@ -383,7 +383,7 @@ type FileDownloadProgressUpdate struct { UpdateType FileDownloadProgressUpdateType `protobuf:"varint,1,opt,name=update_type,json=updateType,proto3,enum=protocol.FileDownloadProgressUpdateType" json:"update_type,omitempty"` Name string `protobuf:"bytes,2,opt,name=name,proto3" json:"name,omitempty"` Version Vector `protobuf:"bytes,3,opt,name=version" json:"version"` - BlockIndexes []int32 `protobuf:"varint,4,rep,packed,name=block_indexes,json=blockIndexes" json:"block_indexes,omitempty"` + BlockIndexes []int32 `protobuf:"varint,4,rep,name=block_indexes,json=blockIndexes" json:"block_indexes,omitempty"` } func (m *FileDownloadProgressUpdate) Reset() { *m = FileDownloadProgressUpdate{} } @@ -1163,22 +1163,11 @@ func (m *FileDownloadProgressUpdate) MarshalTo(data []byte) (int, error) { } i += n3 if len(m.BlockIndexes) > 0 { - data5 := make([]byte, len(m.BlockIndexes)*10) - var j4 int - for _, num1 := range m.BlockIndexes { - num := uint64(num1) - for num >= 1<<7 { - data5[j4] = uint8(uint64(num)&0x7f | 0x80) - num >>= 7 - j4++ - } - data5[j4] = uint8(num) - j4++ + for _, num := range m.BlockIndexes { + data[i] = 0x20 + i++ + i = encodeVarintBep(data, i, uint64(num)) } - data[i] = 0x22 - i++ - i = encodeVarintBep(data, i, uint64(j4)) - i += copy(data[i:], data5[:j4]) } return i, nil } @@ -1568,11 +1557,9 @@ func (m *FileDownloadProgressUpdate) ProtoSize() (n int) { l = m.Version.ProtoSize() n += 1 + l + sovBep(uint64(l)) if len(m.BlockIndexes) > 0 { - l = 0 for _, e := range m.BlockIndexes { - l += sovBep(uint64(e)) + n += 1 + sovBep(uint64(e)) } - n += 1 + sovBep(uint64(l)) + l } return n } diff --git a/lib/protocol/protocol_test.go b/lib/protocol/protocol_test.go index 778f63494..dc2e35367 100644 --- a/lib/protocol/protocol_test.go +++ b/lib/protocol/protocol_test.go @@ -13,6 +13,8 @@ import ( "testing/quick" "time" + "encoding/hex" + "github.com/syncthing/syncthing/lib/rand" ) @@ -179,6 +181,37 @@ func TestMarshalCloseMessage(t *testing.T) { } } +func TestMarshalFDPU(t *testing.T) { + if testing.Short() { + quickCfg.MaxCount = 10 + } + + f := func(m1 FileDownloadProgressUpdate) bool { + if len(m1.Version.Counters) == 0 { + m1.Version.Counters = nil + } + return testMarshal(t, "close", &m1, &FileDownloadProgressUpdate{}) + } + + if err := quick.Check(f, quickCfg); err != nil { + t.Error(err) + } +} + +func TestUnmarshalFDPUv16v17(t *testing.T) { + var fdpu FileDownloadProgressUpdate + + m0, _ := hex.DecodeString("08cda1e2e3011278f3918787f3b89b8af2958887f0aa9389f3a08588f3aa8f96f39aa8a5f48b9188f19286a0f3848da4f3aba799f3beb489f0a285b9f487b684f2a3bda2f48598b4f2938a89f2a28badf187a0a2f2aebdbdf4849494f4808fbbf2b3a2adf2bb95bff0a6ada4f198ab9af29a9c8bf1abb793f3baabb2f188a6ba1a0020bb9390f60220f6d9e42220b0c7e2b2fdffffffff0120fdb2dfcdfbffffffff0120cedab1d50120bd8784c0feffffffff0120ace99591fdffffffff0120eed7d09af9ffffffff01") + if err := fdpu.Unmarshal(m0); err != nil { + t.Fatal("Unmarshalling message from v0.14.16:", err) + } + + m1, _ := hex.DecodeString("0880f1969905128401f099b192f0abb1b9f3b280aff19e9aa2f3b89e84f484b39df1a7a6b0f1aea4b1f0adac94f3b39caaf1939281f1928a8af0abb1b0f0a8b3b3f3a88e94f2bd85acf29c97a9f2969da6f0b7a188f1908ea2f09a9c9bf19d86a6f29aada8f389bb95f0bf9d88f1a09d89f1b1a4b5f29b9eabf298a59df1b2a589f2979ebdf0b69880f18986b21a440a1508c7d8fb8897ca93d90910e8c4d8e8f2f8f0ccee010a1508afa8ffd8c085b393c50110e5bdedc3bddefe9b0b0a1408a1bedddba4cac5da3c10b8e5d9958ca7e3ec19225ae2f88cb2f8ffffffff018ceda99cfbffffffff01b9c298a407e295e8e9fcffffffff01f3b9ade5fcffffffff01c08bfea9fdffffffff01a2c2e5e1ffffffffff0186dcc5dafdffffffff01e9ffc7e507c9d89db8fdffffffff01") + if err := fdpu.Unmarshal(m1); err != nil { + t.Fatal("Unmarshalling message from v0.14.16:", err) + } +} + func testMarshal(t *testing.T, prefix string, m1, m2 message) bool { buf, err := m1.Marshal() if err != nil { @@ -194,7 +227,7 @@ func testMarshal(t *testing.T, prefix string, m1, m2 message) bool { bs2, _ := json.MarshalIndent(m2, "", " ") if !bytes.Equal(bs1, bs2) { ioutil.WriteFile(prefix+"-1.txt", bs1, 0644) - ioutil.WriteFile(prefix+"-2.txt", bs1, 0644) + ioutil.WriteFile(prefix+"-2.txt", bs2, 0644) return false }