From 08c66eee371e109452747c7e32b6da9923308d8c Mon Sep 17 00:00:00 2001 From: laky55555 Date: Sun, 23 Oct 2022 00:01:23 +0100 Subject: [PATCH] Cover ttl=0 (expire always) _DNSCacheTable case (#7014) Fix bug when using ttl_dns_cache=0 When using `ttl_dns_cache=0`, hostname is added to cache and on expire check there is a KeyError as it is never added into _timestamp dictionary. Added a short description to docs ClientSession advising to use `use_dns_cache` instead of `ttl_dns_cache=0` for always expiring cache. Alternative to this change is to enforce user to use positive none 0 float or None for `ttl_dns_cache` during init time and in case of using 0 implicitly change `use_dns_cache` to False or raise exception. No. - [X] I think the code is well written - [X] Unit tests for the changes exist - [X] Documentation reflects the changes - [X] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [X] Add a new news fragment into the `CHANGES` folder * name it `.` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.feature`: Signifying a new feature. * `.bugfix`: Signifying a bug fix. * `.doc`: Signifying a documentation improvement. * `.removal`: Signifying a deprecation or removal of public API. * `.misc`: A ticket has been closed, but it is not of interest to users. * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files." Co-authored-by: Sam Bull (cherry picked from commit 12f56d7892151154343c6048526d007bbe3a4291) --- CHANGES/7014.bugfix | 1 + CONTRIBUTORS.txt | 2 ++ aiohttp/connector.py | 4 ++-- tests/test_connector.py | 23 ++++++++++++++++++++--- 4 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 CHANGES/7014.bugfix diff --git a/CHANGES/7014.bugfix b/CHANGES/7014.bugfix new file mode 100644 index 00000000000..34a4668288f --- /dev/null +++ b/CHANGES/7014.bugfix @@ -0,0 +1 @@ +Fixed bug when using ``TCPConnector`` with ``ttl_dns_cache=0``. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index c1651d50299..71910b8bae5 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -147,6 +147,8 @@ Illia Volochii Ilya Chichak Ilya Gruzinov Ingmar Steen +Ivan Lakovic +Ivan Larin Jacob Champion Jaesung Lee Jake Davis diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 81b71313931..5c55669ec39 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -709,13 +709,13 @@ def __contains__(self, host: object) -> bool: def add(self, key: Tuple[str, int], addrs: List[Dict[str, Any]]) -> None: self._addrs_rr[key] = (cycle(addrs), len(addrs)) - if self._ttl: + if self._ttl is not None: self._timestamps[key] = monotonic() def remove(self, key: Tuple[str, int]) -> None: self._addrs_rr.pop(key, None) - if self._ttl: + if self._ttl is not None: self._timestamps.pop(key, None) def clear(self) -> None: diff --git a/tests/test_connector.py b/tests/test_connector.py index 0b992df98cb..7d8dc7e31d0 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -2165,10 +2165,27 @@ def test_not_expired_ttl(self) -> None: dns_cache_table.add("localhost", ["127.0.0.1"]) assert not dns_cache_table.expired("localhost") - async def test_expired_ttl(self, loop) -> None: - dns_cache_table = _DNSCacheTable(ttl=0.01) + def test_expired_ttl(self, monkeypatch: pytest.MonkeyPatch) -> None: + dns_cache_table = _DNSCacheTable(ttl=1) + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1) dns_cache_table.add("localhost", ["127.0.0.1"]) - await asyncio.sleep(0.02) + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 2) + assert not dns_cache_table.expired("localhost") + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 3) + assert dns_cache_table.expired("localhost") + + def test_never_expire(self, monkeypatch: pytest.MonkeyPatch) -> None: + dns_cache_table = _DNSCacheTable(ttl=None) + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1) + dns_cache_table.add("localhost", ["127.0.0.1"]) + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 10000000) + assert not dns_cache_table.expired("localhost") + + def test_always_expire(self, monkeypatch: pytest.MonkeyPatch) -> None: + dns_cache_table = _DNSCacheTable(ttl=0) + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1) + dns_cache_table.add("localhost", ["127.0.0.1"]) + monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1.00001) assert dns_cache_table.expired("localhost") def test_next_addrs(self, dns_cache_table) -> None: