Skip to content

Commit

Permalink
[flake8-bandit] Avoid S310 violations for HTTP-safe f-strings (#1…
Browse files Browse the repository at this point in the history
…2305)

this resolves #12245
  • Loading branch information
zzztimbo authored Jul 13, 2024
1 parent 6584886 commit 1a3ee45
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 99 deletions.
16 changes: 14 additions & 2 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,37 @@
import urllib.request

urllib.request.urlopen(url='http://www.google.com')
urllib.request.urlopen(url=f'http://www.google.com')
urllib.request.urlopen(url='http://www.google.com', **kwargs)
urllib.request.urlopen(url=f'http://www.google.com', **kwargs)
urllib.request.urlopen('http://www.google.com')
urllib.request.urlopen(f'http://www.google.com')
urllib.request.urlopen('file:///foo/bar/baz')
urllib.request.urlopen(url)

urllib.request.Request(url='http://www.google.com', **kwargs)
urllib.request.Request(url='http://www.google.com')
urllib.request.Request(url=f'http://www.google.com')
urllib.request.Request(url='http://www.google.com', **kwargs)
urllib.request.Request(url=f'http://www.google.com', **kwargs)
urllib.request.Request('http://www.google.com')
urllib.request.Request(f'http://www.google.com')
urllib.request.Request('file:///foo/bar/baz')
urllib.request.Request(url)

urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs)
urllib.request.URLopener().open(fullurl='http://www.google.com')
urllib.request.URLopener().open(fullurl=f'http://www.google.com')
urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs)
urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs)
urllib.request.URLopener().open('http://www.google.com')
urllib.request.URLopener().open(f'http://www.google.com')
urllib.request.URLopener().open('file:///foo/bar/baz')
urllib.request.URLopener().open(url)

urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'))
urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com'))
urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs)
urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com'), **kwargs)
urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
urllib.request.urlopen(urllib.request.Request(f'http://www.google.com'))
urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
urllib.request.urlopen(urllib.request.Request(url))
Original file line number Diff line number Diff line change
Expand Up @@ -850,16 +850,28 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
// MarkSafe
["django", "utils", "safestring" | "html", "mark_safe"] => Some(SuspiciousMarkSafeUsage.into()),
// URLOpen (`Request`)
["urllib", "request","Request"] |
["urllib", "request", "Request"] |
["six", "moves", "urllib", "request","Request"] => {
// If the `url` argument is a string literal, allow `http` and `https` schemes.
// If the `url` argument is a string literal or an f string, allow `http` and `https` schemes.
if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
if let Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) = &call.arguments.find_argument("url", 0) {
match call.arguments.find_argument("url", 0) {
// If the `url` argument is a string literal, allow `http` and `https` schemes.
Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => {
let url = value.to_str().trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}

},
// If the `url` argument is an f-string literal, allow `http` and `https` schemes.
Some(Expr::FString(ast::ExprFString { value, .. })) => {
if let Some(ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. })) = value.elements().next() {
let url = value.trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}
}
},
_ => {}
}
}
Some(SuspiciousURLOpenUsage.into())
Expand All @@ -868,27 +880,52 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
["urllib", "request", "urlopen" | "urlretrieve" ] |
["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" ] => {
if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
if let Some(arg) = &call.arguments.find_argument("url", 0) {
match call.arguments.find_argument("url", 0) {
// If the `url` argument is a string literal, allow `http` and `https` schemes.
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = arg {
Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => {
let url = value.to_str().trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}
}
},

// If the `url` argument is an f-string literal, allow `http` and `https` schemes.
Some(Expr::FString(ast::ExprFString { value, .. })) => {
if let Some(ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. })) = value.elements().next() {
let url = value.trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}
}
},

// If the `url` argument is a `urllib.request.Request` object, allow `http` and `https` schemes.
if let Expr::Call(ExprCall { func, arguments, .. }) = arg {
Some(Expr::Call(ExprCall { func, arguments, .. })) => {
if checker.semantic().resolve_qualified_name(func.as_ref()).is_some_and(|name| name.segments() == ["urllib", "request", "Request"]) {
if let Some( Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) = arguments.find_argument("url", 0) {
match arguments.find_argument("url", 0) {
// If the `url` argument is a string literal, allow `http` and `https` schemes.
Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => {
let url = value.to_str().trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}

},

// If the `url` argument is an f-string literal, allow `http` and `https` schemes.
Some(Expr::FString(ast::ExprFString { value, .. })) => {
if let Some(ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. })) = value.elements().next() {
let url = value.trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}
}
},
_ => {}
}
}
}
},

_ => {}
}
}
Some(SuspiciousURLOpenUsage.into())
Expand Down
Loading

0 comments on commit 1a3ee45

Please sign in to comment.