Replace IsDown's third-party scraping with a direct reachability check

isup.me now serves a Cloudflare bot-challenge page to non-browser clients
(confirmed live), so scraping it can never work again regardless of the
earlier words[0]/words[1] fix. Checked two alternatives from munin:
downforeveryoneorjustme.com is also Cloudflare-gated, and
isitdownrightnow.com, while not gated, only exposes status via a CSS class
in a ping-history table with ambiguous boilerplate text ("is up"/"is down"
appear on every page regardless of actual status) - too fragile to scrape
reliably either.

Instead of depending on any third party's HTML, do the obvious thing: a
DNS lookup to distinguish "doesn't exist" from "unreachable", then a
direct request to the target with a timeout. Simpler, more reliable, and
checks the actual target instead of trusting a third party's cache.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

diff --git a/src/distbot/plugins/url.py b/src/distbot/plugins/url.py
index 430f7d069255b99689db3ac09ff2a822f480f7bf..b151af242449519ede6e72ab67a41fe8b5c3e0d5 100644 (file)
--- a/src/distbot/plugins/url.py
+++ b/src/distbot/plugins/url.py
@@ -4,6 +4,7 @@ Plugins for user specific functions
 """
 import logging
 import re
+import socket
 from urllib.parse import urlparse
 
 import requests
@@ -62,14 +63,18 @@ class IsDown(Worker):
         url = words[1]
         if 'http' not in url:
             url = 'http://{}'.format(url)
-        response = requests.get('http://www.isup.me/{}'.format(urlparse(url).hostname)).text
-        if "looks down" in response:
-            return Action(msg='{}: {} looks down'.format(sender, url))
-        elif "is up" in response:
-            return Action(msg='{}: {} looks up'.format(sender, url))
-        elif "site on the interwho" in response:
+
+        try:
+            socket.gethostbyname(urlparse(url).hostname)
+        except socket.gaierror:
             return Action(msg='{}: {} does not exist, you\'re trying to fool me?'.format(sender, url))
 
+        try:
+            requests.get(url, timeout=8)
+            return Action(msg='{}: {} looks up'.format(sender, url))
+        except requests.exceptions.RequestException:
+            return Action(msg='{}: {} looks down'.format(sender, url))
+
 
 class URLResolver(Worker):
     binding_keys = Worker.CATCH_ALL

diff --git a/tests/muc_smoke_check.py b/tests/muc_smoke_check.py
index 2dfce7d463ed9b9b489b8c326284df5349493a72..aba0e1238028357a29832ccd972d632e594075a8 100644 (file)
--- a/tests/muc_smoke_check.py
+++ b/tests/muc_smoke_check.py
@@ -142,9 +142,10 @@ def default_checks(nick: str, bot_nick: str) -> List[Check]:
               re.compile(r"^cake for \S+: "), command="cake please", timeout=20.0),
         Check("mymemory translation (single word sidesteps a words[2:]-as-list bug)",
               re.compile(r"^translation: "), command="translate en|de hello", timeout=20.0),
-        Check("isdown check (regression coverage for a fixed off-by-one: parse_body used to read "
-              "words[0], the command word itself, instead of words[1], the actual target)",
-              re.compile(rf"^{nick_re}: "), command="isdown debian.org", timeout=20.0),
+        Check("isdown check (does a direct reachability check now - the original words[0]/words[1] "
+              "off-by-one is fixed, and isup.me stopped being scrapable once it went behind "
+              "a Cloudflare bot-challenge page)",
+              re.compile(rf"^{nick_re}: "), command="isdown debian.org", timeout=15.0),
         Check("youtube oEmbed title (stable, long-lived video id also used in this repo's own unit tests)",
               ANY, raw_message="https://www.youtube.com/watch?v=H27VcmHVRaw", timeout=20.0),
         Check("URLResolver <title> scrape (stable, non-blacklisted URL)",

diff --git a/tests/test_unit/test_isdown.py b/tests/test_unit/test_isdown.py
index 28471692f3b445fd104d9264448473aacbefe23b..50277d7425503e2673bc4fb50c916d3cb11651c3 100644 (file)
--- a/tests/test_unit/test_isdown.py
+++ b/tests/test_unit/test_isdown.py
@@ -1,7 +1,9 @@
 # -*- coding: utf-8 -*-
+import socket
 from unittest.mock import Mock, patch
 
 import pytest
+import requests
 
 from distbot.bot.worker import Worker
 from distbot.plugins.url import IsDown
@@ -19,26 +21,47 @@ def test_isdown_checks_the_argument_not_the_command_word(deadworker):
     plugin = IsDown("_")
     msg = {"body": "isdown debian.org", "from": "user@test.com/res"}
 
-    with patch("distbot.plugins.url.requests.get") as mock_get:
-        mock_get.return_value = Mock(text="that site on the interwho looks down")
+    with patch("distbot.plugins.url.socket.gethostbyname") as mock_dns, \
+            patch("distbot.plugins.url.requests.get") as mock_get:
         plugin.parse_body(msg)
 
+    assert mock_dns.call_args.args[0] == "debian.org"
     requested_url = mock_get.call_args.args[0]
     assert "debian.org" in requested_url
     assert "isdown" not in requested_url
 
 
-@pytest.mark.parametrize("response_text,expected_fragment", [
-    ("that site on the interwho looks down", "looks down"),
-    ("that site on the interwho is up", "looks up"),
-    ("this looks like a site on the interwho that doesn't exist", "does not exist"),
-])
-def test_isdown_response_mapping(deadworker, response_text, expected_fragment):
+def test_isdown_reachable(deadworker):
+    # direct check now: a resolvable host that answers is "up", regardless
+    # of a third party's (possibly Cloudflare-gated) opinion on the matter.
     plugin = IsDown("_")
     msg = {"body": "isdown debian.org", "from": "user@test.com/res"}
 
-    with patch("distbot.plugins.url.requests.get") as mock_get:
-        mock_get.return_value = Mock(text=response_text)
+    with patch("distbot.plugins.url.socket.gethostbyname"), \
+            patch("distbot.plugins.url.requests.get") as mock_get:
+        mock_get.return_value = Mock()
         action = plugin.parse_body(msg)
 
-    assert expected_fragment in action.msg
+    assert "looks up" in action.msg
+
+
+def test_isdown_unreachable(deadworker):
+    # resolvable host, but the request itself fails (refused/timeout/etc).
+    plugin = IsDown("_")
+    msg = {"body": "isdown debian.org", "from": "user@test.com/res"}
+
+    with patch("distbot.plugins.url.socket.gethostbyname"), \
+            patch("distbot.plugins.url.requests.get", side_effect=requests.exceptions.ConnectionError()):
+        action = plugin.parse_body(msg)
+
+    assert "looks down" in action.msg
+
+
+def test_isdown_nonexistent_domain(deadworker):
+    plugin = IsDown("_")
+    msg = {"body": "isdown thisdoesnotexist12345.invalid", "from": "user@test.com/res"}
+
+    with patch("distbot.plugins.url.socket.gethostbyname", side_effect=socket.gaierror()):
+        action = plugin.parse_body(msg)
+
+    assert "does not exist" in action.msg