Skip to content

Raise TimestampParseError instead of OverflowError for out-of-range timestamps - #101

Open
eeshsaxena wants to merge 1 commit into
cdown:developfrom
eeshsaxena:timestamp-overflow
Open

Raise TimestampParseError instead of OverflowError for out-of-range timestamps#101
eeshsaxena wants to merge 1 commit into
cdown:developfrom
eeshsaxena:timestamp-overflow

Conversation

@eeshsaxena

Copy link
Copy Markdown

Noticed that a timestamp with enough digits in one field makes parse() (and srt_timestamp_to_timedelta directly) leak a raw OverflowError instead of the usual TimestampParseError:

>>> list(srt.parse("1\n9999999999999999:00:00,000 --> 00:00:01,000\nhi\n\n"))
OverflowError: Python int too large to convert to C int

The regex is happy to match arbitrarily long numeric fields, but timedelta only spans about 2.7 million years, so building one overflows. Wrapped that construction so it surfaces as TimestampParseError like every other unparseable timestamp, and added a couple of tests.

@eeshsaxena

Copy link
Copy Markdown
Author

Hi! Gentle nudge on this one whenever you have some bandwidth. It's a small, self-contained fix (Raise TimestampParseError instead of OverflowError for out-of-range timestamps), and it's currently mergeable with no conflicts. No urgency at all, and I'm happy to make any changes you'd like. Thanks for maintaining srt!

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.

1 participant