Repository navigation
Don't crash on blank PO-Revision-Date / POT-Creation-Date headers - #1293
Conversation
|
See #1222 for previous discussion. |
|
Thanks for the pointer -- I read through the #1222 discussion. To make sure this PR doesn't repeat the concern raised there ("I don't think we should be silently ignoring malformed dates" / silent data loss on unparseable values): #1222 was about any unparseable date string being silently swallowed, discarding a value the user actually wrote. This PR is narrower and only triggers on a genuinely blank header value ( |
|
Heads up for maintainers: there are currently three open PRs addressing this exact problem (blank
Notes on the differences:
It would be great if @akx (or whoever picks this up) can choose a preferred approach so two of the three can be closed — happy to re-review whichever direction wins. All three reference the same motivating case, and all include tests, which is nice to see. |
ba35e3f to
e2873dd
Compare
Some tools (e.g. Poedit) can leave the PO-Revision-Date header blank in a generated .po file instead of omitting it or using the conventional 'YEAR-MO-DA HO:MI+ZONE' placeholder. Parsing such a file crashed with: ValueError: time data '' does not match format '%Y-%m-%d %H:%M' because _parse_datetime_header() was called unconditionally with the blank value. Guard both call sites (po-revision-date and, for the same reason, pot-creation-date) to skip parsing when the header value is blank, keeping the previous/default value instead -- the same treatment already given to the 'YEAR-MO-DA HO:MI+ZONE' placeholder value just above, and to a blank Language header elsewhere in this same method. Added a regression test that parses a message block with both headers blank and asserts: no exception is raised, the catalog's creation_date/revision_date fall back to their defaults rather than being corrupted, and no literal 'None' leaks into the round-tripped mime_headers output. Confirmed the test fails with the original ValueError without the fix, and passes with it. Also verified end-to-end with an actual .po file parse + write round-trip (using an installed release build with CLDR data, since a shallow clone here can't build that data), and confirmed normal (non-blank) header values are completely unaffected. Ran the full existing test_catalog.py and test_pofile.py suites: all pass, no regressions. Fixes python-babel#1219
e2873dd to
3735f07
Compare
There was a problem hiding this comment.
🟢 Approval recommended
No active issues are reported, and the change includes targeted regression coverage for blank date headers and preservation of existing defaults.
No open issues 🎉
What changed in this PR
This pull request prevents Babel from crashing when .po files contain blank POT-Creation-Date or PO-Revision-Date headers. Blank values are ignored so existing catalog defaults remain intact.
Changes:
- Guard date parsing against blank creation and revision date headers.
- Add regression coverage for blank headers and default preservation.
- Verify that round-tripped MIME headers do not contain
None.
| File | Description |
|---|---|
tests/messages/test_catalog.py |
Adds regression coverage for blank date headers and default preservation. |
babel/messages/catalog.py |
Skips parsing blank creation and revision date headers. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1293 +/- ##
=======================================
Coverage 93.66% 93.66%
=======================================
Files 27 27
Lines 4970 4971 +1
=======================================
+ Hits 4655 4656 +1
Misses 315 315
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Fixes #1219.
Some tools (e.g. Poedit) can leave the
PO-Revision-Dateheader blank in a generated.pofile instead of omitting it or using the conventionalYEAR-MO-DA HO:MI+ZONEplaceholder. Parsing such a file crashes with:because
_parse_datetime_header()is called unconditionally with the blank value, as reported in the issue.Fix: guard both call sites (
po-revision-dateand, for the same reason,pot-creation-date) to skip parsing when the header value is blank, keeping the previous/default value instead -- the same treatment already given to theYEAR-MO-DA HO:MI+ZONEplaceholder value right above it, and to a blankLanguageheader elsewhere in this same method (self._set_locale(value or None)).Testing: added a regression test that parses a message block with both headers blank and asserts: no exception is raised,
creation_date/revision_datefall back to their defaults rather than being corrupted, and no literalNoneleaks into the round-trippedmime_headersoutput. I confirmed the test fails with the originalValueErrorwithout the fix, and passes with it.I also verified end-to-end with an actual
.pofile parse + write round-trip (using an installed release build with CLDR data, since a shallow clone here can't build that data on its own), confirming noNoneleaks into the output and that normal (non-blank) header values are completely unaffected. Ran the full existingtest_catalog.pyandtest_pofile.pysuites: all pass, no regressions.