From e95d0614600b4c71325c81b2a621184a55a1e4c9 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 22:04:10 +0100 Subject: [PATCH] Fix domain substring matching and NoneType crash in submit dialog (#2367) * Initial plan * Fix domain matching and NoneType error in submit.py - Use regex with domain boundary matching instead of substring matching to prevent x.com from matching 500px.com, mix.com, etc. - Handle None old_site gracefully when user enters a site name not in the matched list, fixing AttributeError crash. - Add tests for both fixes. Co-authored-by: soxoj <31013580+soxoj@users.noreply.github.com> Agent-Logs-Url: https://github.com/soxoj/maigret/sessions/7eabc755-47fd-4b80-a38c-9d6c056c2ce9 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: soxoj <31013580+soxoj@users.noreply.github.com> --- maigret/submit.py | 18 +++++++--- tests/test_submit.py | 85 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/maigret/submit.py b/maigret/submit.py index fae40dd..4603aef 100644 --- a/maigret/submit.py +++ b/maigret/submit.py @@ -409,8 +409,13 @@ class Submitter: self.logger.info('Domain is %s', domain_raw) # check for existence + domain_re = re.compile( + r'://(www\.)?' + re.escape(domain_raw) + r'(/|$)' + ) matched_sites = list( - filter(lambda x: domain_raw in x.url_main + x.url, self.db.sites) + filter( + lambda x: domain_re.search(x.url_main + x.url), self.db.sites + ) ) if matched_sites: @@ -448,9 +453,14 @@ class Submitter: old_site = next( (site for site in matched_sites if site.name == site_name), None ) - print( - f'{Fore.GREEN}[+] We will update site "{old_site.name}" in case of success.{Style.RESET_ALL}' - ) + if old_site is None: + print( + f'{Fore.RED}[!] Site "{site_name}" not found in the matched list. Proceeding without updating an existing site.{Style.RESET_ALL}' + ) + else: + print( + f'{Fore.GREEN}[+] We will update site "{old_site.name}" in case of success.{Style.RESET_ALL}' + ) # Check if the site check is ordinary or not if old_site and (old_site.url_probe or old_site.activation): diff --git a/tests/test_submit.py b/tests/test_submit.py index 7a25437..d957d82 100644 --- a/tests/test_submit.py +++ b/tests/test_submit.py @@ -1,8 +1,10 @@ +import re + import pytest from unittest.mock import MagicMock, patch from maigret.submit import Submitter from aiohttp import ClientSession -from maigret.sites import MaigretDatabase +from maigret.sites import MaigretDatabase, MaigretSite import logging @@ -275,3 +277,84 @@ async def test_dialog_adds_site_negative(settings): await submitter.close() assert result is False + + +def test_domain_matching_exact(): + """Test that domain matching uses proper boundary checks, not substring matching. + + x.com should NOT match sites like 500px.com, mix.com, etc. + """ + domain_raw = "x.com" + domain_re = re.compile( + r'://(www\.)?' + re.escape(domain_raw) + r'(/|$)' + ) + + # These should NOT match x.com + non_matching = [ + MaigretSite("500px", {"url": "https://500px.com/p/{username}", "urlMain": "https://500px.com/"}), + MaigretSite("Mix", {"url": "https://mix.com/{username}", "urlMain": "https://mix.com"}), + MaigretSite("Screwfix", {"url": "{urlMain}{urlSubpath}/members/?username={username}", "urlMain": "https://community.screwfix.com"}), + MaigretSite("Wix", {"url": "https://{username}.wix.com", "urlMain": "https://wix.com/"}), + MaigretSite("1x", {"url": "https://1x.com/{username}", "urlMain": "https://1x.com"}), + MaigretSite("Roblox", {"url": "https://www.roblox.com/user.aspx?username={username}", "urlMain": "https://www.roblox.com/"}), + ] + + for site in non_matching: + assert not domain_re.search(site.url_main + site.url), \ + f"x.com should NOT match site {site.name} ({site.url_main})" + + +def test_domain_matching_positive(): + """Test that domain matching correctly matches the exact domain.""" + domain_raw = "x.com" + domain_re = re.compile( + r'://(www\.)?' + re.escape(domain_raw) + r'(/|$)' + ) + + # These SHOULD match x.com + matching = [ + MaigretSite("X", {"url": "https://x.com/{username}", "urlMain": "https://x.com"}), + MaigretSite("X-www", {"url": "https://www.x.com/{username}", "urlMain": "https://www.x.com"}), + ] + + for site in matching: + assert domain_re.search(site.url_main + site.url), \ + f"x.com SHOULD match site {site.name} ({site.url_main})" + + +def test_dialog_nonexistent_site_name_no_crash(): + """Test that entering a site name not in the matched list doesn't crash. + + This tests the fix for: AttributeError: 'NoneType' object has no attribute 'name' + The old_site should be None when user enters a name not in matched_sites, + and the code should handle it gracefully. + """ + # Simulate the logic that was crashing + matched_sites = [ + MaigretSite("ValidActive", {"url": "https://example.com/{username}", "urlMain": "https://example.com"}), + MaigretSite("InvalidActive", {"url": "https://example.com/alt/{username}", "urlMain": "https://example.com"}), + ] + site_name = "NonExistentSite" + + old_site = next( + (site for site in matched_sites if site.name == site_name), None + ) + + # This is what the old code did - it would crash here + assert old_site is None + + # The fix: check before accessing .name + if old_site is None: + result = "not found" + else: + result = old_site.name + + assert result == "not found" + + # And when site_name IS in matched_sites, it should work + site_name = "ValidActive" + old_site = next( + (site for site in matched_sites if site.name == site_name), None + ) + assert old_site is not None + assert old_site.name == "ValidActive"