commit d75eca3ec7501ac5a101fd77583740cd11924ead
parent 2f6f8bd901aa7eb953660fb601592aa734319e4e
Author: Gavin Peacock <gpeacock@adobe.com>
Date: Wed, 3 May 2023 12:25:10 -0700
(MINOR) Added ResourceNotFound error (#244)
* Added ResourceNotFound error
returned if thumbnails or c2pa manifests are missing
reports the identifier or path of missing resource
* Catch errors on manifest thumbnails too
* make Errors non-exhaustive
ensure folders are created on embed
Diffstat:
4 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/sdk/src/error.rs b/sdk/src/error.rs
@@ -17,6 +17,7 @@ use thiserror::Error;
/// `Error` enumerates errors returned by most C2PA toolkit operations.
#[derive(Debug, Error)]
+#[non_exhaustive]
pub enum Error {
// --- c2pa errors ---
/// Could not find a claim with this label.
@@ -198,6 +199,9 @@ pub enum Error {
#[error("file not found: {0}")]
FileNotFound(String),
+ #[error("resource not found: {0}")]
+ ResourceNotFound(String),
+
#[error("XMP read error")]
XmpReadError,
diff --git a/sdk/src/ingredient.rs b/sdk/src/ingredient.rs
@@ -855,8 +855,8 @@ impl Ingredient {
// add the ingredient manifest_data to the claim
// this is how any existing claims are added to the new store
- let c2pa_manifest = match self.manifest_data() {
- Some(buffer) => {
+ let c2pa_manifest = match self.manifest_data_ref() {
+ Some(resource_ref) => {
let manifest_label = self
.active_manifest
.clone()
@@ -873,9 +873,12 @@ impl Ingredient {
false => None,
};
+ // get the c2pa manifest bytes
+ let data = self.resources.get(&resource_ref.identifier)?;
+
// have Store check and load ingredients and add them to a claim
let ingredient_store =
- Store::load_ingredient_to_claim(claim, &manifest_label, &buffer, redactions)?;
+ Store::load_ingredient_to_claim(claim, &manifest_label, &data, redactions)?;
// get the ingredient map loaded in previous
match claim.claim_ingredient(&manifest_label) {
@@ -931,10 +934,11 @@ impl Ingredient {
// if the ingredient defines a thumbnail, add it to the claim
// otherwise use the parent claim thumbnail if available
- if let Some((format, data)) = self.thumbnail() {
+ if let Some(thumb_ref) = self.thumbnail_ref() {
+ let data = self.thumbnail_bytes()?;
let hash_url = claim.add_assertion(&Thumbnail::new(
- &labels::add_thumbnail_format(labels::INGREDIENT_THUMBNAIL, format),
- data.to_vec(),
+ &labels::add_thumbnail_format(labels::INGREDIENT_THUMBNAIL, &thumb_ref.format),
+ data.into_owned(),
))?;
thumbnail = Some(hash_url);
}
@@ -1371,7 +1375,7 @@ mod tests_file_io {
println!("ingredient = {ingredient}");
assert_eq!(ingredient.validation_status(), None);
- // verify we can't set a references that don't exist
+ // verify we can't set references that don't exist
assert!(ingredient
.set_thumbnail_ref(ResourceRef::new("Foo", "bar"))
.is_err());
diff --git a/sdk/src/manifest.rs b/sdk/src/manifest.rs
@@ -11,9 +11,9 @@
// specific language governing permissions and limitations under
// each license.
-#[cfg(feature = "file_io")]
-use std::path::Path;
use std::{borrow::Cow, collections::HashMap, io::Cursor};
+#[cfg(feature = "file_io")]
+use std::{fs::create_dir_all, path::Path};
use log::{debug, error, warn};
use serde::{de::DeserializeOwned, Deserialize, Serialize};
@@ -483,7 +483,7 @@ impl Manifest {
/// Ingredients resources will also be relative to this path
#[cfg(feature = "file_io")]
pub fn with_base_path<P: AsRef<Path>>(&mut self, base_path: P) -> Result<&Self> {
- std::fs::create_dir_all(&base_path)?;
+ create_dir_all(&base_path)?;
self.resources.set_base_path(base_path.as_ref());
for i in 0..self.ingredients.len() {
// todo: create different subpath for each ingredient?
@@ -626,7 +626,7 @@ impl Manifest {
}
// if a thumbnail is not already defined, create one here
- if self.thumbnail().is_none() {
+ if self.thumbnail_ref().is_none() {
#[cfg(feature = "add_thumbnails")]
if let Ok((format, image)) = crate::utils::thumbnail::make_thumbnail(path.as_ref()) {
// Do not write this as a file when reading from files
@@ -669,10 +669,12 @@ impl Manifest {
}
claim.format = self.format().to_owned();
claim.instance_id = self.instance_id().to_owned();
- if let Some((format, data)) = self.thumbnail() {
+
+ if let Some(thumb_ref) = self.thumbnail_ref() {
+ let data = self.resources.get(&thumb_ref.identifier)?;
claim.add_assertion(&Thumbnail::new(
- &labels::add_thumbnail_format(labels::CLAIM_THUMBNAIL, format),
- data.to_vec(),
+ &labels::add_thumbnail_format(labels::CLAIM_THUMBNAIL, &thumb_ref.format),
+ data.into_owned(),
))?;
}
@@ -804,6 +806,10 @@ impl Manifest {
}
// we need to copy the source to target before setting the asset info
if !dest_path.as_ref().exists() {
+ // ensure the path to the file exists
+ if let Some(output_dir) = dest_path.as_ref().parent() {
+ create_dir_all(output_dir)?;
+ }
std::fs::copy(&source_path, &dest_path)?;
copied = true;
}
@@ -1769,10 +1775,14 @@ pub(crate) mod tests {
#[test]
#[cfg(feature = "file_io")]
fn test_create_file_based_ingredient() {
- let mut folder = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
- folder.push("tests/fixtures");
+ let mut fixtures = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+ fixtures.push("tests/fixtures");
+
+ let temp_dir = tempdir().expect("temp dir");
+ let output = temp_fixture_path(&temp_dir, TEST_SMALL_JPEG);
+
let mut manifest = Manifest::new("claim_generator");
- manifest.resources.set_base_path(folder);
+ manifest.with_base_path(fixtures).expect("with_base");
// verify we can't set a references that don't exist
assert!(manifest
.set_thumbnail_ref(ResourceRef::new("image/jpg", "foo"))
@@ -1783,5 +1793,10 @@ pub(crate) mod tests {
.set_thumbnail_ref(ResourceRef::new("image/jpg", "C.jpg"))
.is_ok());
assert!(manifest.thumbnail_ref().is_some());
+
+ let signer = temp_signer();
+ manifest
+ .embed(&output, &output, signer.as_ref())
+ .expect("embed");
}
}
diff --git a/sdk/src/resource_store.rs b/sdk/src/resource_store.rs
@@ -135,10 +135,13 @@ impl ResourceStore {
Some(base) => {
// read the file, save in Map and then return a reference
let path = base.join(id);
- let value = std::fs::read(path)?;
+ let value = std::fs::read(path).map_err(|_| {
+ let path = base.join(id).to_string_lossy().into_owned();
+ Error::ResourceNotFound(path)
+ })?;
return Ok(Cow::Owned(value));
}
- None => return Err(Error::NotFound),
+ None => return Err(Error::ResourceNotFound(id.to_string())),
}
}
self.resources
