Date a document when it is written, not when it is opened - #496
Open
jafin wants to merge 3 commits into
Open
Conversation
Opening a document for modification set its modification date to the current time, there and then. Nothing had been modified yet, and nothing need ever be: a caller that opened a document to read its dates was handed a date that its own reading had just written, and a caller that opened one and thought better of it left with a document dated as though it had been edited. There was no way to tell a document that had been changed from one that had only been looked at, which is the one thing a modification date is for. Stamp it in PrepareForSave instead, where the creator and the producer are already filled in, and only for a document PdfReader opened for modification. A document written from scratch is dated by its creation date alone, as it has always been, and carries no /ModDate at all. A date the caller has chosen is left alone, which is what opening used to leave room for by stamping first and saving later. The property records that it was set from outside, and the stamp goes in through the element rather than the property, so that saving twice stamps twice rather than taking the first stamp for a date somebody asked for. The test for what opening reports compares it against what opening the same file read only reports, which never stamped anything. That asks whether opening for modification answers differently, and leaves out of it how a date is spelled in a file and what time zone it comes back in.
Opening for modification changes plenty about the document in memory: the block above the comment gives it new document IDs, and the one below drops unreachable objects, flattens the page tree and renumbers. Saying that opening does not modify it overstated what had changed to what anyone reading the file would take as a promise. Narrow it to the date.
The change comes from a fork that has since moved to AwesomeAssertions and to target frameworks where Encoding.Latin1 exists. Neither is true here: FluentAssertions is what the test project references, and Encoding.Latin1 arrived in .NET 5 while this project still targets netcoreapp3.1. The bytes being searched for are ASCII either way.
There was a problem hiding this comment.
Pull request overview
This PR fixes an observed metadata side-effect where opening a PDF in Modify mode immediately updated /ModDate, even if the document was never saved. It moves modification-date stamping to save time and adds regression tests to ensure /ModDate reflects actual writes while preserving caller-specified dates.
Changes:
- Move modification-date stamping from
PdfReader.Open(..., Modify)toPdfDocument.PrepareForSave(). - Track whether the modification date was explicitly set by the caller to avoid overwriting it on save.
- Add a dedicated test suite covering open/save behaviors for
/ModDate.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| PdfSharpCore/Pdf/PdfDocumentInformation.cs | Adds a flag to detect caller-set modification dates so save-time stamping can skip them. |
| PdfSharpCore/Pdf/PdfDocument.cs | Stamps /ModDate during PrepareForSave only for imported documents opened in Modify mode (unless caller-set). |
| PdfSharpCore/Pdf.IO/PdfReader.cs | Removes open-time /ModDate stamping to prevent read-only-like operations from mutating metadata. |
| PdfSharpCore.Test/IO/ModificationDateTests.cs | Adds regression tests validating open-time reporting and save-time stamping behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| /// <summary> | ||
| /// (Optional; PDF 1.1) The document�s title. | ||
| /// (Optional; PDF 1.1) The document�s title. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #365.
The problem
PdfReader.Opensets the modification date to the current time as part of opening a document for modification, before anything has been modified and whether or not the document is ever written:PdfSharpCore/Pdf.IO/PdfReader.cs, in theopenmode == PdfDocumentOpenMode.Modifyblock.Opening a file and touching nothing else is enough to date it as edited:
That leaves no way to tell a document that has been changed from one that has only been read, which is the one thing a modification date is for. The reporter of #365 worked around it by opening read only.
The change
The stamp moves to
PdfDocument.PrepareForSave, where the creator and the producer are already filled in. The condition is the one that was there before, evaluated at save time: a documentPdfReaderopened for modification.IsImportedis part of it becauseModifyis the zero value ofPdfDocumentOpenMode, so a newly created document would otherwise match it.Two details:
PdfDocumentInformation.ModificationDatenow records that it was set from outside, and the stamp skips it.A document written from scratch is unaffected: dated by its creation date alone, carrying no
/ModDate, as before.Behaviour change to be aware of
Code that reads
Info.ModificationDatestraight afterPdfReader.Open(..., PdfDocumentOpenMode.Modify)and expects a value now gets whatever the file says, which for a document with no/ModDateisDateTime.MinValue. That is the fix rather than a side effect, but it is a visible change and worth knowing before merging.Tests
PdfSharpCore.Test/IO/ModificationDateTests.cs, six tests: what opening reports, that an undated document stays undated, that writing stamps, that writing twice stamps twice, that a caller's date survives a save, and that an authored document is not stamped. Four of them fail without the change.The test for what opening reports compares it against opening the same file read only, rather than against a literal date, which keeps it clear of how a date is spelled in a file and what time zone it comes back in.
The tests build their documents in memory and draw nothing, so they need no font resolver and no assets.
Note on the existing test suite
Five
XTextFormatterTestcases fail on my machine, and they fail the same way on an unmodified checkout ofmaster, so they are not from this change — they compare rendered pages against PNG baselines and are sensitive to the fonts installed. They may well be green on the Linux CI runner the baselines came from.