From 5a66c2df516ac02d08ffc1f11863eb7adaf26466 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Fri, 31 Jul 2026 17:02:14 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/avito):=20=D1=8D=D0=BA=D1=80=D0=B0?= =?UTF-8?q?=D0=BD=D0=B8=D1=80=D0=BE=D0=B2=D0=B0=D1=82=D1=8C=20=5F=20=D0=B2?= =?UTF-8?q?=20LIKE-=D0=BF=D0=B0=D1=82=D1=82=D0=B5=D1=80=D0=BD=D0=B0=D1=85?= =?UTF-8?q?=20+=20=D1=87=D0=B5=D1=81=D1=82=D0=BD=D0=B0=D1=8F=20=D1=8D?= =?UTF-8?q?=D0=BC=D1=83=D0=BB=D1=8F=D1=86=D0=B8=D1=8F=20LIKE=20=D0=B2=20?= =?UTF-8?q?=D1=82=D0=B5=D1=81=D1=82=D0=B5=20(#2576)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../app/tasks/avito_detail_backfill.py | 17 ++- .../tests/tasks/test_avito_detail_backfill.py | 110 ++++++++++++++---- 2 files changed, 105 insertions(+), 22 deletions(-) diff --git a/tradein-mvp/backend/app/tasks/avito_detail_backfill.py b/tradein-mvp/backend/app/tasks/avito_detail_backfill.py index 92b5b4a8..55336a29 100644 --- a/tradein-mvp/backend/app/tasks/avito_detail_backfill.py +++ b/tradein-mvp/backend/app/tasks/avito_detail_backfill.py @@ -79,7 +79,22 @@ __all__ = [ # города (может отличаться от нашего city_slug: kamensk-uralskiy через дефис, # verhnyaya_pyshma без "kh") -- дублировать список тут вместо импорта было бы # risk дрейфа при добавлении новых oblast-городов. -_OBLAST_AVITO_URL_PATTERNS = tuple(f"%/{loc.avito_slug}/%" for loc in CITY_LOCATIONS.values()) +# +# #2578 review: Postgres LIKE трактует '_' как wildcard "один любой символ" (не +# литерал) и '%' как wildcard "любая последовательность" -- два слага из пяти +# (nizhniy_tagil, verhnyaya_pyshma) содержат '_', без экранирования это латентная +# дыра: город с похожим слагом (напр. nizhniyXtagil) молча совпал бы. Сегодня +# коллизий нет (проверено на проде: raw vs escaped паттерны дают одинаковые 776 +# совпадений), но экранируем сейчас, а не когда появится реальная коллизия. +# LIKE по умолчанию использует '\' как escape-символ (без явного ESCAPE) — +# подтверждено на живом Postgres 16.4 (см. коммит #2578-fixup): 'nizhniyXtagil' +# матчит неэкранированный '%/nizhniy_tagil/%' (LIKE default '_'=wildcard) и НЕ +# матчит экранированный '%/nizhniy\_tagil/%' (LIKE '\_' = литерал '_'); точный +# слаг 'nizhniy_tagil' матчит оба варианта -- позитивный кейс не сломан. +_OBLAST_AVITO_URL_PATTERNS = tuple( + "%/" + loc.avito_slug.replace("\\", "\\\\").replace("_", "\\_").replace("%", "\\%") + "/%" + for loc in CITY_LOCATIONS.values() +) @dataclass diff --git a/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py b/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py index 599e3308..84e58798 100644 --- a/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py +++ b/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import fnmatch import os import sys from unittest.mock import AsyncMock, MagicMock, patch @@ -398,42 +399,109 @@ def test_oblast_avito_url_patterns_cover_region66_cities() -> None: """#2576: _OBLAST_AVITO_URL_PATTERNS строится из CITY_LOCATIONS.avito_slug — список должен покрывать реальные Avito-слаги oblast-городов (в т.ч. те, что ОТЛИЧАЮТСЯ от нашего city_slug: kamensk-uralskiy через дефис, а не - kamensk_uralskiy).""" + kamensk_uralskiy). + + #2578 review: '_' в слаге -- LIKE wildcard, экранируем при построении паттерна + ('_' -> '\\_') -- nizhniy_tagil/verhnyaya_pyshma здесь ожидаются С обратным + слэшем перед '_', НЕ голым подчёркиванием.""" assert "%/kamensk-uralskiy/%" in _OBLAST_AVITO_URL_PATTERNS - assert "%/nizhniy_tagil/%" in _OBLAST_AVITO_URL_PATTERNS + assert "%/nizhniy\\_tagil/%" in _OBLAST_AVITO_URL_PATTERNS assert "%/pervouralsk/%" in _OBLAST_AVITO_URL_PATTERNS - assert "%/verhnyaya_pyshma/%" in _OBLAST_AVITO_URL_PATTERNS + assert "%/verhnyaya\\_pyshma/%" in _OBLAST_AVITO_URL_PATTERNS assert "%/serov/%" in _OBLAST_AVITO_URL_PATTERNS # ЕКБ обрабатывается отдельным жёстко закодированным паттерном (ekb CTE), # НЕ через этот oblast-список — не должен в него затесаться. assert not any("ekaterinburg" in p for p in _OBLAST_AVITO_URL_PATTERNS) +def _like_pattern_to_fnmatch(pattern: str) -> str: + """Точный перевод семантики Postgres `LIKE` (default `ESCAPE '\\'`) в fnmatch- + паттерн -- посимвольно, а НЕ наивным `.replace()`. + + LIKE: `%` = любая последовательность символов, `_` = РОВНО один любой символ, + `\\%`/`\\_`/`\\\\` = литералы (экранирование). fnmatch: `*` = любая + последовательность, `?` = один любой символ; голые `_`/`%` в fnmatch не + специальны (можно вставлять как литерал без экранирования). + + #2578 review: наивный `pat.replace("%", "*")` (как было раньше) НЕ отражал бы + семантику `_` вообще -- fnmatch трактует `_` как литерал, LIKE -- как wildcard. + Из-за этого расхождения прежний тест не поймал бы латентный баг (нет + экранирования `_` в продовых паттернах). Посимвольный разбор здесь корректно + различает голый `_` (-> `?` wildcard) и экранированный `\\_` (-> литерал `_`). + """ + out: list[str] = [] + i = 0 + n = len(pattern) + while i < n: + ch = pattern[i] + if ch == "\\" and i + 1 < n and pattern[i + 1] in ("%", "_", "\\"): + out.append(pattern[i + 1]) # экранированный символ -> литерал as-is + i += 2 + continue + if ch == "%": + out.append("*") + elif ch == "_": + out.append("?") + else: + out.append(ch) + i += 1 + return "".join(out) + + +def _in_oblast_or_ekb_scope(source_url: str) -> bool: + """Локальная реплика WHERE-условия snapshot-запроса (ekb CTE OR oblast CTE) + через корректную LIKE-эмуляцию -- без поднятия БД.""" + if fnmatch.fnmatchcase(source_url, _like_pattern_to_fnmatch("%/ekaterinburg/%")): + return True + return any( + fnmatch.fnmatchcase(source_url, _like_pattern_to_fnmatch(pat)) + for pat in _OBLAST_AVITO_URL_PATTERNS + ) + + def test_oblast_avito_url_patterns_include_oblast_and_ekb_exclude_foreign_region() -> None: """#2576 DoD: листинг города области и екатеринбургский листинг проходят scope-фильтр; листинг чужого региона (Москва/СПб) — нет. - Постгресовый `LIKE '%pat%'` эквивалентен fnmatch с `%` -> `*` (сам паттерн - без иных SQL-метасимволов) — реплицируем ту же семантику локально, чтобы - проверить реальные продовые паттерны (_OBLAST_AVITO_URL_PATTERNS) без - поднятия БД (юнит-тесты этого файла её не используют).""" - import fnmatch - - def _in_scope(source_url: str) -> bool: - if fnmatch.fnmatchcase(source_url, "*/ekaterinburg/*"): - return True - return any( - fnmatch.fnmatchcase(source_url, pat.replace("%", "*")) - for pat in _OBLAST_AVITO_URL_PATTERNS - ) - + Использует корректную LIKE-эмуляцию (_like_pattern_to_fnmatch), а не наивный + `%` -> `*` replace (#2578 review — тот не различал бы `_`-семантику).""" # Область (Каменск-Уральский, #2576 — реальный кейс из тикета) -- проходит. - assert _in_scope("https://www.avito.ru/kamensk-uralskiy/kvartiry/prodam_123") + assert _in_oblast_or_ekb_scope("https://www.avito.ru/kamensk-uralskiy/kvartiry/prodam_123") # ЕКБ — по-прежнему проходит (не деградировал). - assert _in_scope("https://www.avito.ru/ekaterinburg/kvartiry/prodam_456") + assert _in_oblast_or_ekb_scope("https://www.avito.ru/ekaterinburg/kvartiry/prodam_456") # Чужой регион — НЕ проходит (иначе поехали бы Москва/СПб/Тюмень legacy-строки). - assert not _in_scope("https://www.avito.ru/moskva/kvartiry/prodam_789") - assert not _in_scope("https://www.avito.ru/sankt-peterburg/kvartiry/prodam_000") + assert not _in_oblast_or_ekb_scope("https://www.avito.ru/moskva/kvartiry/prodam_789") + assert not _in_oblast_or_ekb_scope("https://www.avito.ru/sankt-peterburg/kvartiry/prodam_000") + + +def test_like_underscore_wildcard_regression_caught_by_escaped_patterns() -> None: + """#2578 deep-review latent bug: Postgres `LIKE` трактует `_` как wildcard + "ровно один любой символ", а НЕ литерал. Два слага из пяти (nizhniy_tagil, + verhnyaya_pyshma) содержат `_` -- БЕЗ экранирования 'nizhniy_tagil' молча + совпал бы с 'nizhniyXtagil' (X = любой символ), т.е. коллизия слагов при + появлении похожего города. Сегодня коллизий нет (проверено на проде: raw vs + escaped паттерны дают одинаковые 776 совпадений), но дыра латентная. + + Этот тест ДОЛЖЕН падать на RAW (неэкранированном) варианте паттерна -- именно + так выглядели продовые паттерны ДО фикса #2578 (`%/nizhniy_tagil/%`, без + `\\`). Экранированный прод-паттерн (_OBLAST_AVITO_URL_PATTERNS, ПОСЛЕ фикса) + коллизию отклоняет, точный слаг по-прежнему матчит (позитивный кейс жив). + """ + raw_pattern = "%/nizhniy_tagil/%" # как было бы БЕЗ фикса #2578 (голый '_') + escaped_pattern = next(p for p in _OBLAST_AVITO_URL_PATTERNS if "nizhniy" in p) + # Сам факт экранирования: прод-паттерн ДОЛЖЕН отличаться от raw ('_' -> '\_'). + assert escaped_pattern != raw_pattern, "фикс #2578 должен экранировать '_' в avito_slug" + + collision_url = "https://www.avito.ru/nizhniyXtagil/kvartiry/prodam_1" + exact_url = "https://www.avito.ru/nizhniy_tagil/kvartiry/prodam_1" + + # RAW: '_' -- wildcard -> ложное совпадение с ЛЮБЫМ символом на его месте. + assert fnmatch.fnmatchcase(collision_url, _like_pattern_to_fnmatch(raw_pattern)) + # Экранированный прод-паттерн (после фикса): '_' -- литерал -> коллизия отклонена. + assert not fnmatch.fnmatchcase(collision_url, _like_pattern_to_fnmatch(escaped_pattern)) + # Позитивный кейс не сломан: точный слаг матчит ОБА варианта паттерна. + assert fnmatch.fnmatchcase(exact_url, _like_pattern_to_fnmatch(raw_pattern)) + assert fnmatch.fnmatchcase(exact_url, _like_pattern_to_fnmatch(escaped_pattern)) @pytest.mark.asyncio