From 0fc1c203cc24a1522146de470afaff85a6d82d2f Mon Sep 17 00:00:00 2001 From: Hans Hasselberg 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 | Bin 0 -> 10240 bytes 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 test/snapshot/spaces-meta.tar diff --git a/snapshot/archive.go b/snapshot/archive.go index b6db126a8a..4f2f3dbdc1 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 9d07e32dc1..e41802ab34 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 0000000000000000000000000000000000000000..abd107f8a80045c5262810282830c4fb4e0855be GIT binary patch literal 10240 zcmeHL%ah|q8FwII6Q>CL0UTT|ZoA&qTXIu5$dYEp*^xcAD99`@@_7RfZB$X5#sA0JBIB?4qiVOb%m*tb>d2K>iiW(}ZvgV=gufJD+ zU-z%S){w;bWSzyXsSf>oFLG=$gnKO6=B4; z$P{>}8%2YJ!y;mC6gLQng1`WCQ@wAFb%mR(_cfue>Jwdl^;Hm40^fF>afsv8#OuWt z33pSe??HQP+aY#H5rNj!w8;wa!;%tV%5163aYBh_h*h%avdME+m&TX{t+Hi>BuY%v zjChlR9~wz4MHS!p{u@u(JoWoVvE|Od635e0Vfma!;Env3VUaFo}RuFlNMb-vg996bxPU>R9SRx~^Jm5IX3Y=)0 z6qdR?V{)`WFhfL>VN8~YB9<&hWC{g1i(_)L>-tMrI#1D+w$37cg;qX)|I?Jf38(v? z7kDGjNe~W75pjl~dVZ7?iLObG z-)#-tQNrFHSpW_5Ot`dL*P7tBphau+`sMm z*8Df?fr&kDuSpy?fIWpWK3C3~ma<-CizYogoLgrJU1!v|sI^SOhwIC+Z{IEWbN8|X{vIASHfIV1X z1?*b|h`!lNmeBo$hmh5Wi)Nb2(@q-x!%X|pC=31<6$UPC{L7%QfPt5O^1k8`?2kt( zBq5nE+(LhUF`>_Yth!MWnD)pCAy2vsuOz!CA0`k9+X9d;HhOUL>;<0nr;=p zJzkiHRghfE6Pd5SeJGm=LR|9q#SgS8EF&L#MhG7<)L@8RAJCWQleN6uJQ~P zOI<98C9~U=x`buQgqSIAe@CE@WFvt`T@hgmaSXyDvP{H7gGUUFL=IUtVt|c80)rSH zVH?pbP#&jCct|W4kVzq799;sH7x$B;^Z~>S@V%hOg=P@Mg@g~#KbDg{@!oSV<5&Lv z^YJf|v*^i_`!G-;b`X+m>h4*JZ8)V?hzie|IWxdSS@s2X8R7n1{ZfC_Q52_Zf zLtrKo$I&=l=2m68ZK)Hja%cV+=o>j!%^V>AS2zHS5) z-3S;EaO=jX2h6Sx%&7qMfv>j>%C}YE>5sg;4%$JG0*h>|U)f3BZ0TBOR_Qs<;8Ff* zm;PIi@=v>T*(3Q@7C23-t1{E;H)rOmH{2e!h4npYT4OqFURT@G?G`oBXN|peL)+^Z zX51NSOl^EjMl9-HiDq|3poqU@};bgrC-)uhOjDeGE8sj4z1 zm}71l`$la?^StirmaU7g)&1*cyBesh%WM7HHD=gp_hh$Xm$x?Ee8p|B-YrvTR{9)f zytN(Cq>Z4{ABMr8Q>9kha&L`?PjvZo>u5Fg)7$BtW{}**EI3ExINg)UMoph5Pu>9- zbFo(&54$A1_ORtT$&s9F()oi$=Qi1!zjNM(O7XdGu^eJq#9}0H37|lOrIA2iZcr#; z%Z7xMnMFb-1^>l#2pSoMN34|c@JL9RL19J#C7Bcp^lS($0zX)XZf2s4Soj57U^H_> zIJE`L=}_WcW&$W@KA+h-9EdF$e7^tBYC ze?W-|_L>s12(ksl!FNspG35le%O>hdhz7(4x`a#ySv)d%#G6R85oIEZMYf1c;1m&$ z6KprZDG6bW7#3lIB>K_QFh0L{eqDR85xTpu1|XH6zmSe5&l7`>wZr43wt!(-`yOB+qQlHyx^oD$`kB2omxVY%$iMAHB z8=peOX9dOlzeB|{P;6XW@Sin@#*gkR9@MZTOur{QO9i{QrrvqD zPOzPvJBG1-S7dtcrl%GZ&Kqz*1=ayW3)H21+Y1*F?uC1B3*CiFbhx;5e)G;<0J}po z`REZHY+`RrUdm|i!xcNZt(Qz(>W*Cx?#9{pwQT&+h4Byy)`j$o+30}}UojS>d`5aV w%D!^k1VN~hFB!YH7&~?0^x$2-ml0S-U>SjB1eOt4Mqn9%WdxQH_}?P%FB)|65&!@I literal 0 HcmV?d00001