Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datetime leap day parsing warning on python3.13 #1451

Open
rafsaf opened this issue Oct 24, 2024 · 3 comments
Open

Datetime leap day parsing warning on python3.13 #1451

rafsaf opened this issue Oct 24, 2024 · 3 comments

Comments

@rafsaf
Copy link

rafsaf commented Oct 24, 2024

Hello! There is a warning in the code of time.py on 3.13:

/home/rafsaf/.cache/pypoetry/virtualenvs/ogion-RX2QUkiy-py3.13/lib/python3.13/site-packages/minio/time.py:77: DeprecationWarning: Parsing dates involving a day of month without a year specified is ambiguious
  and fails to parse leap day. The default behavior will change in Python 3.15
  to either always raise an exception or to use a different default year (TBD).
  To avoid trouble, add a specific year to the input & format.
  See https://github.com/python/cpython/issues/70647.
    day = datetime.strptime(value[4:8], " %d ").day

It's related to this PR in cpython: https://github.com/python/cpython/pull/117107/files

long story short looking at time.py module at first glance:

  • remove _WEEK_DAYS and _MONTHS, both from_http_header and to_http_header should use datetime.strptime(value, "%a, %d %b %Y %H:%M:%S %Z") (and equivalent strftime) instead of hand crafted parsing RFC2616 date format.
  • bonus- remove _UTC_IMPORTED as its not needed, return datetime.now(timezone.utc) should do the trick

If anybody want to play with that, feel free, i will find myself some spare time in a few days to prepare PR

PS. Maybe I'm wrong and "%a, %d %b %Y %H:%M:%S %Z" is wrong for some reason in some situations and that's why it's custom code here? But I am not aware of such thing.

Rafał

@balamurugana
Copy link
Member

@rafsaf We have to time format with US locale irrespective of system's locale. I didn't find a way like below java code in python. The custom code ensures to locale US.

  public static final DateTimeFormatter HTTP_HEADER_DATE_FORMAT =
      DateTimeFormatter.ofPattern("EEE',' dd MMM yyyy HH':'mm':'ss 'GMT'", Locale.US).withZone(UTC);

If we fix it like java code, it is much better.

@rafsaf
Copy link
Author

rafsaf commented Oct 25, 2024

Thanks! That is valuable input about the context @balamurugana, indeed looks interesting.

At minimum %Y can be added for the " %d " part to fix the actual problem and not try to change the world :p

As I said I can look at it, have a nice day!

@balamurugana
Copy link
Member

@rafsaf I guess below is the fix

diff --git a/minio/time.py b/minio/time.py
index 39a57c6..6dc3552 100644
--- a/minio/time.py
+++ b/minio/time.py
@@ -74,7 +74,10 @@ def from_http_header(value: str) -> datetime:
             f"time data {value} does not match HTTP header format")
     weekday = _WEEK_DAYS.index(value[0:3])
 
-    day = datetime.strptime(value[4:8], " %d ").day
+    if value[4] != " " or value[7] != " ":
+        raise ValueError(
+            f"time data {value} does not match HTTP header format")
+    day = int(value[5:7])
 
     if value[8:11] not in _MONTHS:
         raise ValueError(

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

No branches or pull requests

2 participants