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
prevent long strings as int inputs #4480
Conversation
should now pass, please review. |
@pydantic/pydantic-maintainers do you think we should backport this to v1.9 and v1.8? |
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.
LGTM 👍
I think we don't need backporting this because this is not Pydantic issue and the proper fix will be out in Python patch releases. Also, 1.10.x is mostly backward compatible release and users can have the fix by upgrading to 1.10.x
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.
LGTM 👍
I think we don't need backporting this because this is not Pydantic issue and the proper fix will be out in Python patch releases. Also, 1.10.x is mostly backward compatible release and users can have the fix by upgrading to 1.10.x
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.
LGTM 👍
I think we don't need backporting this because this is not Pydantic issue and the proper fix will be out in Python patch releases. Also, 1.10.x is mostly backward compatible release and users can have the fix by upgrading to 1.10.x
sorry. I think it was my internet problem. I approved the PR 3 times 🤦♂️ |
No problem. Good to see you're sure 😄. |
# NOTICE: this does not fully protect user from the DOS risk since the standard library JSON implementation | ||
# (and other std lib modules like xml) use `int()` and are likely called before this, the best workaround is to | ||
# 1. update to the latest patch release of python once released, 2. use a different JSON library like ujson | ||
if isinstance(v, (str, bytes, bytearray)) and len(v) > max_str_int: |
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.
maybe overkill, but should this include memoryview
as well?
from pydantic import BaseModel
class A(BaseModel):
x: int
A(x=memoryview(b"1" * 5000))
this seems like a much less common use case, but perhaps could still be used as a dos in some cases
$ python -m timeit -s 'x=b"1" * 100_000' 'int(memoryview(x))'
5 loops, best of 5: 59.6 msec per loop
i can make a pr to address this
see #1477 and in turn, python/cpython#95778 this check should be unnecessary once patch releases are out for 3.7, 3.8, 3.9 and 3.10 but better to check here until then.
See this twitter thread the python patch releases should be out in the next few days, but I know how slow people are to upgrade these things, so I think better to fix here.
NOTICE:
This does not full protect user from the DOS risk since the standard library JSON implementation
(and other std lib modules like xml) use
int()
and are called before this, the best workaround is toRelated issue number
fix #1477
Checklist
changes/<pull request or issue id>-<github username>.md
file added describing change