Skip to content

Commit d75eca3

Browse files
authored
(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
1 parent 2f6f8bd commit d75eca3

File tree

4 files changed

+45
-19
lines changed

4 files changed

+45
-19
lines changed

sdk/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use thiserror::Error;
1717

1818
/// `Error` enumerates errors returned by most C2PA toolkit operations.
1919
#[derive(Debug, Error)]
20+
#[non_exhaustive]
2021
pub enum Error {
2122
// --- c2pa errors ---
2223
/// Could not find a claim with this label.
@@ -198,6 +199,9 @@ pub enum Error {
198199
#[error("file not found: {0}")]
199200
FileNotFound(String),
200201

202+
#[error("resource not found: {0}")]
203+
ResourceNotFound(String),
204+
201205
#[error("XMP read error")]
202206
XmpReadError,
203207

sdk/src/ingredient.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -855,8 +855,8 @@ impl Ingredient {
855855

856856
// add the ingredient manifest_data to the claim
857857
// this is how any existing claims are added to the new store
858-
let c2pa_manifest = match self.manifest_data() {
859-
Some(buffer) => {
858+
let c2pa_manifest = match self.manifest_data_ref() {
859+
Some(resource_ref) => {
860860
let manifest_label = self
861861
.active_manifest
862862
.clone()
@@ -873,9 +873,12 @@ impl Ingredient {
873873
false => None,
874874
};
875875

876+
// get the c2pa manifest bytes
877+
let data = self.resources.get(&resource_ref.identifier)?;
878+
876879
// have Store check and load ingredients and add them to a claim
877880
let ingredient_store =
878-
Store::load_ingredient_to_claim(claim, &manifest_label, &buffer, redactions)?;
881+
Store::load_ingredient_to_claim(claim, &manifest_label, &data, redactions)?;
879882

880883
// get the ingredient map loaded in previous
881884
match claim.claim_ingredient(&manifest_label) {
@@ -931,10 +934,11 @@ impl Ingredient {
931934

932935
// if the ingredient defines a thumbnail, add it to the claim
933936
// otherwise use the parent claim thumbnail if available
934-
if let Some((format, data)) = self.thumbnail() {
937+
if let Some(thumb_ref) = self.thumbnail_ref() {
938+
let data = self.thumbnail_bytes()?;
935939
let hash_url = claim.add_assertion(&Thumbnail::new(
936-
&labels::add_thumbnail_format(labels::INGREDIENT_THUMBNAIL, format),
937-
data.to_vec(),
940+
&labels::add_thumbnail_format(labels::INGREDIENT_THUMBNAIL, &thumb_ref.format),
941+
data.into_owned(),
938942
))?;
939943
thumbnail = Some(hash_url);
940944
}
@@ -1371,7 +1375,7 @@ mod tests_file_io {
13711375
println!("ingredient = {ingredient}");
13721376
assert_eq!(ingredient.validation_status(), None);
13731377

1374-
// verify we can't set a references that don't exist
1378+
// verify we can't set references that don't exist
13751379
assert!(ingredient
13761380
.set_thumbnail_ref(ResourceRef::new("Foo", "bar"))
13771381
.is_err());

sdk/src/manifest.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111
// specific language governing permissions and limitations under
1212
// each license.
1313

14-
#[cfg(feature = "file_io")]
15-
use std::path::Path;
1614
use std::{borrow::Cow, collections::HashMap, io::Cursor};
15+
#[cfg(feature = "file_io")]
16+
use std::{fs::create_dir_all, path::Path};
1717

1818
use log::{debug, error, warn};
1919
use serde::{de::DeserializeOwned, Deserialize, Serialize};
@@ -483,7 +483,7 @@ impl Manifest {
483483
/// Ingredients resources will also be relative to this path
484484
#[cfg(feature = "file_io")]
485485
pub fn with_base_path<P: AsRef<Path>>(&mut self, base_path: P) -> Result<&Self> {
486-
std::fs::create_dir_all(&base_path)?;
486+
create_dir_all(&base_path)?;
487487
self.resources.set_base_path(base_path.as_ref());
488488
for i in 0..self.ingredients.len() {
489489
// todo: create different subpath for each ingredient?
@@ -626,7 +626,7 @@ impl Manifest {
626626
}
627627

628628
// if a thumbnail is not already defined, create one here
629-
if self.thumbnail().is_none() {
629+
if self.thumbnail_ref().is_none() {
630630
#[cfg(feature = "add_thumbnails")]
631631
if let Ok((format, image)) = crate::utils::thumbnail::make_thumbnail(path.as_ref()) {
632632
// Do not write this as a file when reading from files
@@ -669,10 +669,12 @@ impl Manifest {
669669
}
670670
claim.format = self.format().to_owned();
671671
claim.instance_id = self.instance_id().to_owned();
672-
if let Some((format, data)) = self.thumbnail() {
672+
673+
if let Some(thumb_ref) = self.thumbnail_ref() {
674+
let data = self.resources.get(&thumb_ref.identifier)?;
673675
claim.add_assertion(&Thumbnail::new(
674-
&labels::add_thumbnail_format(labels::CLAIM_THUMBNAIL, format),
675-
data.to_vec(),
676+
&labels::add_thumbnail_format(labels::CLAIM_THUMBNAIL, &thumb_ref.format),
677+
data.into_owned(),
676678
))?;
677679
}
678680

@@ -804,6 +806,10 @@ impl Manifest {
804806
}
805807
// we need to copy the source to target before setting the asset info
806808
if !dest_path.as_ref().exists() {
809+
// ensure the path to the file exists
810+
if let Some(output_dir) = dest_path.as_ref().parent() {
811+
create_dir_all(output_dir)?;
812+
}
807813
std::fs::copy(&source_path, &dest_path)?;
808814
copied = true;
809815
}
@@ -1769,10 +1775,14 @@ pub(crate) mod tests {
17691775
#[test]
17701776
#[cfg(feature = "file_io")]
17711777
fn test_create_file_based_ingredient() {
1772-
let mut folder = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
1773-
folder.push("tests/fixtures");
1778+
let mut fixtures = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
1779+
fixtures.push("tests/fixtures");
1780+
1781+
let temp_dir = tempdir().expect("temp dir");
1782+
let output = temp_fixture_path(&temp_dir, TEST_SMALL_JPEG);
1783+
17741784
let mut manifest = Manifest::new("claim_generator");
1775-
manifest.resources.set_base_path(folder);
1785+
manifest.with_base_path(fixtures).expect("with_base");
17761786
// verify we can't set a references that don't exist
17771787
assert!(manifest
17781788
.set_thumbnail_ref(ResourceRef::new("image/jpg", "foo"))
@@ -1783,5 +1793,10 @@ pub(crate) mod tests {
17831793
.set_thumbnail_ref(ResourceRef::new("image/jpg", "C.jpg"))
17841794
.is_ok());
17851795
assert!(manifest.thumbnail_ref().is_some());
1796+
1797+
let signer = temp_signer();
1798+
manifest
1799+
.embed(&output, &output, signer.as_ref())
1800+
.expect("embed");
17861801
}
17871802
}

sdk/src/resource_store.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,13 @@ impl ResourceStore {
135135
Some(base) => {
136136
// read the file, save in Map and then return a reference
137137
let path = base.join(id);
138-
let value = std::fs::read(path)?;
138+
let value = std::fs::read(path).map_err(|_| {
139+
let path = base.join(id).to_string_lossy().into_owned();
140+
Error::ResourceNotFound(path)
141+
})?;
139142
return Ok(Cow::Owned(value));
140143
}
141-
None => return Err(Error::NotFound),
144+
None => return Err(Error::ResourceNotFound(id.to_string())),
142145
}
143146
}
144147
self.resources

0 commit comments

Comments
 (0)