Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion sdk/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ impl Store {
self.remote_url.as_deref()
}

// Set the remote url of the manifest if this Store
pub(crate) fn set_remote_url(&mut self, url: &str) {
self.remote_url = Some(url.to_string());
}

/// Returns if the [`Store`] was created from an embedded manifest.
pub fn is_embedded(&self) -> bool {
self.embedded
Expand Down Expand Up @@ -3756,12 +3761,19 @@ impl Store {
stream.rewind()?;

// First we convert the JUMBF into a usable store.
let store = Store::from_jumbf_with_settings(c2pa_data, validation_log, settings)
let mut store = Store::from_jumbf_with_settings(c2pa_data, validation_log, settings)
.inspect_err(|e| {
log_item!("asset", "error loading file", "load_from_asset")
.failure_no_throw(validation_log, e);
})?;

// fetch remote manifest if available
if let Some(ext_ref) =
crate::utils::xmp_inmemory_utils::XmpInfo::from_source(&mut stream, format).provenance
{
store.set_remote_url(&ext_ref);
}
Comment on lines +3770 to +3775
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for Reader::remote_url is defined as:

Returns the remote url of the manifest if this Reader obtained the manifest remotely.

Here we are changing that definition but also the behavior that exists in other parts of the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that no remote_url is returned when using this path and Tania reported an error. What do you want here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current behavior is to ignore remote_url if the manifest is found as embedded first. Here we are always setting the remote_url. It's an unusual situation though because we aren't actually looking for an embedded manifest anyways.

If we are going to make this change here, we should probably make that change in the other constructors and change the reader docs to state that it's always returned regardless of how the manifest was obtained.

Copy link
Contributor

@tmathern tmathern Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(For context, on the error, if it's the one I am thinking of( #1532): The reported issue is that in certain cases you can "keep" and outdated remote URL manifest. Even if our SDK does not look at it, it still could be found. For instance, even if our SDK does the right thing, you could still "find" URLs by using exiftool. More details in #1532).


if verify {
stream.rewind()?;
let mut asset_data = ClaimAssetData::Stream(&mut stream, format);
Expand Down
66 changes: 65 additions & 1 deletion sdk/tests/test_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// specific language governing permissions and limitations under
// each license.

use std::io::{self, Cursor};
use std::io::{self, Cursor, Seek};

use c2pa::{
settings::Settings, validation_status, Builder, BuilderIntent, ManifestAssertionKind, Reader,
Expand Down Expand Up @@ -79,6 +79,70 @@ fn test_builder_riff() -> Result<()> {
Ok(())
}

#[test]
fn test_builder_sidecar_only_bmff_cawg() -> Result<()> {
Settings::from_toml(include_str!("../tests/fixtures/test_settings.toml"))?;
Settings::from_toml(include_str!(
"../tests/fixtures/test_settings_with_cawg_signing.toml"
))?; // overlay cawg settings

let mut source = Cursor::new(include_bytes!("fixtures/video1.mp4"));
let format = "video/mp4";
let mut dest = Cursor::new(Vec::new());

let manifest_json = r#"
{
"assertions": [
{
"label": "c2pa.metadata",
"kind": "Json",
"data": {
"@context": { "exif": "http://ns.adobe.com/exif/1.0/" },
"exif:GPSLatitude": "39,21.102N"
}
},
{
"label": "cawg.metadata",
"kind": "Json",
"data": {
"@context": { "cawg": "http://cawg.org/ns/1.0/" },
"cawg:SomeField": "SomeValue"
}
},
{
"label": "c2pa.assertion.metadata",
"data": {
"@context": { "custom": "http://custom.org/ns/1.0/" },
"custom:Field": "CustomValue"
}
},
{
"label": "org.myorg.metadata",
"data": {
"@context": { "myorg": "http://myorg.org/ns/1.0/" },
"myorg:Field": "MyOrgValue"
}
}
]
}
"#;

let mut builder = Builder::from_json(manifest_json)?;
builder.set_intent(BuilderIntent::Edit);
builder.definition.claim_version = Some(2); // use v1 for this test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that's using v2?

builder.no_embed = true;
builder.remote_url = Some("http://somecompany.org".to_string());
let c2pa_data = builder.sign(&Settings::signer()?, format, &mut source, &mut dest)?;

dest.rewind()?;
let reader1 = Reader::from_manifest_data_and_stream(&c2pa_data, format, &mut dest)?;
assert!(!reader1.is_embedded());
assert_eq!(reader1.remote_url().unwrap(), "http://somecompany.org/");
println!("reader1: {reader1}");

Ok(())
}

#[test]
fn test_builder_sidecar_only() -> Result<()> {
Settings::from_toml(include_str!("../tests/fixtures/test_settings.toml"))?;
Expand Down
Loading