From 08502cfa6152bec7fa91379edba0f4edda204b1e Mon Sep 17 00:00:00 2001
From: Hans Hasselberg <me@hans.io>
Date: Tue, 8 Jan 2019 17:06:28 +0100
Subject: [PATCH] snapshot: read meta.json correctly. (#5193)

* snapshot: read meta.json correctly.

Fixes #4452.
---
 snapshot/archive.go           | 16 ++++++++++++++--
 snapshot/archive_test.go      | 19 +++++++++++++++++++
 test/snapshot/spaces-meta.tar |  3 +++
 3 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 test/snapshot/spaces-meta.tar

diff --git a/snapshot/archive.go b/snapshot/archive.go
index b6db126a8..4f2f3dbdc 100644
--- a/snapshot/archive.go
+++ b/snapshot/archive.go
@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"hash"
 	"io"
+	"io/ioutil"
 	"time"
 
 	"github.com/hashicorp/raft"
@@ -193,8 +194,19 @@ func read(in io.Reader, metadata *raft.SnapshotMeta, snap io.Writer) error {
 
 		switch hdr.Name {
 		case "meta.json":
-			dec := json.NewDecoder(io.TeeReader(archive, metaHash))
-			if err := dec.Decode(&metadata); err != nil {
+			// Previously we used json.Decode to decode the archive stream. There are
+			// edgecases in which it doesn't read all the bytes from the stream, even
+			// though the json object is still being parsed properly. Since we
+			// simutaniously feeded everything to metaHash, our hash ended up being
+			// different than what we calculated when creating the snapshot. Which in
+			// turn made the snapshot verification fail. By explicitly reading the
+			// whole thing first we ensure that we calculate the correct hash
+			// independent of how json.Decode works internally.
+			buf, err := ioutil.ReadAll(io.TeeReader(archive, metaHash))
+			if err != nil {
+				return fmt.Errorf("failed to read snapshot metadata: %v", err)
+			}
+			if err := json.Unmarshal(buf, &metadata); err != nil {
 				return fmt.Errorf("failed to decode snapshot metadata: %v", err)
 			}
 
diff --git a/snapshot/archive_test.go b/snapshot/archive_test.go
index 9d07e32dc..e41802ab3 100644
--- a/snapshot/archive_test.go
+++ b/snapshot/archive_test.go
@@ -63,6 +63,25 @@ func TestArchive(t *testing.T) {
 	}
 }
 
+func TestArchive_GoodData(t *testing.T) {
+	paths := []string{
+		"../test/snapshot/spaces-meta.tar",
+	}
+	for i, p := range paths {
+		f, err := os.Open(p)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		defer f.Close()
+
+		var metadata raft.SnapshotMeta
+		err = read(f, &metadata, ioutil.Discard)
+		if err != nil {
+			t.Fatalf("case %d: should've read the snapshot, but didn't: %v", i, err)
+		}
+	}
+}
+
 func TestArchive_BadData(t *testing.T) {
 	cases := []struct {
 		Name  string
diff --git a/test/snapshot/spaces-meta.tar b/test/snapshot/spaces-meta.tar
new file mode 100644
index 000000000..6a5989e5a
--- /dev/null
+++ b/test/snapshot/spaces-meta.tar
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:4ea983be1745568bb5ad9c4d10eb64ce566cb78dde4ca870093e0c48e93d8b07
+size 10240