Fix ParserError on subsecond precision beyond nanoseconds - #1020
Open
afonsojanu wants to merge 1 commit into
Open
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
The pure-Python datetime parsers (used as a fallback when the compiled
Rust extension is unavailable, e.g. PENDULUM_EXTENSIONS=0, and for the
"common" format that has no Rust equivalent) capped the fractional
seconds group at 9 digits (\\d{1,9}). A 10th digit made the whole
string fail to match, raising ParserError instead of parsing it.
The value is already truncated to 6 digits (microseconds) a few lines
below, so the cap served no purpose beyond rejecting otherwise valid
input. Python's datetime.fromisoformat() has no such limit and just
truncates, which is the behavior restored here by widening both
regexes to \\d+.
Fixes ParserError on inputs like:
pendulum.parse("2001-01-01T12:34:56.1234567890Z")
pendulum.parse("2016/10/06 12:34:56.1234567890")
This branch has not been deployed
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.
Summary
pendulum.parse()raisesParserErrorfor an otherwise valid-looking timestamp when the fractional-seconds part has more than 9 digits, e.g.:datetime.fromisoformat()has no such limit and just truncates to microseconds:Reproduces with the "common" datetime format too (this path has no Rust equivalent, so it's hit regardless of the compiled extension):
Root cause
Both the pure-Python ISO 8601 parser (
pendulum/parsing/iso8601.py, used as a fallback when the compiled extension isn't available, e.g.PENDULUM_EXTENSIONS=0) and the "common" format parser (pendulum/parsing/__init__.py, always pure Python) match the subsecond group with\d{1,9}. A 10th digit makes the whole regex fail to match, so the entire string is rejected.Both call sites already truncate the captured group to 6 characters a few lines below before converting it to microseconds:
so the
{1,9}cap wasn't protecting anything downstream, it was just rejecting valid input before it ever got there.Fix
Widened both regexes from
\d{1,9}to\d+. The existing truncate-to-6-and-pad logic already does the right thing for any number of digits, so no other changes were needed.Tests
test_rfc_3339_extended_beyond_nanosecondsandtest_common_format_extended_beyond_nanosecondstotests/parsing/test_parsing.py, covering the publicparse()API for both the ISO 8601 and "common" format paths.test_parse_iso8601_subsecond_beyond_nanoseconds_pure_pythontotests/parsing/test_parse_iso8601.py, importingpendulum.parsing.iso8601.parse_iso8601directly so the pure-Python parser is exercised even when the compiled extension is installed (the Rust parser doesn't have this bug since it isn't regex-based).Confirmed all three new tests fail with
ParserErroragainst the unpatched regexes and pass after the fix. Ran the full test suite (pytest tests/, with the Rust extension built viamaturin develop --release): 1850 passed, 3 skipped (pre-existing, unrelated to this change), no new failures or warnings.Closes #935.