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

_dump_date year conversion doesn't zfill(4) #1739

Closed
nicolary opened this issue Feb 25, 2020 · 4 comments
Closed

_dump_date year conversion doesn't zfill(4) #1739

nicolary opened this issue Feb 25, 2020 · 4 comments
Milestone

Comments

@nicolary
Copy link
Contributor

@nicolary nicolary commented Feb 25, 2020

I have some dates that have a year before year 1000 and the _dump_date function doesn't represent them as zero filled to 4 so parsing the output with '%a, %d %b %Y %H:%M:%S %Z' throws an exception.

index b428ceeb..d233d0f2 100644
--- a/src/werkzeug/http.py
+++ b/src/werkzeug/http.py
@@ -882,7 +882,7 @@ def _dump_date(d, delim):
             "Dec",
         )[d.tm_mon - 1],
         delim,
-        str(d.tm_year),
+        str(d.tm_year).zfill(4),
         d.tm_hour,
         d.tm_min,
         d.tm_sec,
@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Feb 25, 2020

I'm not clear why dates prior to 1000 are relevant in an HTTP context. Can you explain why you need Werkzeug's http_date for this?

@nicolary

This comment has been minimized.

Copy link
Contributor Author

@nicolary nicolary commented Feb 25, 2020

_dump_date is used as the default datetime and date encoding function in Flask's JSONEncoder class:

https://github.com/pallets/flask/blob/master/src/flask/json/__init__.py

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Feb 25, 2020

Since you have specialized data, you should probably be using a better representation than HTTP dates anyway. I'd recommend overriding app.json_encoder, or using something like Marshmallow to serialize your data first. I guess I'm fine with a PR, it just doesn't seem very relevant to typical HTTP date use cases.

@nicolary

This comment has been minimized.

Copy link
Contributor Author

@nicolary nicolary commented Feb 25, 2020

I did override it. I figured I'd mention it in case you wanted to apply the included patch - it mitigates this python issue https://bugs.python.org/issue13305 in this case.

Since the function is being used for common date encoding in Flask maybe they should rethink the http_date use in their default encoder...

Thanks for the response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.