Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C2PA metadata handling #662

Merged
merged 2 commits into from
Dec 26, 2024
Merged
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
4 changes: 4 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub enum PngError {
ChunkMissing(&'static str),
InvalidDepthForType(BitDepth, ColorType),
IncorrectDataLength(usize, usize),
C2PAMetadataPreventsChanges,
Other(Box<str>),
}

Expand Down Expand Up @@ -41,6 +42,9 @@ impl fmt::Display for PngError {
"Data length {} does not match the expected length {}",
l1, l2
),
PngError::C2PAMetadataPreventsChanges => f.write_str(
"The image contains C2PA manifest that would be invalidated by any file changes",
),
PngError::Other(ref s) => f.write_str(s),
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ pub struct Chunk {
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum StripChunks {
/// None
///
/// ...except caBX chunk if it contains a C2PA.org signature.
None,
/// Remove specific chunks
Strip(IndexSet<[u8; 4]>),
Expand Down Expand Up @@ -111,6 +113,36 @@ pub struct RawChunk<'a> {
pub data: &'a [u8],
}

impl RawChunk<'_> {
// Is it a chunk for C2PA/CAI JUMBF metadata
pub(crate) fn is_c2pa(&self) -> bool {
if self.name == *b"caBX" {
if let Some((b"jumb", data)) = parse_jumbf_box(self.data) {
if let Some((b"jumd", data)) = parse_jumbf_box(data) {
if data.get(..4) == Some(b"c2pa") {
return true;
}
}
}
}
false
}
}

fn parse_jumbf_box(data: &[u8]) -> Option<(&[u8], &[u8])> {
if data.len() < 8 {
return None;
}
let (len, rest) = data.split_at(4);
let len = u32::from_be_bytes(len.try_into().unwrap()) as usize;
if len < 8 || len > data.len() {
return None;
}
let (box_name, data) = rest.split_at(4);
let data = data.get(..len - 8)?;
Some((box_name, data))
}

pub fn parse_next_chunk<'a>(
byte_data: &'a [u8],
byte_offset: &mut usize,
Expand Down
64 changes: 41 additions & 23 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,19 @@ mod rayon;

#[cfg(feature = "zopfli")]
use std::num::NonZeroU8;
use std::{ffi::OsString, fs::DirBuilder, io::Write, path::PathBuf, process::exit, time::Duration};
use std::process::ExitCode;
use std::{ffi::OsString, fs::DirBuilder, io::Write, path::PathBuf, time::Duration};

use clap::ArgMatches;
mod cli;
use indexmap::IndexSet;
use log::{error, warn, Level, LevelFilter};
use oxipng::{Deflaters, InFile, Options, OutFile, RowFilter, StripChunks};
use oxipng::{Deflaters, InFile, Options, OutFile, PngError, RowFilter, StripChunks};
use rayon::prelude::*;

use crate::cli::DISPLAY_CHUNKS;

fn main() {
fn main() -> ExitCode {
let matches = cli::build_command()
// Set the value parser for filters which isn't appropriate to do in the build_command function
.mut_arg("filters", |arg| {
Expand All @@ -43,15 +44,15 @@ fn main() {
.get_matches_from(std::env::args());

if matches.get_flag("backup") {
eprintln!("The --backup flag is no longer supported. Please use --out or --dir to preserve your existing files.");
exit(1)
error!("The --backup flag is no longer supported. Please use --out or --dir to preserve your existing files.");
return ExitCode::FAILURE;
}

let (out_file, out_dir, opts) = match parse_opts_into_struct(&matches) {
Ok(x) => x,
Err(x) => {
error!("{}", x);
exit(1)
return ExitCode::FAILURE;
}
};

Expand All @@ -75,28 +76,45 @@ fn main() {
true,
);

let success = files.into_par_iter().filter(|(input, output)| {
match oxipng::optimize(input, output, &opts) {
// For optimizing single files, this will return the correct exit code always.
// For recursive optimization, the correct choice is a bit subjective.
// We're choosing to return a 0 exit code if ANY file in the set
// runs correctly.
// The reason for this is that recursion may pick up files that are not
// PNG files, and return an error for them.
// We don't really want to return an error code for those files.
Ok(_) => true,
Err(e) => {
error!("{}: {}", input, e);
false
let summary = files
.into_par_iter()
.map(|(input, output)| {
match oxipng::optimize(&input, &output, &opts) {
// For optimizing single files, this will return the correct exit code always.
// For recursive optimization, the correct choice is a bit subjective.
// We're choosing to return a 0 exit code if ANY file in the set
// runs correctly.
// The reason for this is that recursion may pick up files that are not
// PNG files, and return an error for them.
// We don't really want to return an error code for those files.
Ok(_) => OptimizationResult::Ok,
Err(e @ PngError::C2PAMetadataPreventsChanges) => {
warn!("{input}: {e}");
OptimizationResult::Skipped
}
Err(e) => {
error!("{input}: {e}");
OptimizationResult::Failed
}
}
}
});
})
.min()
.unwrap_or(OptimizationResult::Skipped);

if success.count() == 0 {
exit(1);
match summary {
OptimizationResult::Ok => ExitCode::SUCCESS,
OptimizationResult::Failed => ExitCode::FAILURE,
OptimizationResult::Skipped => ExitCode::from(3),
}
}

#[derive(Eq, PartialEq, Ord, PartialOrd)]
enum OptimizationResult {
Ok,
Failed,
Skipped,
}

fn collect_files(
files: Vec<PathBuf>,
out_dir: &Option<PathBuf>,
Expand Down
9 changes: 9 additions & 0 deletions src/png/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ impl PngData {
}
_ => {
if opts.strip.keep(&chunk.name) {
if chunk.is_c2pa() {
// StripChunks::None is the default value, so to keep optimizing by default,
// interpret it as stripping the C2PA metadata.
// The C2PA metadata is invalidated if the file changes, so it shouldn't be kept.
if opts.strip == StripChunks::None {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also strip automatically if StripChunks::Strip was used? I.e. only error if caBX was explicitly kept with StripChunks::Keep.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This point is certainly open to discussion and could be adjusted in the future based on user feedback and similar inputs. For now, I believe the logic implemented in this PR offers a good default behavior without overburdening the Strip and Keep variants with additional complexity, as users relying on them are, in my view, more likely to be sensitive to different chunk selection strategies. But I don't have a strong opinion one way or another, caBX is by design a bit of a weird chunk anyway.

continue;
}
return Err(PngError::C2PAMetadataPreventsChanges);
}
aux_chunks.push(Chunk {
name: chunk.name,
data: chunk.data.to_owned(),
Expand Down
Binary file added tests/files/c2pa-signed.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
13 changes: 13 additions & 0 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ fn optimize() {
assert!(result.is_ok());
}

#[test]
fn skip_c2pa() {
let result = oxipng::optimize(
&"tests/files/c2pa-signed.png".into(),
&OutFile::None,
&Options {
strip: StripChunks::Keep(indexset! {*b"caBX"}),
..Options::default()
},
);
assert!(matches!(result, Err(PngError::C2PAMetadataPreventsChanges)));
}

#[test]
fn optimize_corrupted() {
let result = oxipng::optimize(
Expand Down
Loading