Skip to content
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

[ADD] missing-timeout: Used when a method calls an external request #6780

Merged
merged 1 commit into from
Jun 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ contributors:
* Add consider-merging-isinstance, superfluous-else-return
* Fix consider-using-ternary for 'True and True and True or True' case
* Add bad-docstring-quotes and docstring-first-line-empty
* Add missing-timeout
moylop260 marked this conversation as resolved.
Show resolved Hide resolved
- Ville Skyttä <ville.skytta@iki.fi>
- Pierre-Yves David <pierre-yves.david@logilab.fr>
- David Shea <dshea@redhat.com>: invalid sequence and slice index
Expand Down
3 changes: 3 additions & 0 deletions doc/data/messages/m/missing-timeout/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import requests

requests.post("http://localhost") # [missing-timeout]
10 changes: 10 additions & 0 deletions doc/data/messages/m/missing-timeout/details.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
You can add new methods that should have a defined ```timeout`` argument as qualified names
in the ``timeout-methods`` option, for example:

* ``requests.api.get``
* ``requests.api.head``
* ``requests.api.options``
* ``requests.api.patch``
* ``requests.api.post``
* ``requests.api.put``
* ``requests.api.request``
3 changes: 3 additions & 0 deletions doc/data/messages/m/missing-timeout/good.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import requests

requests.post("http://localhost", timeout=10)
3 changes: 3 additions & 0 deletions doc/whatsnew/2/2.15/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ Summary -- Release highlights
New checkers
============

Added new checker ``missing-timeout`` to warn of default timeout values that could cause
a program to be hanging indefinitely.


Removed checkers
================
Expand Down
86 changes: 86 additions & 0 deletions pylint/checkers/method_args.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt

"""Variables checkers for Python code."""

from __future__ import annotations

from typing import TYPE_CHECKING

from astroid import arguments, nodes

from pylint.checkers import BaseChecker, utils
from pylint.interfaces import INFERENCE

if TYPE_CHECKING:
from pylint.lint import PyLinter


class MethodArgsChecker(BaseChecker):
moylop260 marked this conversation as resolved.
Show resolved Hide resolved
"""BaseChecker for method_args.

Checks for
* missing-timeout
"""

name = "method_args"
msgs = {
"W3101": (
"Missing timeout argument for method '%s' can cause your program to hang indefinitely",
"missing-timeout",
"Used when a method needs a 'timeout' parameter in order to avoid waiting "
"for a long time. If no timeout is specified explicitly the default value "
"is used. For example for 'requests' the program will never time out "
"(i.e. hang indefinitely).",
),
}
options = (
(
"timeout-methods",
{
"default": (
"requests.api.delete",
"requests.api.get",
"requests.api.head",
"requests.api.options",
"requests.api.patch",
"requests.api.post",
"requests.api.put",
"requests.api.request",
),
"type": "csv",
"metavar": "<comma separated list>",
"help": "List of qualified names (i.e., library.method) which require a timeout parameter "
"e.g. 'requests.api.get,requests.api.post'",
},
),
)

@utils.only_required_for_messages("missing-timeout")
def visit_call(self, node: nodes.Call) -> None:
"""Check if the call needs a timeout parameter based on package.func_name
configured in config.timeout_methods.

Package uses inferred node in order to know the package imported.
"""
inferred = utils.safe_infer(node.func)
call_site = arguments.CallSite.from_call(node)
if (
inferred
and not call_site.has_invalid_keywords()
and inferred.qname() in self.config.timeout_methods
):
keyword_arguments = [keyword.arg for keyword in node.keywords]
keyword_arguments.extend(call_site.keyword_arguments)
if "timeout" not in keyword_arguments:
self.add_message(
"missing-timeout",
node=node,
args=(node.func.as_string(),),
confidence=INFERENCE,
)


def register(linter: PyLinter) -> None:
linter.register_checker(MethodArgsChecker(linter))
1 change: 1 addition & 0 deletions requirements_test_min.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ typing-extensions~=4.2
pytest~=7.1
pytest-benchmark~=3.4
pytest-timeout~=2.1
requests
moylop260 marked this conversation as resolved.
Show resolved Hide resolved
85 changes: 85 additions & 0 deletions tests/functional/m/missing/missing_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
"""Tests for missing-timeout."""

# pylint: disable=consider-using-with,import-error,no-member,no-name-in-module,reimported

import requests
from requests import (
delete,
delete as delete_r,
get,
get as get_r,
head,
head as head_r,
options,
options as options_r,
patch,
patch as patch_r,
post,
post as post_r,
put,
put as put_r,
request,
request as request_r,
)

# requests without timeout
requests.delete("http://localhost") # [missing-timeout]
requests.get("http://localhost") # [missing-timeout]
requests.head("http://localhost") # [missing-timeout]
requests.options("http://localhost") # [missing-timeout]
requests.patch("http://localhost") # [missing-timeout]
requests.post("http://localhost") # [missing-timeout]
requests.put("http://localhost") # [missing-timeout]
requests.request("call", "http://localhost") # [missing-timeout]

delete_r("http://localhost") # [missing-timeout]
get_r("http://localhost") # [missing-timeout]
head_r("http://localhost") # [missing-timeout]
options_r("http://localhost") # [missing-timeout]
patch_r("http://localhost") # [missing-timeout]
post_r("http://localhost") # [missing-timeout]
put_r("http://localhost") # [missing-timeout]
request_r("call", "http://localhost") # [missing-timeout]

delete("http://localhost") # [missing-timeout]
get("http://localhost") # [missing-timeout]
head("http://localhost") # [missing-timeout]
options("http://localhost") # [missing-timeout]
patch("http://localhost") # [missing-timeout]
post("http://localhost") # [missing-timeout]
put("http://localhost") # [missing-timeout]
request("call", "http://localhost") # [missing-timeout]

kwargs_wo_timeout = {}
post("http://localhost", **kwargs_wo_timeout) # [missing-timeout]

# requests valid cases
requests.delete("http://localhost", timeout=10)
requests.get("http://localhost", timeout=10)
requests.head("http://localhost", timeout=10)
requests.options("http://localhost", timeout=10)
requests.patch("http://localhost", timeout=10)
requests.post("http://localhost", timeout=10)
requests.put("http://localhost", timeout=10)
requests.request("call", "http://localhost", timeout=10)

delete_r("http://localhost", timeout=10)
get_r("http://localhost", timeout=10)
head_r("http://localhost", timeout=10)
options_r("http://localhost", timeout=10)
patch_r("http://localhost", timeout=10)
post_r("http://localhost", timeout=10)
put_r("http://localhost", timeout=10)
request_r("call", "http://localhost", timeout=10)

delete("http://localhost", timeout=10)
moylop260 marked this conversation as resolved.
Show resolved Hide resolved
get("http://localhost", timeout=10)
head("http://localhost", timeout=10)
options("http://localhost", timeout=10)
patch("http://localhost", timeout=10)
post("http://localhost", timeout=10)
put("http://localhost", timeout=10)
request("call", "http://localhost", timeout=10)

kwargs_timeout = {'timeout': 10}
post("http://localhost", **kwargs_timeout)
25 changes: 25 additions & 0 deletions tests/functional/m/missing/missing_timeout.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
missing-timeout:26:0:26:35::Missing timeout argument for method 'requests.delete' can cause your program to hang indefinitely:INFERENCE
missing-timeout:27:0:27:32::Missing timeout argument for method 'requests.get' can cause your program to hang indefinitely:INFERENCE
missing-timeout:28:0:28:33::Missing timeout argument for method 'requests.head' can cause your program to hang indefinitely:INFERENCE
missing-timeout:29:0:29:36::Missing timeout argument for method 'requests.options' can cause your program to hang indefinitely:INFERENCE
missing-timeout:30:0:30:34::Missing timeout argument for method 'requests.patch' can cause your program to hang indefinitely:INFERENCE
missing-timeout:31:0:31:33::Missing timeout argument for method 'requests.post' can cause your program to hang indefinitely:INFERENCE
missing-timeout:32:0:32:32::Missing timeout argument for method 'requests.put' can cause your program to hang indefinitely:INFERENCE
missing-timeout:33:0:33:44::Missing timeout argument for method 'requests.request' can cause your program to hang indefinitely:INFERENCE
missing-timeout:35:0:35:28::Missing timeout argument for method 'delete_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:36:0:36:25::Missing timeout argument for method 'get_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:37:0:37:26::Missing timeout argument for method 'head_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:38:0:38:29::Missing timeout argument for method 'options_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:39:0:39:27::Missing timeout argument for method 'patch_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:40:0:40:26::Missing timeout argument for method 'post_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:41:0:41:25::Missing timeout argument for method 'put_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:42:0:42:37::Missing timeout argument for method 'request_r' can cause your program to hang indefinitely:INFERENCE
missing-timeout:44:0:44:26::Missing timeout argument for method 'delete' can cause your program to hang indefinitely:INFERENCE
missing-timeout:45:0:45:23::Missing timeout argument for method 'get' can cause your program to hang indefinitely:INFERENCE
missing-timeout:46:0:46:24::Missing timeout argument for method 'head' can cause your program to hang indefinitely:INFERENCE
missing-timeout:47:0:47:27::Missing timeout argument for method 'options' can cause your program to hang indefinitely:INFERENCE
missing-timeout:48:0:48:25::Missing timeout argument for method 'patch' can cause your program to hang indefinitely:INFERENCE
missing-timeout:49:0:49:24::Missing timeout argument for method 'post' can cause your program to hang indefinitely:INFERENCE
missing-timeout:50:0:50:23::Missing timeout argument for method 'put' can cause your program to hang indefinitely:INFERENCE
missing-timeout:51:0:51:35::Missing timeout argument for method 'request' can cause your program to hang indefinitely:INFERENCE
missing-timeout:54:0:54:45::Missing timeout argument for method 'post' can cause your program to hang indefinitely:INFERENCE