Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(rules): detect httpx for S113 #12174

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

mkniewallner
Copy link
Contributor

@mkniewallner mkniewallner commented Jul 3, 2024

Summary

Bandit now also reports B113 on httpx (PyCQA/bandit#1060). This PR implements the same logic, to detect missing or None timeouts for httpx alongside requests.

Test Plan

Snapshot tests.

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks -- this looks good to me!

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jul 3, 2024
@mkniewallner mkniewallner marked this pull request as ready for review July 3, 2024 22:45
@charliermarsh charliermarsh merged commit 8210c1e into astral-sh:main Jul 3, 2024
20 checks passed
@mkniewallner mkniewallner deleted the feat/detect-httpx-S113 branch July 4, 2024 07:00
@jyggen
Copy link

jyggen commented Jul 5, 2024

Not sure a missing timeout is a problem since httpx uses 5s as default. The rule is about preventing the client from waiting indefinitely, which it never will in the case of httpx and a missing timeout. Setting timeout to None is still problematic though.

@trim21
Copy link
Contributor

trim21 commented Jul 6, 2024

httpx has default timeout, this PR would be a false positive

https://www.python-httpx.org/advanced/timeouts/

HTTPX is careful to enforce timeouts everywhere by default.

The default behavior is to raise a TimeoutException after 5 seconds of network inactivity.

B113: Test for missing requests timeout
This plugin test checks for requests or httpx calls without a timeout specified.

Nearly all production code should use this parameter in nearly all requests, Failure to do so can cause your program to hang indefinitely.

trim21 added a commit to trim21/ruff that referenced this pull request Jul 6, 2024
@mkniewallner
Copy link
Contributor Author

Not sure a missing timeout is a problem since httpx uses 5s as default. The rule is about preventing the client from waiting indefinitely, which it never will in the case of httpx and a missing timeout. Setting timeout to None is still problematic though.

Indeed, really sorry for missing that obvious information, I should have better checked that.

@trim21
Copy link
Contributor

trim21 commented Jul 6, 2024

false positive fix #12213

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants