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

CreationDate metadata shows wrong timezone offset for UTC #1261

Closed
stevesimmons opened this issue Sep 9, 2024 · 7 comments · Fixed by #1290
Closed

CreationDate metadata shows wrong timezone offset for UTC #1261

stevesimmons opened this issue Sep 9, 2024 · 7 comments · Fixed by #1290

Comments

@stevesimmons
Copy link

stevesimmons commented Sep 9, 2024

Dates such as the PDF metadata's CreationDate have the general format "D:".

The datetime format strings in PDFDate.serialize() (included below) are correct for non-UTC: the timezone is shown as "+01'00'" or "-01'00'" for times ahead/behind of UTC.

However for UTC, the code in PDFDate.serialize() is given on line 318 of fpdf.syntax as f"D:{self.date:%Y%m%d%H%M%SZ%H'%M'}". This incorrectly takes the timestamp's hour and seconds as the timezone.

I think in this UTC case, the serialized date should either finish at the "Z", or have a timezone of "+00'00'". Printing "Z00'00'" for UTC looks wrong (I've only ever seen "Z" terminated strings", and printing "Z15:22" (my current time) looks very wrong indeed.

The incorrect line of fpdf.syntax is here:

class PDFDate:
    ...
    def serialize(self, _security_handler=None, _obj_id=None):
        if self.with_tz:
            assert self.date.tzinfo
            if self.date.tzinfo == timezone.utc:
                out_str = f"D:{self.date:%Y%m%d%H%M%SZ%H'%M'}"   # This should finish at 'Z'                            
                # out_str = f"D:{self.date:%Y%m%d%H%M%SZ}"   # Instead finish at 'Z' like this            
                # out_str = f"D:{self.date:%Y%m%d%H%M%S+00'00'}"   # Or make UTC explicit with "+00'00'"
else:
                out_str = f"D:{self.date:%Y%m%d%H%M%S%z}"   
                out_str = out_str[:-2] + "'" + out_str[-2:] + "'"
        else:
            out_str = f"D:{self.date:%Y%m%d%H%M%S}"
@Lucas-C
Copy link
Member

Lucas-C commented Sep 9, 2024

Thank you for reporting this bug @stevesimmons 👍

Your analysis seems accurate to me.

Would you like to sumbit a Pull Request in order to fix this?

We already have a unit test for this case in test/metadata/test_set_date.py
I guess it should be fixed.
FYI, we have some documentation on how to regenerate reference PDF files used in unit tests there: https://py-pdf.github.io/fpdf2/Development.html#assert_pdf_equal-writing-new-tests

@Lucas-C
Copy link
Member

Lucas-C commented Sep 9, 2024

@allcontributors please add @stevesimmons for bug

Copy link

@Lucas-C

I've put up a pull request to add @stevesimmons! 🎉

@Shenile
Copy link

Shenile commented Oct 17, 2024

Hi @stevesimmons,

I noticed that the issue regarding the incorrect timezone offset for the CreationDate metadata (#1261) is marked as open, and I wanted to check if it has been fixed in any recent updates.

If it's still open, I would love to contribute to this issue. Could you provide any guidance on how best to proceed or if there are any existing discussions I should review?

Thank you!

@Lucas-C
Copy link
Member

Lucas-C commented Oct 17, 2024

I noticed that the issue regarding the incorrect timezone offset for the CreationDate metadata (#1261) is marked as open, and I wanted to check if it has been fixed in any recent updates.

Yes, this issue is still open!

If it's still open, I would love to contribute to this issue. Could you provide any guidance on how best to proceed or if there are any existing discussions I should review?

Great, you are welcome!
Are you planning to contribute as part of Hacktoberfest?

A starting point would be to read this page: https://py-pdf.github.io/fpdf2/Development.html

Then you could fork this repository, create a dedicated branch (say issue-1261), fix PDFDate.serialize() in fpdf/syntax.py and run the unit tests. test/metadata/test_set_date.py should then fail, and you will have to regenerate the associated reference PDF file.
Once you have done that, you can add a mention of this fix in CHANGELOG.md and create a Pull Request from your branch!

@Shenile
Copy link

Shenile commented Oct 18, 2024

I noticed that the issue regarding the incorrect timezone offset for the CreationDate metadata (#1261) is marked as open, and I wanted to check if it has been fixed in any recent updates.

Yes, this issue is still open!

If it's still open, I would love to contribute to this issue. Could you provide any guidance on how best to proceed or if there are any existing discussions I should review?

Great, you are welcome! Are you planning to contribute as part of Hacktoberfest?

Yes, I am planning to contribute as part of Hacktoberfest!

A starting point would be to read this page: https://py-pdf.github.io/fpdf2/Development.html

Then you could fork this repository, create a dedicated branch (say issue-1261), fix PDFDate.serialize() in fpdf/syntax.py and run the unit tests. test/metadata/test_set_date.py should then fail, and you will have to regenerate the associated reference PDF file. Once you have done that, you can add a mention of this fix in CHANGELOG.md and create a Pull Request from your branch!

I created a branch named issue-1261, fixed the PDFDate.serialize() method in fpdf/syntax.py, and ran the unit tests. As expected, the test in test/metadata/test_set_date.py failed, so I regenerated the associated reference PDF file. I also added a mention of this fix in CHANGELOG.md.

I’ve submitted the pull request and look forward to your feedback!

@Lucas-C
Copy link
Member

Lucas-C commented Nov 21, 2024

PR #1290 has been merged to fix that.

While the fix has not been released in a new version published on Pypi, you can test that it works for you using:

pip install git+https://github.com/py-pdf/fpdf2.git@master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants