From 65a18ed669d64dc7a6f85baeda0de8e38e7add07 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Wed, 11 Jul 2018 12:24:37 -0300 Subject: [PATCH 1/3] unixfs: fix `dagTruncate` to preserve node type Extract the original `FSNode` passed inside the `ipld.Node` argument and modify its `Blocksizes` (removing all of them and re-adding the ones that were not truncated). In contrast, the replaced code was creating a new `FSNode` that was not preserving some of the features of the original one. Change `TRUNC_HASH` values in `sharness` that were created with the bug to the correct values. License: MIT Signed-off-by: Lucas Molas --- test/sharness/t0250-files-api.sh | 8 ++++---- unixfs/mod/dagmodifier.go | 12 +++++++++--- unixfs/unixfs.go | 6 ++++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/test/sharness/t0250-files-api.sh b/test/sharness/t0250-files-api.sh index 61f52c2bf57..5499ab3d450 100755 --- a/test/sharness/t0250-files-api.sh +++ b/test/sharness/t0250-files-api.sh @@ -613,7 +613,7 @@ tests_for_files_api() { ROOT_HASH=QmcwKfTMCT7AaeiD92hWjnZn9b6eh9NxnhfSzN5x2vnDpt CATS_HASH=Qma88m8ErTGkZHbBWGqy1C7VmEmX8wwNDWNpGyCaNmEgwC FILE_HASH=QmQdQt9qooenjeaNhiKHF3hBvmNteB4MQBtgu3jxgf9c7i - TRUNC_HASH=QmdaQZbLwK5ykweGdCVovNnvBom7QhikovDUVqTPHQG4L8 + TRUNC_HASH=QmPVnT9gocPbqzN4G6SMp8vAPyzcjDbUJrNdKgzQquuDg4 test_files_api "($EXTRA)" test_expect_success "can create some files for testing with raw-leaves ($EXTRA)" ' @@ -629,13 +629,13 @@ tests_for_files_api() { ROOT_HASH=QmW3dMSU6VNd1mEdpk9S3ZYRuR1YwwoXjGaZhkyK6ru9YU CATS_HASH=QmPqWDEg7NoWRX8Y4vvYjZtmdg5umbfsTQ9zwNr12JoLmt FILE_HASH=QmRCgHeoKxCqK2Es6M6nPUDVWz19yNQPnsXGsXeuTkSKpN - TRUNC_HASH=QmRFJEKWF5A5FyFYZgNhusLw2UziW9zBKYr4huyHjzcB6o + TRUNC_HASH=QmckstrVxJuecVD1FHUiURJiU9aPURZWJieeBVHJPACj8L test_files_api "($EXTRA, raw-leaves)" '' --raw-leaves ROOT_HASH=QmageRWxC7wWjPv5p36NeAgBAiFdBHaNfxAehBSwzNech2 CATS_HASH=zdj7WkEzPLNAr5TYJSQC8CFcBjLvWFfGdx6kaBrJXnBguwWeX FILE_HASH=zdj7WYHvf5sBRgSBjYnq64QFr449CCbgupXfBvoYL3aHC1DzJ - TRUNC_HASH=zdj7WYLYbka6Ydg8gZUJRLKnFBVehCADhQKBsFbNiMxZSB5Gj + TRUNC_HASH=zdj7Wjr8GHZonPFVCWvz2SLLo9H6MmqBxyeB34ArHfyCbmdJG if [ "$EXTRA" = "offline" ]; then test_files_api "($EXTRA, cidv1)" --cid-version=1 fi @@ -660,7 +660,7 @@ tests_for_files_api() { ROOT_HASH=zDMZof1kxEsAwSgCZsGQRVcHCMtHLjkUQoiZUbZ87erpPQJGUeW8 CATS_HASH=zDMZof1kuAhr3zBkxq48V7o9HJZCTVyu1Wd9wnZtVcPJLW8xnGft FILE_HASH=zDMZof1kxbB9CvxgRioBzESbGnZUxtSCsZ18H1EUkxDdWt1DYEkK - TRUNC_HASH=zDMZof1kxXqKdVsVo231qVdN3hCTF5a34UuQZpzmm5K7CbRJ4u2S + TRUNC_HASH=zDMZof1kpH1vxK3k2TeYc8w59atCbzMzrhZonsztMWSptVro2zQa test_files_api "($EXTRA, blake2b-256 root)" fi diff --git a/unixfs/mod/dagmodifier.go b/unixfs/mod/dagmodifier.go index 28de7228d61..f8aa5ce6367 100644 --- a/unixfs/mod/dagmodifier.go +++ b/unixfs/mod/dagmodifier.go @@ -529,7 +529,13 @@ func dagTruncate(ctx context.Context, n ipld.Node, size uint64, ds ipld.DAGServi var cur uint64 end := 0 var modified ipld.Node - ndata := ft.NewFSNode(ft.TRaw) + ndata, err := ft.FSNodeFromBytes(nd.Data()) + if err != nil { + return nil, err + } + // Reset the block sizes of the node to adjust them + // with the new values of the truncated children. + ndata.RemoveAllBlockSizes() for i, lnk := range nd.Links() { child, err := lnk.GetNode(ctx, ds) if err != nil { @@ -558,7 +564,7 @@ func dagTruncate(ctx context.Context, n ipld.Node, size uint64, ds ipld.DAGServi ndata.AddBlockSize(childsize) } - err := ds.Add(ctx, modified) + err = ds.Add(ctx, modified) if err != nil { return nil, err } @@ -573,7 +579,7 @@ func dagTruncate(ctx context.Context, n ipld.Node, size uint64, ds ipld.DAGServi if err != nil { return nil, err } - + // Save the new block sizes to the original node. nd.SetData(d) // invalidate cache and recompute serialized data diff --git a/unixfs/unixfs.go b/unixfs/unixfs.go index 9cd9731ed9d..d048c422b83 100644 --- a/unixfs/unixfs.go +++ b/unixfs/unixfs.go @@ -201,6 +201,12 @@ func (n *FSNode) BlockSize(i int) uint64 { return n.format.Blocksizes[i] } +// RemoveAllBlockSizes removes all the child block sizes of this node. +func (n *FSNode) RemoveAllBlockSizes() { + n.format.Blocksizes = []uint64{} + n.format.Filesize = proto.Uint64(uint64(len(n.Data()))) +} + // GetBytes marshals this node as a protobuf message. func (n *FSNode) GetBytes() ([]byte, error) { return proto.Marshal(&n.format) From 508af1e12c72ef5ad8d3f8757a0db4667e9197b0 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Wed, 11 Jul 2018 12:36:10 -0300 Subject: [PATCH 2/3] mfs: add test case for MFS repeated truncation failure License: MIT Signed-off-by: Lucas Molas --- mfs/mfs_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/mfs/mfs_test.go b/mfs/mfs_test.go index 42a87fde46d..c91d77168b2 100644 --- a/mfs/mfs_test.go +++ b/mfs/mfs_test.go @@ -1136,3 +1136,43 @@ func TestTruncateAtSize(t *testing.T) { } fd.Truncate(4) } + +func TestTruncateAndWrite(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ds, rt := setupRoot(ctx, t) + + dir := rt.GetDirectory() + + nd := dag.NodeWithData(ft.FilePBData(nil, 0)) + fi, err := NewFile("test", nd, dir, ds) + if err != nil { + t.Fatal(err) + } + + fd, err := fi.Open(OpenReadWrite, true) + defer fd.Close() + if err != nil { + t.Fatal(err) + } + for i := 0; i < 200; i++ { + err = fd.Truncate(0) + if err != nil { + t.Fatal(err) + } + l, err := fd.Write([]byte("test")) + if err != nil { + t.Fatal(err) + } + if l != len("test") { + t.Fatal("incorrect write length") + } + data, err := ioutil.ReadAll(fd) + if err != nil { + t.Fatal(err) + } + if string(data) != "test" { + t.Errorf("read error at read %d, read: %v", i, data) + } + } +} From 70d0f13c9caa6ce618f7260fde84bbba9d50173c Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Wed, 18 Jul 2018 11:07:52 -0300 Subject: [PATCH 3/3] unixfs/mod: add test to `Truncate` to the same size License: MIT Signed-off-by: Lucas Molas --- unixfs/mod/dagmodifier_test.go | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/unixfs/mod/dagmodifier_test.go b/unixfs/mod/dagmodifier_test.go index 92e4ce2d6e8..0a5c5a74f33 100644 --- a/unixfs/mod/dagmodifier_test.go +++ b/unixfs/mod/dagmodifier_test.go @@ -406,6 +406,44 @@ func testDagTruncate(t *testing.T, opts testu.NodeOpts) { } } +// TestDagTruncateSameSize tests that a DAG truncated +// to the same size (i.e., doing nothing) doesn't modify +// the DAG (its hash). +func TestDagTruncateSameSize(t *testing.T) { + runAllSubtests(t, testDagTruncateSameSize) +} +func testDagTruncateSameSize(t *testing.T, opts testu.NodeOpts) { + dserv := testu.GetDAGServ() + _, n := testu.GetRandomNode(t, dserv, 50000, opts) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + dagmod, err := NewDagModifier(ctx, n, dserv, testu.SizeSplitterGen(512)) + if err != nil { + t.Fatal(err) + } + // Copied from `TestDagTruncate`. + + size, err := dagmod.Size() + if err != nil { + t.Fatal(err) + } + + err = dagmod.Truncate(size) + if err != nil { + t.Fatal(err) + } + + modifiedNode, err := dagmod.GetNode() + if err != nil { + t.Fatal(err) + } + + if modifiedNode.Cid().Equals(n.Cid()) == false { + t.Fatal("the node has been modified!") + } +} + func TestSparseWrite(t *testing.T) { runAllSubtests(t, testSparseWrite) }