From 494cc8312cff496bab3653d5b2ab1f00ac43a40c Mon Sep 17 00:00:00 2001
From: Peter Amstutz <pamstutz@veritasgenetics.com>
Date: Thu, 20 Sep 2018 10:46:11 -0400
Subject: [PATCH] 14242: Add a test for manifest_text PDH checking

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>
---
 lib/controller/federation.go      |  8 +++--
 lib/controller/federation_test.go | 56 +++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index 3715edae9a..4cbbe465d4 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -159,15 +159,19 @@ func (rw rewriteSignaturesClusterId) rewriteSignatures(resp *http.Response, requ
 		sz += n
 	}
 
-	// Certify that the computed hash of the manifest matches our expectation
+	// Check that expected hash is consistent with
+	// portable_data_hash field of the returned record
 	if rw.expectHash == "" {
 		rw.expectHash = col.PortableDataHash
+	} else if rw.expectHash != col.PortableDataHash {
+		return nil, fmt.Errorf("portable_data_hash %q on returned record did not match expected hash %q ", rw.expectHash, col.PortableDataHash)
 	}
 
+	// Certify that the computed hash of the manifest_text matches our expectation
 	sum := hasher.Sum(nil)
 	computedHash := fmt.Sprintf("%x+%v", sum, sz)
 	if computedHash != rw.expectHash {
-		return nil, fmt.Errorf("Computed hash %q did not match expected hash %q", computedHash, rw.expectHash)
+		return nil, fmt.Errorf("Computed manifest_text hash %q did not match expected hash %q", computedHash, rw.expectHash)
 	}
 
 	col.ManifestText = updatedManifest.String()
diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index 1d113065ee..52906ead32 100644
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -439,6 +439,62 @@ func (s *FederationSuite) TestGetCollectionByPDHError(c *check.C) {
 	c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
 }
 
+func (s *FederationSuite) TestGetCollectionByPDHErrorBadHash(c *check.C) {
+	srv := &httpserver.Server{
+		Server: http.Server{
+			Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+				w.WriteHeader(404)
+			}),
+		},
+	}
+
+	c.Assert(srv.Start(), check.IsNil)
+	defer srv.Close()
+
+	// Make the "local" cluster to a service that always returns 404
+	np := arvados.NodeProfile{
+		Controller: arvados.SystemServiceInstance{Listen: ":"},
+		RailsAPI: arvados.SystemServiceInstance{Listen: srv.Addr,
+			TLS: false, Insecure: true}}
+	s.testHandler.Cluster.NodeProfiles["*"] = np
+	s.testHandler.NodeProfile = &np
+
+	srv2 := &httpserver.Server{
+		Server: http.Server{
+			Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+				w.WriteHeader(200)
+				// Return a collection where the hash
+				// of the manifest text doesn't match
+				// PDH that was requested.
+				var col arvados.Collection
+				col.PortableDataHash = "99999999999999999999999999999999+99"
+				col.ManifestText = `. 6a4ff0499484c6c79c95cd8c566bd25f\+249025 0:249025:GNU_General_Public_License,_version_3.pdf
+`
+				enc := json.NewEncoder(w)
+				enc.Encode(col)
+			}),
+		},
+	}
+
+	c.Assert(srv2.Start(), check.IsNil)
+	defer srv2.Close()
+
+	// Direct zzzzz to service that returns a 200 result with a bogus manifest_text
+	s.testHandler.Cluster.RemoteClusters["zzzzz"] = arvados.RemoteCluster{
+		Host:   srv2.Addr,
+		Proxy:  true,
+		Scheme: "http",
+	}
+
+	req := httptest.NewRequest("GET", "/arvados/v1/collections/99999999999999999999999999999999+99", nil)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+
+	resp := s.testRequest(req)
+	defer resp.Body.Close()
+
+	c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
+}
+
 func (s *FederationSuite) TestSaltedTokenGetCollectionByPDH(c *check.C) {
 	np := arvados.NodeProfile{
 		Controller: arvados.SystemServiceInstance{Listen: ":"},
-- 
2.30.2