Use PKL when verifying DCPs.

4 files changed, 98 insertions, 10 deletions

diff --git a/src/pkl.cc b/src/pkl.cc
index 236313aa..a46ab313 100644
--- a/src/pkl.cc
+++ b/src/pkl.cc
@@ -35,6 +35,7 @@
 #include "exceptions.h"
 #include "util.h"
 #include "raw_convert.h"
+#include "dcp_assert.h"
 #include <libxml++/libxml++.h>
 #include <boost/foreach.hpp>
 
@@ -112,3 +113,15 @@ PKL::write (boost::filesystem::path file, shared_ptr<const CertificateChain> sig
 
 	doc.write_to_file (file.string(), "UTF-8");
 }
+
+string
+PKL::hash (string id) const
+{
+	BOOST_FOREACH (shared_ptr<Asset> i, _asset_list) {
+		if (i->id() == id) {
+			return i->hash;
+		}
+	}
+
+	DCP_ASSERT (false);
+}
diff --git a/src/pkl.h b/src/pkl.h
index 3b960ddf..d17b8fe1 100644
--- a/src/pkl.h
+++ b/src/pkl.h
@@ -36,6 +36,7 @@
 
 #include "object.h"
 #include "types.h"
+#include "util.h"
 #include "certificate_chain.h"
 #include <libcxml/cxml.h>
 #include <boost/filesystem.hpp>
@@ -59,6 +60,8 @@ public:
 		return _standard;
 	}
 
+	std::string hash (std::string id) const;
+
 	void add_asset (std::string id, boost::optional<std::string> annotation_text, std::string hash, int64_t size, std::string type);
 	void write (boost::filesystem::path file, boost::shared_ptr<const CertificateChain> signer) const;
 
@@ -68,7 +71,8 @@ private:
 	{
 	public:
 		Asset (cxml::ConstNodePtr node)
-			: annotation_text (node->optional_string_child("AnnotationText"))
+			: Object (remove_urn_uuid(node->string_child("Id")))
+			, annotation_text (node->optional_string_child("AnnotationText"))
 			, hash (node->string_child("Hash"))
 			, size (node->number_child<int64_t>("Size"))
 			, type (node->string_child("Type"))
diff --git a/src/verify.cc b/src/verify.cc
index 87958ed9..1e8c3091 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -53,13 +53,35 @@ using boost::function;
 
 using namespace dcp;
 
-static bool
-verify_asset (shared_ptr<ReelAsset> asset, function<void (float)> progress)
+enum Result {
+	RESULT_GOOD,
+	RESULT_CPL_PKL_DIFFER,
+	RESULT_BAD
+};
+
+static Result
+verify_asset (shared_ptr<DCP> dcp, shared_ptr<ReelAsset> reel_asset, function<void (float)> progress)
 {
-	string actual_hash = asset->asset_ref()->hash(progress);
-	optional<string> cpl_hash = asset->hash();
-	DCP_ASSERT (cpl_hash);
-	return actual_hash != *cpl_hash;
+	string const actual_hash = reel_asset->asset_ref()->hash(progress);
+
+	shared_ptr<PKL> pkl = dcp->pkl();
+	/* We've read this DCP in so it must have a PKL */
+	DCP_ASSERT (pkl);
+
+	shared_ptr<Asset> asset = reel_asset->asset_ref().asset();
+	cout << "looking for hash of " << reel_asset->asset_ref()->id() << "\n";
+	string const pkl_hash = pkl->hash (reel_asset->asset_ref()->id());
+
+	optional<string> cpl_hash = reel_asset->hash();
+	if (cpl_hash && *cpl_hash != pkl_hash) {
+		return RESULT_CPL_PKL_DIFFER;
+	}
+
+	if (actual_hash != pkl_hash) {
+		return RESULT_BAD;
+	}
+
+	return RESULT_GOOD;
 }
 
 list<VerificationNote>
@@ -89,14 +111,30 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
 				stage ("Checking reel", optional<boost::filesystem::path>());
 				if (reel->main_picture()) {
 					stage ("Checking picture asset hash", reel->main_picture()->asset()->file());
-					if (verify_asset (reel->main_picture(), progress)) {
+					Result const r = verify_asset (dcp, reel->main_picture(), progress);
+					switch (r) {
+					case RESULT_BAD:
 						notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "Picture asset hash is incorrect."));
+						break;
+					case RESULT_CPL_PKL_DIFFER:
+						notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "PKL and CPL hashes differ for picture asset."));
+						break;
+					default:
+						break;
 					}
 				}
 				if (reel->main_sound()) {
 					stage ("Checking sound asset hash", reel->main_sound()->asset()->file());
-					if (verify_asset (reel->main_sound(), progress)) {
+					Result const r = verify_asset (dcp, reel->main_sound(), progress);
+					switch (r) {
+					case RESULT_BAD:
 						notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "Sound asset hash is incorrect."));
+						break;
+					case RESULT_CPL_PKL_DIFFER:
+						notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "PKL and CPL hashes differ for sound asset."));
+						break;
+					default:
+						break;
 					}
 				}
 			}
diff --git a/test/verify_test.cc b/test/verify_test.cc
index f39d7ae6..3c52bdd8 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -32,8 +32,10 @@
 */
 
 #include "verify.h"
+#include "util.h"
 #include <boost/test/unit_test.hpp>
 #include <cstdio>
+#include <iostream>
 
 using std::list;
 using std::pair;
@@ -64,10 +66,14 @@ BOOST_AUTO_TEST_CASE (verify_test1)
 		boost::filesystem::copy_file (i->path(), "build/test/verify_test1" / i->path().filename());
 	}
 
+	/* Check DCP as-is (should be OK) */
+
 	vector<boost::filesystem::path> directories;
 	directories.push_back ("build/test/verify_test1");
 	list<dcp::VerificationNote> notes = dcp::verify (directories, &stage, &progress);
 
+	boost::filesystem::path const cpl_file = "build/test/verify_test1/cpl_81fb54df-e1bf-4647-8788-ea7ba154375b.xml";
+
 	list<pair<string, optional<boost::filesystem::path> > >::const_iterator st = stages.begin();
 	BOOST_CHECK_EQUAL (st->first, "Checking DCP");
 	BOOST_REQUIRE (st->second);
@@ -75,7 +81,7 @@ BOOST_AUTO_TEST_CASE (verify_test1)
 	++st;
 	BOOST_CHECK_EQUAL (st->first, "Checking CPL");
 	BOOST_REQUIRE (st->second);
-	BOOST_CHECK_EQUAL (st->second.get(), boost::filesystem::canonical("build/test/verify_test1/cpl_81fb54df-e1bf-4647-8788-ea7ba154375b.xml"));
+	BOOST_CHECK_EQUAL (st->second.get(), boost::filesystem::canonical(cpl_file));
 	++st;
 	BOOST_CHECK_EQUAL (st->first, "Checking reel");
 	BOOST_REQUIRE (!st->second);
@@ -92,6 +98,8 @@ BOOST_AUTO_TEST_CASE (verify_test1)
 
 	BOOST_CHECK_EQUAL (notes.size(), 0);
 
+	/* Corrupt the MXFs and check that this is spotted */
+
 	FILE* mod = fopen("build/test/verify_test1/video.mxf", "r+b");
 	BOOST_REQUIRE (mod);
 	fseek (mod, 4096, SEEK_SET);
@@ -111,4 +119,29 @@ BOOST_AUTO_TEST_CASE (verify_test1)
 	BOOST_CHECK_EQUAL (notes.front().note(), "Picture asset hash is incorrect.");
 	BOOST_CHECK_EQUAL (notes.back().type(), dcp::VerificationNote::VERIFY_ERROR);
 	BOOST_CHECK_EQUAL (notes.back().note(), "Sound asset hash is incorrect.");
+
+	/* Corrupt the hashes in the CPL and check that the disagreement between CPL and PKL is spotted */
+	string const cpl = dcp::file_to_string (cpl_file);
+	string hacked_cpl = "";
+	for (size_t i = 0; i < (cpl.length() - 6); ++i) {
+		if (cpl.substr(i, 6) == "<Hash>") {
+			hacked_cpl += "<Hash>x";
+			i += 6;
+		} else {
+			hacked_cpl += cpl[i];
+		}
+	}
+	hacked_cpl += "list>";
+
+	FILE* f = fopen(cpl_file.string().c_str(), "w");
+	fwrite(hacked_cpl.c_str(), hacked_cpl.length(), 1, f);
+	fclose(f);
+
+	notes = dcp::verify (directories, &stage, &progress);
+	BOOST_CHECK_EQUAL (notes.size(), 2);
+	BOOST_CHECK_EQUAL (notes.front().type(), dcp::VerificationNote::VERIFY_ERROR);
+	BOOST_CHECK_EQUAL (notes.front().note(), "PKL and CPL hashes differ for picture asset.");
+	BOOST_CHECK_EQUAL (notes.back().type(), dcp::VerificationNote::VERIFY_ERROR);
+	BOOST_CHECK_EQUAL (notes.back().note(), "PKL and CPL hashes differ for sound asset.");
+
 }