From 8a7873ee3ad2865dbc8630f75f241795ba44b0ba Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 18:45:16 +0200
Subject: [PATCH 1/8] Handle invalid subtree IDs

---
 checker/checker.go | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/checker/checker.go b/checker/checker.go
index d8ac497d6..438577ffb 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -321,7 +321,7 @@ func loadTreeWorker(repo *repository.Repository,
 			debug.Log("checker.loadTreeWorker", "load tree %v", treeID.Str())
 
 			tree, err := restic.LoadTree(repo, treeID)
-			debug.Log("checker.loadTreeWorker", "load tree %v (%v) returned err %v", tree, treeID.Str(), err)
+			debug.Log("checker.loadTreeWorker", "load tree %v (%v) returned err: %v", tree, treeID.Str(), err)
 			job = treeJob{ID: treeID, error: err, Tree: tree}
 			outCh = out
 			inCh = nil
@@ -437,9 +437,31 @@ func filterTrees(backlog backend.IDs, loaderChan chan<- backend.ID, in <-chan tr
 			}
 
 			outstandingLoadTreeJobs--
-			debug.Log("checker.filterTrees", "input job tree %v", j.ID.Str())
 
-			backlog = append(backlog, j.Tree.Subtrees()...)
+			debug.Log("checker.filterTrees", "input job tree %v, subtrees %v", j.ID.Str(), j.Tree.Subtrees())
+
+			var err error
+
+			if j.error != nil {
+				debug.Log("checker.filterTrees", "received job with error: %v (tree %v, ID %v)", j.error, j.Tree, j.ID.Str())
+			} else if j.Tree == nil {
+				debug.Log("checker.filterTrees", "received job with nil tree pointer: %v (ID %v)", j.error, j.ID.Str())
+				err = errors.New("tree is nil and error is nil")
+			} else {
+				for _, id := range j.Tree.Subtrees() {
+					if id.IsNull() {
+						debug.Log("checker.filterTrees", "tree %v has nil subtree", j.ID.Str())
+						err = fmt.Errorf("tree %v has subtree with null ID", j.ID)
+						continue
+					}
+					backlog = append(backlog, id)
+				}
+			}
+
+			if err != nil {
+				// send a new job with the new error instead of the old one
+				j = treeJob{ID: j.ID, error: err}
+			}
 
 			job = j
 			outCh = out

From 72fcd008594256a4f8a17afef0fe2c5e37f2ced6 Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 18:46:26 +0200
Subject: [PATCH 2/8] Check subtrees with null ID

---
 checker/checker.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/checker/checker.go b/checker/checker.go
index 438577ffb..73d59b408 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -522,6 +522,11 @@ func (c *Checker) checkTree(id backend.ID, tree *restic.Tree) (errs []error) {
 				errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("node %d is dir but has no subtree", i)})
 				continue
 			}
+
+			if node.Subtree.IsNull() {
+				errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("node %d is dir subtree id is null", i)})
+				continue
+			}
 		}
 	}
 

From 86c8328f62e84fb8c46b422e9cd4819c7c07e86e Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 19:13:35 +0200
Subject: [PATCH 3/8] Handle null subtree IDs

---
 checker/checker.go | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/checker/checker.go b/checker/checker.go
index 73d59b408..845d919c9 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -276,7 +276,7 @@ func loadSnapshotTreeIDs(repo *repository.Repository) (backend.IDs, []error) {
 	return trees.IDs, errs.errs
 }
 
-// TreeError is returned when loading a tree from the repository failed.
+// TreeError collects several errors that occurred while processing a tree.
 type TreeError struct {
 	ID     backend.ID
 	Errors []error
@@ -335,7 +335,7 @@ func loadTreeWorker(repo *repository.Repository,
 }
 
 // checkTreeWorker checks the trees received and sends out errors to errChan.
-func (c *Checker) checkTreeWorker(in <-chan treeJob, out chan<- TreeError, done <-chan struct{}, wg *sync.WaitGroup) {
+func (c *Checker) checkTreeWorker(in <-chan treeJob, out chan<- error, done <-chan struct{}, wg *sync.WaitGroup) {
 	defer func() {
 		debug.Log("checker.checkTreeWorker", "exiting")
 		wg.Done()
@@ -351,10 +351,12 @@ func (c *Checker) checkTreeWorker(in <-chan treeJob, out chan<- TreeError, done
 	for {
 		select {
 		case <-done:
+			debug.Log("checker.checkTreeWorker", "done channel closed, exiting")
 			return
 
 		case job, ok := <-inCh:
 			if !ok {
+				debug.Log("checker.checkTreeWorker", "input channel closed, exiting")
 				return
 			}
 
@@ -372,9 +374,15 @@ func (c *Checker) checkTreeWorker(in <-chan treeJob, out chan<- TreeError, done
 				continue
 			}
 
-			debug.Log("checker.checkTreeWorker", "load tree %v", job.ID.Str())
+			debug.Log("checker.checkTreeWorker", "check tree %v (tree %v, err %v)", job.ID.Str(), job.Tree, job.error)
+
+			var errs []error
+			if job.error != nil {
+				errs = append(errs, job.error)
+			} else {
+				errs = c.checkTree(job.ID, job.Tree)
+			}
 
-			errs := c.checkTree(job.ID, job.Tree)
 			if len(errs) > 0 {
 				debug.Log("checker.checkTreeWorker", "checked tree %v: %v errors", job.ID.Str(), len(errs))
 				treeError = TreeError{ID: job.ID, Errors: errs}
@@ -438,7 +446,7 @@ func filterTrees(backlog backend.IDs, loaderChan chan<- backend.ID, in <-chan tr
 
 			outstandingLoadTreeJobs--
 
-			debug.Log("checker.filterTrees", "input job tree %v, subtrees %v", j.ID.Str(), j.Tree.Subtrees())
+			debug.Log("checker.filterTrees", "input job tree %v", j.ID.Str())
 
 			var err error
 
@@ -448,10 +456,13 @@ func filterTrees(backlog backend.IDs, loaderChan chan<- backend.ID, in <-chan tr
 				debug.Log("checker.filterTrees", "received job with nil tree pointer: %v (ID %v)", j.error, j.ID.Str())
 				err = errors.New("tree is nil and error is nil")
 			} else {
+				debug.Log("checker.filterTrees", "subtrees for tree %v: %v", j.ID.Str(), j.Tree.Subtrees())
 				for _, id := range j.Tree.Subtrees() {
 					if id.IsNull() {
+						// We do not need to raise this error here, it is
+						// checked when the tree is checked. Just make sure
+						// that we do not add any null IDs to the backlog.
 						debug.Log("checker.filterTrees", "tree %v has nil subtree", j.ID.Str())
-						err = fmt.Errorf("tree %v has subtree with null ID", j.ID)
 						continue
 					}
 					backlog = append(backlog, id)

From db85ab8aa0bae6385d72f429ba3550ba3094daee Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 19:13:45 +0200
Subject: [PATCH 4/8] Use the correct channel for sending errors

---
 checker/checker.go      | 3 +--
 cmd/restic/cmd_check.go | 9 ++++++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/checker/checker.go b/checker/checker.go
index 845d919c9..dc2d88c8c 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -505,13 +505,12 @@ func (c *Checker) Structure(errChan chan<- error, done <-chan struct{}) {
 	treeIDChan := make(chan backend.ID)
 	treeJobChan1 := make(chan treeJob)
 	treeJobChan2 := make(chan treeJob)
-	treeErrChan := make(chan TreeError)
 
 	var wg sync.WaitGroup
 	for i := 0; i < defaultParallelism; i++ {
 		wg.Add(2)
 		go loadTreeWorker(c.repo, treeIDChan, treeJobChan1, done, &wg)
-		go c.checkTreeWorker(treeJobChan2, treeErrChan, done, &wg)
+		go c.checkTreeWorker(treeJobChan2, errChan, done, &wg)
 	}
 
 	filterTrees(trees, treeIDChan, treeJobChan1, treeJobChan2, done)
diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go
index d47de1c7b..0d5993845 100644
--- a/cmd/restic/cmd_check.go
+++ b/cmd/restic/cmd_check.go
@@ -79,7 +79,14 @@ func (cmd CmdCheck) Execute(args []string) error {
 
 	for err := range errChan {
 		errorsFound = true
-		fmt.Fprintf(os.Stderr, "error: %v\n", err)
+		if e, ok := err.(checker.TreeError); ok {
+			fmt.Fprintf(os.Stderr, "error for tree %v:\n", e.ID.Str())
+			for _, treeErr := range e.Errors {
+				fmt.Fprintf(os.Stderr, "  %v\n", treeErr)
+			}
+		} else {
+			fmt.Fprintf(os.Stderr, "error: %v\n", err)
+		}
 	}
 
 	for _, id := range chkr.UnusedBlobs() {

From 7db2369081ba37baf820ce5927057fcfac0b0ac2 Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 19:25:02 +0200
Subject: [PATCH 5/8] Shorten error message for tree errors

---
 checker/checker.go  | 6 +++---
 repository/index.go | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/checker/checker.go b/checker/checker.go
index dc2d88c8c..7c6f2dad9 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -204,14 +204,14 @@ type Error struct {
 
 func (e Error) Error() string {
 	if e.BlobID != nil && e.TreeID != nil {
-		msg := "tree " + e.TreeID.String()
-		msg += ", blob " + e.BlobID.String()
+		msg := "tree " + e.TreeID.Str()
+		msg += ", blob " + e.BlobID.Str()
 		msg += ": " + e.Err.Error()
 		return msg
 	}
 
 	if e.TreeID != nil {
-		return "tree " + e.TreeID.String() + ": " + e.Err.Error()
+		return "tree " + e.TreeID.Str() + ": " + e.Err.Error()
 	}
 
 	return e.Err.Error()
diff --git a/repository/index.go b/repository/index.go
index 0611ced6b..39152e6a5 100644
--- a/repository/index.go
+++ b/repository/index.go
@@ -219,7 +219,7 @@ func (idx *Index) generatePackList(selectFn func(indexEntry) bool) ([]*packJSON,
 			continue
 		}
 
-		debug.Log("Index.generatePackList", "handle blob %q", id[:8])
+		debug.Log("Index.generatePackList", "handle blob %v", id.Str())
 
 		if blob.packID.IsNull() {
 			debug.Log("Index.generatePackList", "blob %q has no packID! (type %v, offset %v, length %v)",

From f188cf81dc722630effc8bc917f6d8ece77c4e52 Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 20:45:42 +0200
Subject: [PATCH 6/8] Add more panic() calls for invalid conditions

---
 archiver.go         | 7 +++++++
 repository/index.go | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/archiver.go b/archiver.go
index 5621345dd..c472a0eeb 100644
--- a/archiver.go
+++ b/archiver.go
@@ -334,6 +334,10 @@ func (arch *Archiver) dirWorker(wg *sync.WaitGroup, p *Progress, done <-chan str
 
 				if node.Type == "dir" {
 					debug.Log("Archiver.dirWorker", "got tree node for %s: %v", node.path, node.blobs)
+
+					if node.Subtree.IsNull() {
+						panic("invalid null subtree ID")
+					}
 				}
 			}
 
@@ -359,6 +363,9 @@ func (arch *Archiver) dirWorker(wg *sync.WaitGroup, p *Progress, done <-chan str
 				panic(err)
 			}
 			debug.Log("Archiver.dirWorker", "save tree for %s: %v", dir.Path(), id.Str())
+			if id.IsNull() {
+				panic("invalid null subtree ID return from SaveTreeJSON()")
+			}
 
 			node.Subtree = &id
 
diff --git a/repository/index.go b/repository/index.go
index 39152e6a5..863388b60 100644
--- a/repository/index.go
+++ b/repository/index.go
@@ -215,6 +215,10 @@ func (idx *Index) generatePackList(selectFn func(indexEntry) bool) ([]*packJSON,
 	packs := make(map[backend.ID]*packJSON)
 
 	for id, blob := range idx.pack {
+		if blob.packID == nil {
+			panic("nil pack id")
+		}
+
 		if selectFn != nil && !selectFn(blob) {
 			continue
 		}

From cc7acba02b60d1bd82f0f8a26c319f94b226eec2 Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 20:45:50 +0200
Subject: [PATCH 7/8] Return the original backend ID on duplicate entries

---
 repository/repository.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repository/repository.go b/repository/repository.go
index c6900f43f..57373cd2e 100644
--- a/repository/repository.go
+++ b/repository/repository.go
@@ -315,7 +315,7 @@ func (r *Repository) SaveAndEncrypt(t pack.BlobType, data []byte, id *backend.ID
 	err = r.idx.StoreInProgress(t, *id)
 	if err != nil {
 		debug.Log("Repo.Save", "another goroutine is already working on %v (%v) does already exist", id.Str, t)
-		return backend.ID{}, nil
+		return *id, nil
 	}
 
 	// find suitable packer and add blob

From 1020e9c3af1cd9d780af64f2c624bca19f2562d7 Mon Sep 17 00:00:00 2001
From: Alexander Neumann <alexander@bumpern.de>
Date: Sun, 11 Oct 2015 20:55:28 +0200
Subject: [PATCH 8/8] Check for data blobs with null ID, improve errors

---
 checker/checker.go | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/checker/checker.go b/checker/checker.go
index 7c6f2dad9..ec5f1da9a 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -523,18 +523,24 @@ func (c *Checker) checkTree(id backend.ID, tree *restic.Tree) (errs []error) {
 
 	var blobs []backend.ID
 
-	for i, node := range tree.Nodes {
+	for _, node := range tree.Nodes {
 		switch node.Type {
 		case "file":
-			blobs = append(blobs, node.Content...)
+			for b, blobID := range node.Content {
+				if blobID.IsNull() {
+					errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("file %q blob %d has null ID", node.Name, b)})
+					continue
+				}
+				blobs = append(blobs, blobID)
+			}
 		case "dir":
 			if node.Subtree == nil {
-				errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("node %d is dir but has no subtree", i)})
+				errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("dir node %q has no subtree", node.Name)})
 				continue
 			}
 
 			if node.Subtree.IsNull() {
-				errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("node %d is dir subtree id is null", i)})
+				errs = append(errs, Error{TreeID: &id, Err: fmt.Errorf("dir node %q subtree id is null", node.Name)})
 				continue
 			}
 		}