Skip to content

Commit

Permalink
[flake8-bandit] Detect httpx for S113 (#12174)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
mkniewallner authored Jul 3, 2024
1 parent a184f84 commit 8210c1e
Show file tree
Hide file tree
Showing 3 changed files with 388 additions and 122 deletions.
62 changes: 55 additions & 7 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S113.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,71 @@
import httpx
import requests

# OK
requests.get('https://gmail.com', timeout=5)
requests.post('https://gmail.com', timeout=5)
requests.put('https://gmail.com', timeout=5)
requests.delete('https://gmail.com', timeout=5)
requests.patch('https://gmail.com', timeout=5)
requests.options('https://gmail.com', timeout=5)
requests.head('https://gmail.com', timeout=5)

httpx.get('https://gmail.com', timeout=5)
httpx.post('https://gmail.com', timeout=5)
httpx.put('https://gmail.com', timeout=5)
httpx.delete('https://gmail.com', timeout=5)
httpx.patch('https://gmail.com', timeout=5)
httpx.options('https://gmail.com', timeout=5)
httpx.head('https://gmail.com', timeout=5)
httpx.Client(timeout=5)
httpx.AsyncClient(timeout=5)
with httpx.Client(timeout=5) as client:
client.get('https://gmail.com')
async def foo():
async with httpx.AsyncClient(timeout=5) as client:
await client.get('https://gmail.com')

# Errors
requests.get('https://gmail.com')
requests.get('https://gmail.com', timeout=None)
requests.get('https://gmail.com', timeout=5)
requests.post('https://gmail.com')
requests.post('https://gmail.com', timeout=None)
requests.post('https://gmail.com', timeout=5)
requests.put('https://gmail.com')
requests.put('https://gmail.com', timeout=None)
requests.put('https://gmail.com', timeout=5)
requests.delete('https://gmail.com')
requests.delete('https://gmail.com', timeout=None)
requests.delete('https://gmail.com', timeout=5)
requests.patch('https://gmail.com')
requests.patch('https://gmail.com', timeout=None)
requests.patch('https://gmail.com', timeout=5)
requests.options('https://gmail.com')
requests.options('https://gmail.com', timeout=None)
requests.options('https://gmail.com', timeout=5)
requests.head('https://gmail.com')
requests.head('https://gmail.com', timeout=None)
requests.head('https://gmail.com', timeout=5)

httpx.get('https://gmail.com')
httpx.get('https://gmail.com', timeout=None)
httpx.post('https://gmail.com')
httpx.post('https://gmail.com', timeout=None)
httpx.put('https://gmail.com')
httpx.put('https://gmail.com', timeout=None)
httpx.delete('https://gmail.com')
httpx.delete('https://gmail.com', timeout=None)
httpx.patch('https://gmail.com')
httpx.patch('https://gmail.com', timeout=None)
httpx.options('https://gmail.com')
httpx.options('https://gmail.com', timeout=None)
httpx.head('https://gmail.com')
httpx.head('https://gmail.com', timeout=None)
httpx.Client()
httpx.Client(timeout=None)
httpx.AsyncClient()
httpx.AsyncClient(timeout=None)
with httpx.Client() as client:
client.get('https://gmail.com')
with httpx.Client(timeout=None) as client:
client.get('https://gmail.com')
async def bar():
async with httpx.AsyncClient() as client:
await client.get('https://gmail.com')
async def baz():
async with httpx.AsyncClient(timeout=None) as client:
await client.get('https://gmail.com')
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of the Python `requests` module that omit the `timeout`
/// parameter.
/// Checks for uses of the Python `requests` or `httpx` module that omit the
/// `timeout` parameter.
///
/// ## Why is this bad?
/// The `timeout` parameter is used to set the maximum time to wait for a
Expand All @@ -31,48 +31,50 @@ use crate::checkers::ast::Checker;
///
/// ## References
/// - [Requests documentation: Timeouts](https://requests.readthedocs.io/en/latest/user/advanced/#timeouts)
/// - [httpx documentation: Timeouts](https://www.python-httpx.org/advanced/timeouts/)
#[violation]
pub struct RequestWithoutTimeout {
implicit: bool,
module: String,
}

impl Violation for RequestWithoutTimeout {
#[derive_message_formats]
fn message(&self) -> String {
let RequestWithoutTimeout { implicit } = self;
let RequestWithoutTimeout { implicit, module } = self;
if *implicit {
format!("Probable use of requests call without timeout")
format!("Probable use of `{module}` call without timeout")
} else {
format!("Probable use of requests call with timeout set to `None`")
format!("Probable use of `{module}` call with timeout set to `None`")
}
}
}

/// S113
pub(crate) fn request_without_timeout(checker: &mut Checker, call: &ast::ExprCall) {
if checker
if let Some(module) = checker
.semantic()
.resolve_qualified_name(&call.func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
[
"requests",
"get" | "options" | "head" | "post" | "put" | "patch" | "delete" | "request"
]
)
.and_then(|qualified_name| match qualified_name.segments() {
["requests", "get" | "options" | "head" | "post" | "put" | "patch" | "delete" | "request"] => {
Some("requests")
}
["httpx", "get" | "options" | "head" | "post" | "put" | "patch" | "delete" | "request" | "stream" | "Client" | "AsyncClient"] => {
Some("httpx")
}
_ => None,
})
{
if let Some(keyword) = call.arguments.find_keyword("timeout") {
if keyword.value.is_none_literal_expr() {
checker.diagnostics.push(Diagnostic::new(
RequestWithoutTimeout { implicit: false },
RequestWithoutTimeout { implicit: false, module: module.to_string() },
keyword.range(),
));
}
} else {
checker.diagnostics.push(Diagnostic::new(
RequestWithoutTimeout { implicit: true },
RequestWithoutTimeout { implicit: true, module: module.to_string() },
call.func.range(),
));
}
Expand Down
Loading

0 comments on commit 8210c1e

Please sign in to comment.