From 06f5253f5b7772c0db514ab3d185acac9ba9622e Mon Sep 17 00:00:00 2001
From: Peter Rabbitson <ribasushi@protocol.ai>
Date: Mon, 4 Oct 2021 23:16:02 +0200
Subject: [PATCH] Error on corner cases with corresponding negative tests

---
 itests/deals_partial_retrieval_test.go | 48 ++++++++++++++++++++++----
 node/impl/client/client.go             | 10 ++++--
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/itests/deals_partial_retrieval_test.go b/itests/deals_partial_retrieval_test.go
index d60b137ca10..926d9ad28b2 100644
--- a/itests/deals_partial_retrieval_test.go
+++ b/itests/deals_partial_retrieval_test.go
@@ -25,12 +25,14 @@ import (
 // use the mainnet carfile as text fixture: it will always be here
 // https://dweb.link/ipfs/bafy2bzacecnamqgqmifpluoeldx7zzglxcljo6oja4vrmtj7432rphldpdmm2/8/1/8/1/0/1/0
 var (
-	sourceCar      = "../build/genesis/mainnet.car"
-	carRoot, _     = cid.Parse("bafy2bzacecnamqgqmifpluoeldx7zzglxcljo6oja4vrmtj7432rphldpdmm2")
-	carCommp, _    = cid.Parse("baga6ea4seaqmrivgzei3fmx5qxtppwankmtou6zvigyjaveu3z2zzwhysgzuina")
-	carPieceSize   = abi.PaddedPieceSize(2097152)
-	textSelector   = textselector.Expression("8/1/8/1/0/1/0")
-	expectedResult = "fil/1/storagepower"
+	sourceCar               = "../build/genesis/mainnet.car"
+	carRoot, _              = cid.Parse("bafy2bzacecnamqgqmifpluoeldx7zzglxcljo6oja4vrmtj7432rphldpdmm2")
+	carCommp, _             = cid.Parse("baga6ea4seaqmrivgzei3fmx5qxtppwankmtou6zvigyjaveu3z2zzwhysgzuina")
+	carPieceSize            = abi.PaddedPieceSize(2097152)
+	textSelector            = textselector.Expression("8/1/8/1/0/1/0")
+	textSelectorNonLink     = textselector.Expression("8/1/8/1/0/1")
+	textSelectorNonexistent = textselector.Expression("42")
+	expectedResult          = "fil/1/storagepower"
 )
 
 func TestPartialRetrieval(t *testing.T) {
@@ -121,6 +123,40 @@ func TestPartialRetrieval(t *testing.T) {
 			time.Sleep(time.Second)
 		}
 	}
+
+	// ensure non-existent paths fail
+	require.EqualError(
+		t,
+		testGenesisRetrieval(
+			ctx,
+			client,
+			api.RetrievalOrder{
+				FromLocalCAR:          sourceCar,
+				Root:                  carRoot,
+				DatamodelPathSelector: &textSelectorNonexistent,
+			},
+			&api.FileRef{},
+			nil,
+		),
+		fmt.Sprintf("retrieval failed: path selection '%s' does not match a node within %s", textSelectorNonexistent, carRoot),
+	)
+
+	// ensure non-boundary retrievals fail
+	require.EqualError(
+		t,
+		testGenesisRetrieval(
+			ctx,
+			client,
+			api.RetrievalOrder{
+				FromLocalCAR:          sourceCar,
+				Root:                  carRoot,
+				DatamodelPathSelector: &textSelectorNonLink,
+			},
+			&api.FileRef{},
+			nil,
+		),
+		fmt.Sprintf("retrieval failed: error while locating partial retrieval sub-root: unsupported selection path '%s' does not correspond to a node boundary (a.k.a. CID link)", textSelectorNonLink),
+	)
 }
 
 func testGenesisRetrieval(ctx context.Context, client *kit.TestFullNode, retOrder api.RetrievalOrder, retRef *api.FileRef, outFile *os.File) error {
diff --git a/node/impl/client/client.go b/node/impl/client/client.go
index 2971d887248..09f52280b0d 100644
--- a/node/impl/client/client.go
+++ b/node/impl/client/client.go
@@ -1039,22 +1039,28 @@ func (a *API) clientRetrieve(ctx context.Context, order api.RetrievalOrder, ref
 			selspec.Node(),
 			func(p traversal.Progress, n ipld.Node, r traversal.VisitReason) error {
 				if r == traversal.VisitReason_SelectionMatch {
+
+					if p.LastBlock.Path.String() != p.Path.String() {
+						return xerrors.Errorf("unsupported selection path '%s' does not correspond to a node boundary (a.k.a. CID link)", p.Path.String())
+					}
+
 					cidLnk, castOK := p.LastBlock.Link.(cidlink.Link)
 					if !castOK {
 						return xerrors.Errorf("cidlink cast unexpectedly failed on '%s'", p.LastBlock.Link.String())
 					}
+
 					root = cidLnk.Cid
 					subRootFound = true
 				}
 				return nil
 			},
 		); err != nil {
-			finish(xerrors.Errorf("Finding partial retrieval sub-root: %w", err))
+			finish(xerrors.Errorf("error while locating partial retrieval sub-root: %w", err))
 			return
 		}
 
 		if !subRootFound {
-			finish(xerrors.Errorf("Path selection '%s' does not match a node within %s", order.DatamodelPathSelector, root))
+			finish(xerrors.Errorf("path selection '%s' does not match a node within %s", *order.DatamodelPathSelector, root))
 			return
 		}
 	}