aleleba/pdfcraft
1
0
Fork 0
mirror of https://github.com/storytold/pdfcraft.git synced 2026-10-29 12:38:58 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(cos): keep saves incremental when only the /Encrypt place differs (#3276)

Browse Source 
From review. Where the /Encrypt dictionary is stored, its object number
or inline, is no longer part of the security comparison: the overlay
takes it from the snapshot with its trailer, so undo past an optimized
save still appends to that save. A full rewrite is left for an actual
change of encryption: another dictionary or key, or below revision 5 a
different first /ID. AES-256 keys don't depend on /ID, so it is no
longer compared there, here or when reopening with a known key.

overlay_changes_from now decides alone: Ok(false), changing nothing,
when security differs, and save_bytes acts on that. A signed document
refuses with SignedRewrite instead of being rewritten.

The overlay also writes again any unchanged object packed in an object
stream that the update replaces or frees. After an optimized save such
an object read back as null from the next save.
This commit is contained in:
skylar taylor-barrick
2026-10-26 12:50:50 -06:00
parent 6210a7292c
commit de776f6fc9
4 changed files with 275 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/cos/src/document.rs
+54 -27
View File
@@ -606,7 +606,7 @@ impl Document {
/// encrypted differently fails with `NeedsPassword`.
pub fn open_with_same_credentials(&self, data: Arc<Vec<u8>>) -> Result<Self, CosError> {
let credentials = match &self.security {
Some(handler) => Credentials::Handler { handler, id0: self.first_id() },
Some(handler) => Credentials::Handler { handler, id0: self.first_id().to_vec() },
None => Credentials::Password(None),
};
Self::open_as(data, credentials, self.stream_limit)
@@ -717,15 +717,16 @@ impl Document {
return Ok(());
};
let params = crate::security::encrypt_dict(self, &dict);
let id0 = self.first_id();
let id0 = self.first_id().to_vec();
let handler = match credentials {
Credentials::Password(password) => Arc::new(pdfcraft_crypt::SecurityHandler::open(params, &id0, *password).map_err(|e| match e {
pdfcraft_crypt::CryptError::WrongPassword if password.is_none() => CosError::NeedsPassword,
pdfcraft_crypt::CryptError::WrongPassword => CosError::WrongPassword,
other => CosError::Security(other.to_string()),
})?),
// The same encryption as the document whose key this is: that key opens it.
Credentials::Handler { handler, id0: known } if *handler.dict() == params && *known == id0 => Arc::clone(handler),
// The same encryption as the document whose key this is: that key opens it. Below
// revision 5 the first /ID is part of the key; AES-256 keys don't depend on it.
Credentials::Handler { handler, id0: known } if *handler.dict() == params && (params.r >= 5 || *known == id0) => Arc::clone(handler),
Credentials::Handler { .. } => return Err(CosError::NeedsPassword),
};
self.security = Some(handler);
@@ -736,11 +737,12 @@ impl Document {
Ok(())
}
/// The first file identifier, part of the encryption key (empty when there is none).
fn first_id(&self) -> Vec<u8> {
/// The first file identifier, part of the encryption key below revision 5 (empty when there
/// is none, as key derivation reads a missing one).
fn first_id(&self) -> &[u8] {
match self.trailer.get(b"ID") {
Some(Object::Array(a)) => a.first().and_then(|s| s.as_string()).map(|s| s.bytes.clone()).unwrap_or_default(),
_ => Vec::new(),
Some(Object::Array(a)) => a.first().and_then(|s| s.as_string()).map_or(&[], |s| s.bytes.as_slice()),
_ => &[],
}
}
@@ -753,7 +755,7 @@ impl Document {
if current.auth() == pdfcraft_crypt::Auth::Owner {
return Ok(());
}
let owner = current.authenticate_owner(&self.first_id(), password).map_err(|e| match e {
let owner = current.authenticate_owner(self.first_id(), password).map_err(|e| match e {
pdfcraft_crypt::CryptError::WrongPassword => CosError::WrongPassword,
other => CosError::Security(other.to_string()),
})?;
@@ -799,18 +801,18 @@ impl Document {
self.output_security().0
}
/// True when an incremental save of `self` can carry `other`'s objects under `self`'s
/// encryption (see [`Document::overlay_changes_from`]). Passwords and authentication level
/// are not an encryption identity: the same passwords can protect a different key,
/// algorithm or permissions. The dictionary's object number also matters, since the writer
/// must leave that object unencrypted. A different file key always changes the dictionary
/// too; comparing it as well costs nothing.
/// True when `self` and `other` are written with the same encryption: the same security
/// dictionary and file key, and below revision 5, where the first file identifier is part of
/// the key, the same identifier. Passwords and authentication level are not an encryption
/// identity: the same passwords can protect a different key, algorithm or permissions, and
/// user and owner openings share one. Where the dictionary is stored (its object number, or
/// inline) is not compared: [`Document::overlay_changes_from`] takes that from the document
/// it copies. A different file key always changes the dictionary too; comparing it as well
/// costs nothing.
pub fn has_same_output_security(&self, other: &Document) -> bool {
match (self.output_security(), other.output_security()) {
((None, _), (None, _)) => true,
((Some(a), a_num), (Some(b), b_num)) => {
a_num == b_num && a.dict() == b.dict() && a.file_key() == b.file_key() && self.first_id() == other.first_id()
}
match (self.output_handler(), other.output_handler()) {
(None, None) => true,
(Some(a), Some(b)) => a.dict() == b.dict() && a.file_key() == b.file_key() && (a.dict().r >= 5 || self.first_id() == other.first_id()),
_ => false,
}
}
@@ -1347,15 +1349,19 @@ impl Document {
/// Record `current`'s object graph as unsaved edits of this document.
///
/// Used when the working snapshot was restored from an earlier revision and the next
/// incremental save must still append to this file (the last saved revision). Objects
/// that already match are left untouched. Reads go through private readers so neither
/// incremental save must still append to this file (the last saved revision), opened afresh:
/// which objects sit in object streams is read from the file. Objects that already match are
/// left untouched. Reads go through private readers so neither
/// document's editing cache is filled.
///
/// Output security must match: the copied trailer must describe the handler and
/// encryption-dictionary exemption this writer uses. Otherwise write `current` in full.
pub fn overlay_changes_from(&mut self, current: &Document) -> Result<(), CosError> {
/// Returns `false`, changing nothing, when output security differs
/// ([`Document::has_same_output_security`]): an update appended to this file would be read
/// under other security than the objects before it, so `current` must be written in full. Otherwise the `/Encrypt` dictionary
/// comes with `current`'s objects and trailer, and so does the number the writer leaves
/// unencrypted, even where a full save had moved the dictionary.
pub fn overlay_changes_from(&mut self, current: &Document) -> Result<bool, CosError> {
if !self.has_same_output_security(current) {
return Err(CosError::Security("cannot overlay different output encryption; write the current document in full".into()));
return Ok(false);
}
let saved_nums = self.object_numbers();
let current_nums = current.object_numbers();
@@ -1379,6 +1385,19 @@ impl Document {
}
}
}
// An unchanged object packed in an object stream that this update replaces or frees would
// be lost: the new section no longer has that stream. Write it as an object of its own.
// (A stream container is never itself in a stream, so one pass is enough.)
let gone = |stream: u32| changed.binary_search(&stream).is_ok() || freed.binary_search(&stream).is_ok();
let stranded: Vec<u32> = current_nums
.iter()
.copied()
.filter(|&num| {
changed.binary_search(&num).is_err() && matches!(self.xref_entry(num), Some(XrefEntry::InStream { stream, .. }) if gone(stream))
})
.collect();
changed.extend(stranded);
changed.sort_unstable();
for num in freed {
let generation = self.generation(num);
self.free(ObjRef::new(num, generation));
@@ -1390,7 +1409,15 @@ impl Document {
self.set(ObjRef::new(num, generation), (*object).clone());
}
*self.trailer_mut() = current.trailer().clone();
Ok(())
// Both numbers normally changed above (or the dictionary is inline), so nothing is read
// from this file under the other exemption.
let encrypt_num = current.output_security().1;
if self.encryption_changed {
self.out_encrypt_num = encrypt_num;
} else {
self.encrypt_num = encrypt_num;
}
Ok(true)
}
/// Edit a dictionary (or stream dictionary) object in place.
crates/cos/tests/it/encryption.rs
+116 -72
View File
@@ -498,10 +498,19 @@ fn a_stream_labelled_xref_is_encrypted_when_saved() {
// Undo can restore a snapshot whose output security differs from the last saved file (#3276).
// Overlaying such a snapshot onto that file must be refused before anything changes.
/// Edit `/Title` in the document information dictionary: an unsaved change to compare.
/// Edit `/Title` in the document information dictionary, adding one if there is none: an unsaved
/// change to compare.
fn set_title(doc: &mut Document, title: &str) {
let info = doc.trailer().reference(b"Info").unwrap();
doc.update_dict(info, |d| d.set(b"Title".to_vec(), Object::String(PdfString { bytes: title.as_bytes().to_vec(), hex: false }))).unwrap();
let title = Object::String(PdfString { bytes: title.as_bytes().to_vec(), hex: false });
match doc.trailer().reference(b"Info") {
Some(info) => doc.update_dict(info, |d| d.set(b"Title".to_vec(), title)).unwrap(),
None => {
let mut info = Dict::new();
info.set(b"Title".to_vec(), title);
let info = doc.add(Object::Dict(info));
doc.trailer_mut().set(b"Info".to_vec(), Object::Ref(info));
}
}
}
/// Protect again with the same passwords and no restrictions; `seed` picks the key and `/ID`.
@@ -530,8 +539,18 @@ fn state(doc: &Document) -> (Dict, Vec<u32>, Vec<u32>, bool, Vec<u8>) {
(doc.trailer().clone(), doc.object_numbers(), doc.modified_objects(), doc.is_modified(), save)
}
/// Every incompatible pairing is refused with `CosError::Security` and leaves the target exactly
/// as it was; the same overlay with matching security does apply.
/// Protect `fixture()` as [`protect`] does, without its `/Info` dictionary. A full save numbers
/// the trailer's `/Root`, `/Info` and `/Encrypt` first, in that order, so the same seed keeps the
/// dictionary, key and `/ID` but `/Encrypt` is written under another number.
fn protect_moved(alg: Algorithm) -> Vec<u8> {
let mut doc = Document::open(Arc::new(fixture())).unwrap();
doc.trailer_mut().remove(b"Info");
rekey(&mut doc, alg, 9);
write_full(&doc, &SaveOptions::default()).unwrap()
}
/// Every incompatible pairing returns `false` and leaves the target exactly as it was; the same
/// overlay with matching security does apply.
#[test]
fn an_overlay_with_different_output_security_is_refused_before_any_change() {
let encrypted = Arc::new(protect(Algorithm::Aes256, "user", "owner", -1));
@@ -561,16 +580,20 @@ fn an_overlay_with_different_output_security_is_refused_before_any_change() {
let mut other_alg = open_owner();
rekey(&mut other_alg, Algorithm::Rc4_128, 9);
cases.push(("same passwords, other algorithm", encrypted_target(), other_alg));
let mut other_id = open_owner();
// Below revision 5 the first /ID is part of the key.
let aes128 = Arc::new(protect(Algorithm::Aes128, "user", "owner", -1));
let mut aes128_target = Document::open_with_password(aes128.clone(), Some("owner")).unwrap();
set_title(&mut aes128_target, "Saved title");
let mut other_id = Document::open_with_password(aes128, Some("owner")).unwrap();
with_first_id(&mut other_id, b"another file id!");
set_title(&mut other_id, "Restored title");
cases.push(("different first /ID", encrypted_target(), other_id));
cases.push(("AES-128, different first /ID", aes128_target, other_id));
for (case, mut target, current) in cases {
assert!(!target.has_same_output_security(&current) && !current.has_same_output_security(&target), "{case}");
let before = state(&target);
let result = target.overlay_changes_from(&current);
assert!(matches!(result, Err(CosError::Security(_))), "{case}: {result:?}");
assert!(matches!(result, Ok(false)), "{case}: {result:?}");
assert!(before == state(&target), "{case}: a refused overlay changed the target");
}
@@ -580,11 +603,56 @@ fn an_overlay_with_different_output_security_is_refused_before_any_change() {
set_title(&mut current, "Restored title");
assert!(target.has_same_output_security(&current));
let before = state(&target);
target.overlay_changes_from(&current).unwrap();
assert!(target.overlay_changes_from(&current).unwrap());
assert!(before != state(&target), "a compatible overlay applies the snapshot");
assert_eq!(title(&target).as_deref(), Some("Restored title"));
}
/// A full save can move the `/Encrypt` dictionary (renumbered by compaction, or written inline by
/// another tool) without changing the encryption. Undo across it still appends to the saved
/// file, and the update leaves unencrypted the dictionary its trailer names.
#[test]
fn an_overlay_takes_the_encrypt_dictionary_from_the_snapshot() {
let place = |doc: &Document| match doc.trailer().get(b"Encrypt") {
Some(Object::Ref(r)) => Some(r.num),
_ => None,
};
for alg in ALL {
let files =
[("indirect", protect(alg, "user", "owner", -1)), ("moved", protect_moved(alg)), ("inline", protect_inline(alg, "user", "owner"))];
let open = |bytes: &Vec<u8>| Document::open_with_password(Arc::new(bytes.clone()), Some("owner")).unwrap();
for (saved_name, saved) in &files {
for (snapshot_name, snapshot) in &files {
if saved_name == snapshot_name {
continue;
}
let case = format!("{alg:?}: {snapshot_name} snapshot over the {saved_name} save");
let mut base = open(saved);
let mut current = open(snapshot);
assert_ne!(place(&base), place(&current), "{case}");
assert!(base.has_same_output_security(&current), "{case}");
set_title(&mut current, "Restored title");
assert!(base.overlay_changes_from(&current).unwrap(), "{case}");
let bytes = write_incremental(&base, &SaveOptions::default()).unwrap();
assert!(bytes.starts_with(saved), "{case}: appended to the saved file");
assert!(!String::from_utf8_lossy(&bytes).contains("Secret page"), "{case}: plaintext leaked");
let bytes = Arc::new(bytes);
assert!(matches!(Document::open(bytes.clone()), Err(CosError::NeedsPassword)), "{case}");
for password in ["user", "owner"] {
let reopened = Document::open_with_password(bytes.clone(), Some(password)).unwrap_or_else(|e| panic!("{case}/{password}: {e:?}"));
assert_eq!(place(&reopened), place(&current), "{case}");
assert_eq!(content(&reopened), SECRET, "{case}/{password}");
assert_eq!(title(&reopened).as_deref(), Some("Restored title"), "{case}/{password}");
}
// Independent check, as in `every_algorithm_round_trips_and_hides_the_content`.
let hay = hayro_syntax::Pdf::new_with_password(bytes.as_ref().clone(), "user").unwrap_or_else(|e| panic!("{case}: {e:?}"));
let page = hay.pages().first().unwrap().page_stream().map(|s| String::from_utf8_lossy(s).trim().to_string());
assert_eq!(page.as_deref(), Some(SECRET), "{case}");
}
}
}
}
/// The user and owner passwords unlock the same file key: either opening may be overlaid on the
/// other, and doing so neither elevates nor drops the user's restrictions in the saved file.
#[test]
@@ -605,7 +673,7 @@ fn output_security_does_not_depend_on_the_authentication_level() {
let mut target = Document::open_with_password(bytes.clone(), Some(base)).unwrap();
let mut current = Document::open_with_password(bytes.clone(), Some(password)).unwrap();
set_title(&mut current, "Edited");
target.overlay_changes_from(&current).unwrap();
assert!(target.overlay_changes_from(&current).unwrap(), "{alg:?}");
let saved = Arc::new(write_incremental(&target, &SaveOptions::default()).unwrap());
assert!(!String::from_utf8_lossy(&saved).contains("Secret page"), "{alg:?}: plaintext leaked");
assert!(matches!(Document::open(saved.clone()), Err(CosError::NeedsPassword)), "{alg:?}");
@@ -620,37 +688,52 @@ fn output_security_does_not_depend_on_the_authentication_level() {
}
}
/// Compatible output security means the same `/Encrypt` object number, security dictionary, file
/// key and first file identifier. Each differs alone here; unencrypted documents always match.
/// Compatible output security means the same security dictionary and file key, and below
/// revision 5 the same first file identifier. Each differs alone here; where the dictionary is
/// stored is not compared, and unencrypted documents always match.
#[test]
fn output_security_compares_number_dictionary_key_and_first_id() {
fn output_security_compares_dictionary_key_and_first_id() {
let plain = || Document::open(Arc::new(fixture())).unwrap();
assert!(plain().has_same_output_security(&plain()), "two unencrypted documents");
let encrypted = Arc::new(protect(Algorithm::Aes256, "user", "owner", -1));
let open = || Document::open_with_password(encrypted.clone(), Some("owner")).unwrap();
assert!(open().has_same_output_security(&open()), "the same file opened twice");
assert!(!plain().has_same_output_security(&open()) && !open().has_same_output_security(&plain()));
// Only the first /ID differs: same dictionary, key and object number.
let mut other_id = open();
with_first_id(&mut other_id, b"another file id!");
assert!(!open().has_same_output_security(&other_id), "first /ID");
// The second /ID changes on every save and is not part of the key.
let Some(Object::Array(ids)) = open().trailer().get(b"ID").cloned() else { panic!("no /ID") };
let mut second = open();
second.trailer_mut().set(b"ID".to_vec(), Object::Array(vec![ids[0].clone(), Object::String(PdfString { bytes: b"x".to_vec(), hex: true })]));
assert!(open().has_same_output_security(&second), "second /ID");
// Same passwords, new key (and so a new dictionary).
for alg in ALL {
let mut rekeyed = open();
rekey(&mut rekeyed, alg, 3);
assert!(!open().has_same_output_security(&rekeyed), "{alg:?} rekey");
let encrypted = Arc::new(protect(alg, "user", "owner", -1));
let open = || Document::open_with_password(encrypted.clone(), Some("owner")).unwrap();
assert!(open().has_same_output_security(&open()), "{alg:?}: the same file opened twice");
assert!(!plain().has_same_output_security(&open()) && !open().has_same_output_security(&plain()), "{alg:?}");
// Only the first /ID differs. AES-256 (revision 6) draws its key at random instead.
let mut other_id = open();
with_first_id(&mut other_id, b"another file id!");
assert_eq!(open().has_same_output_security(&other_id), alg == Algorithm::Aes256, "{alg:?}: first /ID");
// The second /ID changes on every save and is never part of the key.
let Some(Object::Array(ids)) = open().trailer().get(b"ID").cloned() else { panic!("no /ID") };
let mut second = open();
second.trailer_mut().set(b"ID".to_vec(), Object::Array(vec![ids[0].clone(), Object::String(PdfString { bytes: b"x".to_vec(), hex: true })]));
assert!(open().has_same_output_security(&second), "{alg:?}: second /ID");
// Protected again with the same passwords: AES-256 draws a new key, and another algorithm
// is another dictionary. (Below revision 5 the same algorithm derives the same key from
// the passwords and /ID, so whether it matches is not what this checks.)
for other in ALL.into_iter().filter(|&other| other != alg || alg == Algorithm::Aes256) {
let mut rekeyed = open();
rekey(&mut rekeyed, other, 3);
assert!(!open().has_same_output_security(&rekeyed), "{alg:?} protected again as {other:?}");
}
// Only where the dictionary is stored differs: renumbered, or inline.
let moved = Document::open_with_password(Arc::new(protect_moved(alg)), Some("owner")).unwrap();
let inline = Document::open_with_password(Arc::new(protect_inline(alg, "user", "owner")), Some("owner")).unwrap();
assert_ne!(open().trailer().get(b"Encrypt"), moved.trailer().get(b"Encrypt"), "{alg:?}");
assert!(matches!(inline.trailer().get(b"Encrypt"), Some(Object::Dict(_))), "{alg:?}");
for other in [&moved, &inline] {
assert_eq!(open().security().unwrap().dict(), other.security().unwrap().dict(), "{alg:?}");
assert!(open().has_same_output_security(other) && other.has_same_output_security(&open()), "{alg:?}: dictionary place");
}
}
// Only the dictionary differs: AES-256 draws its file key from the seed, so new permissions
// or metadata encryption keep the key, /ID and object number.
// or metadata encryption keep the key and /ID.
for (permissions, encrypt_metadata) in [(!(4 | 16), true), (-1, false)] {
let mut a = plain();
rekey(&mut a, Algorithm::Aes256, 5);
@@ -667,45 +750,6 @@ fn output_security_compares_number_dictionary_key_and_first_id() {
let (sa, sb) = (a.output_handler().unwrap(), b.output_handler().unwrap());
assert_eq!(sa.file_key(), sb.file_key(), "{permissions}, {encrypt_metadata}");
assert_ne!(sa.dict(), sb.dict(), "{permissions}, {encrypt_metadata}");
assert_eq!(a.trailer().reference(b"Encrypt"), b.trailer().reference(b"Encrypt"));
assert!(!a.has_same_output_security(&b), "{permissions}, {encrypt_metadata}: security dictionary");
}
// Only the /Encrypt object number differs: the same seed gives the same /ID, dictionary and
// key, but an extra object moves the new dictionary to the next number.
for alg in ALL {
let mut a = plain();
rekey(&mut a, alg, 5);
let mut b = plain();
b.add(Object::Int(0));
rekey(&mut b, alg, 5);
let (sa, sb) = (a.output_handler().unwrap(), b.output_handler().unwrap());
assert_eq!(sa.dict(), sb.dict(), "{alg:?}");
assert_eq!(sa.file_key(), sb.file_key(), "{alg:?}");
assert_eq!(a.trailer().get(b"ID"), b.trailer().get(b"ID"), "{alg:?}");
assert_ne!(a.trailer().reference(b"Encrypt"), b.trailer().reference(b"Encrypt"), "{alg:?}");
assert!(!a.has_same_output_security(&b), "{alg:?}: /Encrypt object number");
let mut c = plain();
rekey(&mut c, alg, 5);
assert!(a.has_same_output_security(&c), "{alg:?}: identical setup matches");
}
// An inline /Encrypt dictionary has no object number, an indirect one does: the same
// dictionary and key cannot share an overlay, since only one of them is exempt by number.
for alg in ALL {
let inline = || Document::open_with_password(Arc::new(protect_inline(alg, "user", "owner")), Some("owner")).unwrap();
let indirect = Document::open_with_password(Arc::new(protect(alg, "user", "owner", -1)), Some("owner")).unwrap();
assert!(matches!(inline().trailer().get(b"Encrypt"), Some(Object::Dict(_))), "{alg:?}");
assert!(indirect.trailer().reference(b"Encrypt").is_some(), "{alg:?}");
let (si, sd) = (inline(), &indirect);
assert_eq!(si.security().unwrap().dict(), sd.security().unwrap().dict(), "{alg:?}");
assert_eq!(si.security().unwrap().file_key(), sd.security().unwrap().file_key(), "{alg:?}");
assert!(!si.has_same_output_security(sd) && !sd.has_same_output_security(&si), "{alg:?}: inline vs indirect");
assert!(inline().has_same_output_security(&inline()), "{alg:?}: inline opened twice");
let mut target = inline();
set_title(&mut target, "Saved title");
let before = state(&target);
assert!(matches!(target.overlay_changes_from(&indirect), Err(CosError::Security(_))), "{alg:?}");
assert!(before == state(&target), "{alg:?}: a refused overlay changed the target");
}
}
crates/engine/src/session.rs
+18 -17
View File
@@ -665,9 +665,10 @@ impl Session {
/// break its signatures, so with no edits it keeps its bytes, damage and all.
///
/// Undo past a save that changed security restores a snapshot whose encryption differs from
/// the last saved file. That snapshot is written in full too: an update appended to the saved
/// file would be read with the wrong key, and would keep the revision it was meant to replace
/// (#3276).
/// the last saved file. Like any change of security, that snapshot is written in full: an
/// update appended to the saved file would be read with the wrong key, and would keep the
/// revision it was meant to replace (#3276). In a signed document that fails with
/// [`EditError::SignedRewrite`] rather than invalidate the signatures.
pub fn save_bytes(&self, id: DocId) -> Result<Arc<Vec<u8>>, EditError> {
let doc = self.get(id).ok_or(EditError::NoDocument)?;
let Some(editor) = doc.editor.as_ref() else { return Ok(doc.bytes.clone()) };
@@ -682,25 +683,25 @@ impl Session {
// a reconstructed file) still writes the current graph on its own.
let rewrite = editor.cos.full_save_required() || editor.cos.encryption_changed() || editor.cos.revisions().is_empty();
let cos = writable(&editor.cos)?;
guard(|| {
let signed = doc.is_signed();
let written = guard(|| {
if on_saved || rewrite {
write_incremental(&cos, &opts)
return write_incremental(&cos, &opts).map(Some);
}
let mut base = pdfcraft_cos::Document::open_with_password(editor.saved.clone(), editor.saved_keys.reopen.as_deref())?;
// Undo may restore security from before the last save even though it has not changed
// relative to the snapshot's own source. That can't be appended to the saved file.
if base.overlay_changes_from(&cos)? {
write_incremental(&base, &opts).map(Some)
} else if signed {
Ok(None)
} else {
let mut base = pdfcraft_cos::Document::open_with_password(editor.saved.clone(), editor.saved_keys.reopen.as_deref())?;
// Undo may restore security from before the last save even though it has
// not changed relative to the snapshot's own source. Never combine that
// trailer with the saved file's encryption, or retain its obsolete bytes. A full
// write renumbers objects, so older encrypted snapshots usually land here again.
if !base.has_same_output_security(&cos) {
return write_full(&cos, &opts);
}
base.overlay_changes_from(&cos)?;
write_incremental(&base, &opts)
write_full(&cos, &opts).map(Some)
}
})
.map_err(EditError::Write)?
.map(Arc::new)
.map_err(|e| EditError::Write(e.to_string()))
.map_err(|e| EditError::Write(e.to_string()))?;
written.map(Arc::new).ok_or_else(|| EditError::SignedRewrite("Saving after undoing a security change".into()))
}
/// A compact, garbage-collected rewrite (Save As ▸ "Optimized" / Reduce File Size groundwork).
crates/engine/src/tests/security_save.rs
+87 -3
View File
@@ -161,8 +161,8 @@ fn undo_saved_rekey_with_the_same_passwords_restores_original_security() {
assert!(failures.is_empty(), "{}", failures.join("\n"));
}
/// A full save renumbers `/Encrypt`; undoing past it must not leave the restored dictionary
/// encrypted as an ordinary object.
/// A full save renumbers `/Encrypt` but keeps the encryption. Undo past it appends to that save,
/// keeping its revision, and leaves the restored dictionary unencrypted.
#[test]
fn undo_encrypted_compaction_restores_the_encrypt_dictionary_exemption() {
let mut cos = CosDocument::open(Arc::new(fixture(3))).unwrap();
@@ -184,10 +184,11 @@ fn undo_encrypted_compaction_restores_the_encrypt_dictionary_exemption() {
let compact_doc = CosDocument::open_with_password(compact.clone(), Some("owner")).unwrap();
assert_eq!(original.security().unwrap().dict(), compact_doc.security().unwrap().dict());
assert_ne!(original.trailer().reference(b"Encrypt"), compact_doc.trailer().reference(b"Encrypt"), "compaction renumbered /Encrypt");
s.mark_saved(id, compact, None).unwrap();
s.mark_saved(id, compact.clone(), None).unwrap();
s.undo(id).unwrap();
let restored = s.save_bytes(id).unwrap();
s.check_save(id, &restored).unwrap();
assert!(restored.starts_with(compact.as_slice()), "the same encryption still appends to the last save");
assert!(!restored.windows(b"(Page 1)".len()).any(|w| w == b"(Page 1)"), "page content stays encrypted");
assert!(CosDocument::open(restored.clone()).is_err() && CosDocument::open_with_password(restored.clone(), Some("wrong")).is_err());
let mut reopened = Session::new();
@@ -262,3 +263,86 @@ fn undo_saved_password_change_restores_original_passwords() {
assert!(CosDocument::open_with_password(restored.clone(), Some("new-user")).is_err());
assert!(CosDocument::open_with_password(restored, Some("new-owner")).is_err());
}
/// A signed document is never rewritten in full, even by this fallback. Signing clears the undo
/// history and security can't be changed once signed, so no edit reaches this state; the test
/// marks the document signed directly.
#[test]
fn undo_past_a_security_change_never_rewrites_a_signed_document() {
let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap();
let digital_id = sign::pkcs12::open(&p12, "test").unwrap();
// After the test certificate's start date (2026-10-02).
let mut signer = Session::new().with_clock(|| 1_800_000_000);
let signed_id = signer.open("signed.pdf", None, Arc::new(fixture(1)), None).unwrap();
let signed =
signer.sign(signed_id, &digital_id, SignOptions { page: 0, rect: Some([20.0, 20.0, 180.0, 60.0]), ..SignOptions::default() }).unwrap();
signer.mark_signed(signed_id, signed, None).unwrap();
let signatures = signer.get(signed_id).unwrap().signatures.clone();
let mut s = Session::new();
let id = s.open("encrypted.pdf", None, encrypted_fixture(Algorithm::Aes256, false), Some("owner")).unwrap();
s.apply(id, Edit::RemoveProtection).unwrap();
let plain = s.save_bytes(id).unwrap();
s.mark_saved(id, plain, None).unwrap();
s.undo(id).unwrap();
s.doc_mut(id).unwrap().signatures = signatures;
assert!(s.get(id).unwrap().is_signed());
assert!(matches!(s.save_bytes(id), Err(EditError::SignedRewrite(_))));
}
/// An optimized save packs objects into object streams under new numbers. Undo past it restores
/// a graph that puts other objects at those numbers: objects that look unchanged but sat in such
/// a stream must still be readable from the next save.
#[test]
fn undo_past_an_optimized_save_keeps_the_objects_it_packed() {
let mut encrypted = CosDocument::open(Arc::new(fixture(3))).unwrap();
encrypted
.set_encryption(&NewEncryption {
algorithm: Algorithm::Aes256,
user_password: "user",
owner_password: "owner",
permissions: -1,
encrypt_metadata: true,
seed: [7; 32],
})
.unwrap();
let encrypted = Arc::new(write_full(&encrypted, &SaveOptions::default()).unwrap());
for (bytes, password) in [(Arc::new(fixture(3)), None), (encrypted, Some("owner"))] {
let mut s = Session::new();
let id = s.open("a.pdf", None, bytes, password).unwrap();
s.apply(id, Edit::DeletePages { pages: vec![2] }).unwrap();
let compact = s.save_full_bytes(id).unwrap();
let packed = CosDocument::open_with_password(compact.clone(), password).unwrap();
s.mark_saved(id, compact, None).unwrap();
s.undo(id).unwrap();
// The premise: an object the snapshot keeps unchanged sits in a stream whose number the
// snapshot reuses for something else.
let current = &s.get(id).unwrap().editor.as_ref().unwrap().cos;
let numbers = current.object_numbers();
let at_risk: Vec<u32> = numbers
.iter()
.copied()
.filter(|&n| matches!(packed.xref_entry(n), Some(pdfcraft_cos::XrefEntry::InStream { stream, .. }) if numbers.contains(&stream)))
.filter(|&n| {
let r = pdfcraft_cos::ObjRef::new(n, current.generation(n));
*packed.get(r) == *current.get(r)
})
.collect();
// Only the encrypted file sets this up; the plain one checks there is no regression.
if password.is_some() {
assert!(!at_risk.is_empty(), "no unchanged packed object in a reused stream");
}
let restored = s.save_bytes(id).unwrap();
s.check_save(id, &restored).unwrap();
let saved = CosDocument::open_with_password(restored.clone(), password).unwrap();
assert!(saved.repair_log().is_empty(), "{password:?}: {:?}", saved.repair_log());
let current = &s.get(id).unwrap().editor.as_ref().unwrap().cos;
for num in current.object_numbers() {
let r = pdfcraft_cos::ObjRef::new(num, current.generation(num));
assert_eq!(*saved.get(r), *current.get(r), "{password:?}: object {num}");
}
let mut reopened = Session::new();
let rid = reopened.open("restored.pdf", None, restored, password).unwrap();
assert_eq!(page_texts(&reopened, rid), ["Page 1", "Page 2", "Page 3"], "{password:?}");
}
}