-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Rule S320 should be removed #13707
Comments
Do you have a reference that |
I also found PyCQA/bandit#767 but it's very unclear to me what the recommendation is... |
There is this section in the FAQ page:
|
The current lxml version is 5.0, which seems to be safe by default against entity expansion (since it disables it). So, idk if ruff is able to check the version of lxml, or perhaps check to see if there are any places where |
Yeah, it seems but I would like some more confirmation before deprecating a rule.
Checking the version is beyond what Ruff can do today but checking for |
This is not entirely true. The 5.0 version only disables the expansion of external entities by default: https://lxml.de/FAQ.html#how-do-i-use-lxml-safely-as-a-web-service-endpoint
https://lxml.de/5.0/changes-5.0.0.html
The default value for the And it is still The change protects from a file inclusion attack, but not from quadratic blowup. |
Bandit removed both S410 and S320 from their blacklist since Bandit 1.8.1 (source: https://github.com/PyCQA/bandit/releases/tag/1.8.1). |
The documentation was changed on a pre-release version. Alas, it seems that defusedxml is now largely unmaintained. |
I put up a PR to deprecate the rule (#16680). We should remove it in Ruff 0.11 (or 0.12) |
…6680) ## Summary Deprecate `S320` because defusedxml has deprecated there `lxml` module and `lxml` has been hardened since. flake8-bandit has removed their implementation as well (PyCQA/bandit#1212). Addresses #13707 ## Test Plan I verified that selecting `S320` prints a warning and fails if the preview mode is enabled.
…6680) ## Summary Deprecate `S320` because defusedxml has deprecated there `lxml` module and `lxml` has been hardened since. flake8-bandit has removed their implementation as well (PyCQA/bandit#1212). Addresses #13707 ## Test Plan I verified that selecting `S320` prints a warning and fails if the preview mode is enabled.
…6680) ## Summary Deprecate `S320` because defusedxml has deprecated there `lxml` module and `lxml` has been hardened since. flake8-bandit has removed their implementation as well (PyCQA/bandit#1212). Addresses #13707 ## Test Plan I verified that selecting `S320` prints a warning and fails if the preview mode is enabled.
This comment seems to have been overlooked (at least was not responded to).
If lxml is fixed, then from what I can understand of rule S320, it should be removed as well.
The text was updated successfully, but these errors were encountered: