Skip to content

Reject out-of-range UTC offsets in the pure-Python parsers - #1015

Open
Sharadhi-v98 wants to merge 1 commit into
python-pendulum:masterfrom
Sharadhi-v98:parser-reject-oversized-offset
Open

Sharadhi-v98 wants to merge 1 commit into
python-pendulum:masterfrom
Sharadhi-v98:parser-reject-oversized-offset

Conversation

@Sharadhi-v98

Copy link
Copy Markdown
  1. The ISO 8601 parser (parse_iso8601) and from_format both build a timezone offset from the parsed hour and minute without bounding its magnitude, so an offset like +99:59 is accepted.
  2. That yields a FixedTimezone whose utcoffset sits far outside the plus/minus 24h datetime permits, and the returned value raises the moment it is used (isoformat, astimezone, comparison across zones).

Added an upper bound in both callees, matching the Rust parser which already rejects offsets above 24 hours.

  • Added tests for changed code.
  • Updated documentation for changed code.

offset = ((int(off_hour) * 60) + int(off_minute)) * 60

if abs(offset) >= 24 * 3600:
raise ValueError("Invalid date")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this would be more helpful to users as

Suggested change
raise ValueError("Invalid date")
raise ValueError("Invalid date: timezone offset is out of range")

offset = ((int(off_hour) * 60) + int(off_minute)) * 60

if abs(offset) >= 24 * 3600:
raise ParserError(f"Invalid date string: {text}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with the other errors we raise

Suggested change
raise ParserError(f"Invalid date string: {text}")
raise ParserError("Invalid ISO 8601 timezone offset")

(with or without the input value - up to you)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants