Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-114713: Handle case of an empty bytes object passed tozoneinfo.ZoneInfo
#132582
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
zoneinfo.ZoneInfo
zoneinfo.ZoneInfo
@@ -83,10 +83,14 @@ def find_tzfile(key): | |||
def _validate_tzfile_path(path, _base=_TEST_PATH): | |||
if not path: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This wasn't correct.
If we passNone
, instead of aTypeError
we'll get aValueError
, so we need more exact check here.
elif isinstance(path, bytes) and path == b"": | ||
raise ValueError( | ||
"ZoneInfo key must not be an empty bytes object" | ||
) |
WolframAlphApr 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Does it make sense maybe to just emitZoneInfo key must not be empty
for both string and bytes? Also, since we check for passed data types, does it make sense to also restrict them to str/bytes? Currently, passing for instance[]
results inTypeError: unhashable type: 'list'
somewhere down the road. Maybe worth returning meaningful exception saying what allowed types are? We anyway check passed in types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, that makes sense. Though I think that the current approach is wrong.
We should insert these two checks (actually, bytes objects shouldn't be passed to thezoneinfo.ZoneInfo
) to C and Python implementation ofZoneInfo.__new__
:
- Check that
key
is string (or subclass of string) - Check that
key
is a non-empty string.
WDYM?
Uh oh!
There was an error while loading.Please reload this page.
''
#114713