diff options
author | Robin Watts <Robin.Watts@artifex.com> | 2025-04-16 18:09:25 +0100 |
---|---|---|
committer | Robin Watts <Robin.Watts@artifex.com> | 2025-04-16 18:59:47 +0100 |
commit | 379ef94774d4606dc7fe7a46ae931e8ddf41cf4f (patch) | |
tree | 669365b84a68ab6d6c6c0f522c97f99067b0730d | |
parent | 7f231d48791dd056bcfc1af60d2d1df1e9192dab (diff) |
Bug 708292: Tweak ObjStm repairs.
When we repair a file, we:
1) Ditch all the objects.
2) Scan through the file lexing for objects and reading them in
3) During this scan if we encounter a second or subsequent version
of an object, we keep that one in preference to any previous one.
4) Once we've scanned the objects, we then run through the xref
looking for ObjStm objects.
5) For every ObjStm object we encounter, we run through it creating
xref entries for each object in the ObjStm.
6) We are at pains to never allow an Object in an ObjStm to override
an object that appears after it in the file. This was originally
done to guard against incremental updates that override an object
in an ObjStm with a normal object, but as this file shows, it
can also help us guard against objects in ObjStms being replaced
with other objects in ObjStms.
7) We then run through the file ensuring that all objects that claim
to be in ObjStms really point to objects that are ObjStms.
As part of the check in step 6) we were looking up the offset for
a given objstm object, and asserting that it was an objstm. This
bug is caused by the assert triggering.
As we process the example file, we first encounter object 2, an
objstm that contains object 5. This is no problem.
We then encounter object 6 that contains both object 2 and object 5.
Processing object '2' changes object '2' from being an objstm
to being an object in an objstm. When we process object 5, we lookup
the existing definition, expecting it to be within an objstm - but
object 2 is no longer such. This causes the assert.
If we had just continued through, we'd have been fine, as stage 7)
would have removed any bad entries at the end.
The fix is to remove the assert, and handle it as a special case.
-rw-r--r-- | source/pdf/pdf-repair.c | 19 |
1 files changed, 15 insertions, 4 deletions
diff --git a/source/pdf/pdf-repair.c b/source/pdf/pdf-repair.c index 409f9d6ac..17bd189aa 100644 --- a/source/pdf/pdf-repair.c +++ b/source/pdf/pdf-repair.c @@ -275,7 +275,10 @@ entry_offset(fz_context *ctx, pdf_document *doc, int num) /* It must be in a stream. Return the entry of that stream. */ entry = pdf_get_populating_xref_entry(ctx, doc, entry->ofs); - assert(entry->type == 'n'); + /* If it's NOT in a stream, then we'll invalidate this entry in a moment. + * For now, just return an illegal offset. */ + if (entry->type != 'n') + return -1; return entry->ofs; } @@ -333,10 +336,18 @@ pdf_repair_obj_stm(fz_context *ctx, pdf_document *doc, int stm_num) if (entry->type != 0 && entry->type != 'f') { int64_t existing_entry_offset = entry_offset(ctx, doc, n); - int64_t this_entry_offset = entry_offset(ctx, doc, stm_num); - if (existing_entry_offset > this_entry_offset) - replace = 0; + if (existing_entry_offset < 0) + { + /* The existing entry is invalid. Anything must be better than that! */ + } + else + { + int64_t this_entry_offset = entry_offset(ctx, doc, stm_num); + + if (existing_entry_offset > this_entry_offset) + replace = 0; + } } if (replace) |