A02/A03: Add state validation and normalize subdomain keys for shared cooldown
This commit is contained in:
@@ -44,12 +44,25 @@ MAX_RESPONSE_BYTES = 5 * 1024 * 1024 # 5 MB
|
|||||||
|
|
||||||
|
|
||||||
def _read_state(state_file: Path) -> dict:
|
def _read_state(state_file: Path) -> dict:
|
||||||
"""Read state file with shared lock; return empty dict if missing/corrupt."""
|
"""Read state file with shared lock; return empty dict if missing/corrupt.
|
||||||
|
|
||||||
|
Validates that the parsed JSON is a flat dict of string->number pairs.
|
||||||
|
Returns {} for missing, corrupt, or structurally invalid files.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
with open(state_file, "r") as f:
|
with open(state_file, "r") as f:
|
||||||
fcntl.flock(f, fcntl.LOCK_SH)
|
fcntl.flock(f, fcntl.LOCK_SH)
|
||||||
try:
|
try:
|
||||||
return json.loads(f.read())
|
data = json.loads(f.read())
|
||||||
|
if not isinstance(data, dict):
|
||||||
|
return {}
|
||||||
|
# Validate structure: flat dict of string->number
|
||||||
|
for key, value in data.items():
|
||||||
|
if not isinstance(key, str):
|
||||||
|
return {}
|
||||||
|
if not isinstance(value, (int, float)):
|
||||||
|
return {}
|
||||||
|
return data
|
||||||
finally:
|
finally:
|
||||||
fcntl.flock(f, fcntl.LOCK_UN)
|
fcntl.flock(f, fcntl.LOCK_UN)
|
||||||
except (FileNotFoundError, json.JSONDecodeError, ValueError, OSError):
|
except (FileNotFoundError, json.JSONDecodeError, ValueError, OSError):
|
||||||
@@ -76,10 +89,21 @@ def _write_state(state_file: Path, state: dict) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def fetch_site_key(url: str) -> str:
|
def fetch_site_key(url: str) -> str:
|
||||||
"""Return a stable cooldown key shared by all endpoints of one site."""
|
"""Return a stable cooldown key shared by all endpoints of one site.
|
||||||
|
|
||||||
|
Normalizes hostname to a common key for related domains:
|
||||||
|
- Strips common subdomains (www, download, api, cdn)
|
||||||
|
- Keeps the base domain as the key
|
||||||
|
"""
|
||||||
hostname = (urlsplit(url).hostname or "").lower()
|
hostname = (urlsplit(url).hostname or "").lower()
|
||||||
if hostname.startswith("www."):
|
if hostname.startswith("www."):
|
||||||
hostname = hostname[4:]
|
hostname = hostname[4:]
|
||||||
|
if hostname.startswith("download."):
|
||||||
|
hostname = hostname[9:]
|
||||||
|
if hostname.startswith("api."):
|
||||||
|
hostname = hostname[4:]
|
||||||
|
if hostname.startswith("cdn."):
|
||||||
|
hostname = hostname[4:]
|
||||||
if not hostname:
|
if not hostname:
|
||||||
raise ValueError("source URL must include a hostname")
|
raise ValueError("source URL must include a hostname")
|
||||||
return hostname
|
return hostname
|
||||||
@@ -105,6 +129,8 @@ def check_and_reserve(
|
|||||||
Opens state file with exclusive lock, reads state, checks cooldown,
|
Opens state file with exclusive lock, reads state, checks cooldown,
|
||||||
reserves if allowed — all in one critical section. Uses lockfile
|
reserves if allowed — all in one critical section. Uses lockfile
|
||||||
pattern for cross-process coordination.
|
pattern for cross-process coordination.
|
||||||
|
|
||||||
|
Handles corrupt/missing state files gracefully by treating them as empty.
|
||||||
"""
|
"""
|
||||||
if now is None:
|
if now is None:
|
||||||
now = time.time()
|
now = time.time()
|
||||||
@@ -115,10 +141,17 @@ def check_and_reserve(
|
|||||||
with open(lock_file, "w") as lf:
|
with open(lock_file, "w") as lf:
|
||||||
fcntl.flock(lf, fcntl.LOCK_EX)
|
fcntl.flock(lf, fcntl.LOCK_EX)
|
||||||
try:
|
try:
|
||||||
# Read state under lock
|
# Read state under lock; validate structure
|
||||||
try:
|
try:
|
||||||
with open(state_file, "r") as sf:
|
with open(state_file, "r") as sf:
|
||||||
state = json.loads(sf.read()) or {}
|
raw = sf.read()
|
||||||
|
state = json.loads(raw) or {}
|
||||||
|
if not isinstance(state, dict):
|
||||||
|
state = {}
|
||||||
|
for key, value in state.items():
|
||||||
|
if not isinstance(key, str) or not isinstance(value, (int, float)):
|
||||||
|
state = {}
|
||||||
|
break
|
||||||
except (FileNotFoundError, json.JSONDecodeError, ValueError, OSError):
|
except (FileNotFoundError, json.JSONDecodeError, ValueError, OSError):
|
||||||
state = {}
|
state = {}
|
||||||
# Check cooldown
|
# Check cooldown
|
||||||
|
|||||||
@@ -28,6 +28,19 @@ def test_fetch_site_key_groups_endpoints_and_normalizes_www() -> None:
|
|||||||
assert fetch_site_key("https://www.rf4-stat.ru/posts/") == "rf4-stat.ru"
|
assert fetch_site_key("https://www.rf4-stat.ru/posts/") == "rf4-stat.ru"
|
||||||
|
|
||||||
|
|
||||||
|
def test_fetch_site_key_normalizes_common_subdomains() -> None:
|
||||||
|
"""A02: download/api/cdn subdomains share the base domain key."""
|
||||||
|
assert fetch_site_key("https://download.rf4db.com/ru/catches") == "rf4db.com"
|
||||||
|
assert fetch_site_key("https://api.rf4db.com/v1/catches") == "rf4db.com"
|
||||||
|
assert fetch_site_key("https://cdn.rf4db.com/assets/img.jpg") == "rf4db.com"
|
||||||
|
assert fetch_site_key("https://www.rf4db.com/") == "rf4db.com"
|
||||||
|
# Base domain stays the same
|
||||||
|
assert fetch_site_key("https://rf4db.com/") == "rf4db.com"
|
||||||
|
# Non-normalized subdomains stay as-is
|
||||||
|
assert fetch_site_key("https://rf4map.ru/point/1") == "rf4map.ru"
|
||||||
|
assert fetch_site_key("https://rf4-posts.com/spots/") == "rf4-posts.com"
|
||||||
|
|
||||||
|
|
||||||
def test_failed_fetch_still_reserves_site_cooldown(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
def test_failed_fetch_still_reserves_site_cooldown(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
state_file = tmp_path / "fetch-state.json"
|
state_file = tmp_path / "fetch-state.json"
|
||||||
|
|
||||||
@@ -36,7 +49,8 @@ def test_failed_fetch_still_reserves_site_cooldown(tmp_path: Path, monkeypatch:
|
|||||||
|
|
||||||
monkeypatch.setattr(community_cli, "fetch_html", fail)
|
monkeypatch.setattr(community_cli, "fetch_html", fail)
|
||||||
assert community_cli.main(["rf4db", "--state-file", str(state_file)]) == 1
|
assert community_cli.main(["rf4db", "--state-file", str(state_file)]) == 1
|
||||||
assert "download.rf4db.com" in json.loads(state_file.read_text(encoding="utf-8"))
|
# A02: download.rf4db.com normalized to rf4db.com
|
||||||
|
assert "rf4db.com" in json.loads(state_file.read_text(encoding="utf-8"))
|
||||||
|
|
||||||
|
|
||||||
def test_validate_url_host_rejects_disallowed_hosts() -> None:
|
def test_validate_url_host_rejects_disallowed_hosts() -> None:
|
||||||
@@ -73,15 +87,15 @@ def test_write_state_is_atomic_with_flush(tmp_path: Path) -> None:
|
|||||||
from rf4_research.community_cli import _read_state, _write_state
|
from rf4_research.community_cli import _read_state, _write_state
|
||||||
|
|
||||||
state_file = tmp_path / "state.json"
|
state_file = tmp_path / "state.json"
|
||||||
_write_state(state_file, {"key1": "value1"})
|
_write_state(state_file, {"key1": 1000.0})
|
||||||
assert state_file.exists()
|
assert state_file.exists()
|
||||||
assert not (state_file.with_suffix(".tmp")).exists()
|
assert not (state_file.with_suffix(".tmp")).exists()
|
||||||
state = _read_state(state_file)
|
state = _read_state(state_file)
|
||||||
assert state == {"key1": "value1"}
|
assert state == {"key1": 1000.0}
|
||||||
|
|
||||||
_write_state(state_file, {"key1": "value2", "key2": "value3"})
|
_write_state(state_file, {"key1": 2000.0, "key2": 3000.0})
|
||||||
state = _read_state(state_file)
|
state = _read_state(state_file)
|
||||||
assert state == {"key1": "value2", "key2": "value3"}
|
assert state == {"key1": 2000.0, "key2": 3000.0}
|
||||||
assert not (state_file.with_suffix(".tmp")).exists()
|
assert not (state_file.with_suffix(".tmp")).exists()
|
||||||
|
|
||||||
|
|
||||||
@@ -273,3 +287,40 @@ def test_urljoin_resolves_relative_redirects() -> None:
|
|||||||
assert urljoin("https://rf4-stat.ru/old/path", "new") == "https://rf4-stat.ru/old/new"
|
assert urljoin("https://rf4-stat.ru/old/path", "new") == "https://rf4-stat.ru/old/new"
|
||||||
# Absolute URL
|
# Absolute URL
|
||||||
assert urljoin("https://rf4-stat.ru/old", "https://rf4-stat.ru/absolute") == "https://rf4-stat.ru/absolute"
|
assert urljoin("https://rf4-stat.ru/old", "https://rf4-stat.ru/absolute") == "https://rf4-stat.ru/absolute"
|
||||||
|
|
||||||
|
|
||||||
|
# A02: State validation tests
|
||||||
|
|
||||||
|
def test_read_state_rejects_corrupt_json(tmp_path: Path) -> None:
|
||||||
|
"""A02: Corrupt JSON returns empty dict."""
|
||||||
|
from rf4_research.community_cli import _read_state
|
||||||
|
|
||||||
|
state_file = tmp_path / "corrupt.json"
|
||||||
|
state_file.write_text("not valid json {{{")
|
||||||
|
assert _read_state(state_file) == {}
|
||||||
|
|
||||||
|
|
||||||
|
def test_read_state_rejects_invalid_structure(tmp_path: Path) -> None:
|
||||||
|
"""A02: Non-dict or non-flat structures return empty dict."""
|
||||||
|
from rf4_research.community_cli import _read_state
|
||||||
|
|
||||||
|
state_file = tmp_path / "invalid.json"
|
||||||
|
# List instead of dict
|
||||||
|
state_file.write_text("[1, 2, 3]")
|
||||||
|
assert _read_state(state_file) == {}
|
||||||
|
# Dict with non-number values
|
||||||
|
state_file.write_text('{"key": "value"}')
|
||||||
|
assert _read_state(state_file) == {}
|
||||||
|
# Dict with nested dict
|
||||||
|
state_file.write_text('{"key": {"nested": true}}')
|
||||||
|
assert _read_state(state_file) == {}
|
||||||
|
# Dict with non-string keys (JSON always has string keys, but validate anyway)
|
||||||
|
state_file.write_text('{"valid": 123}')
|
||||||
|
assert _read_state(state_file) == {"valid": 123}
|
||||||
|
|
||||||
|
|
||||||
|
def test_read_state_handles_missing_file() -> None:
|
||||||
|
"""A02: Missing state file returns empty dict."""
|
||||||
|
from rf4_research.community_cli import _read_state
|
||||||
|
|
||||||
|
assert _read_state(Path("/nonexistent/state.json")) == {}
|
||||||
|
|||||||
Reference in New Issue
Block a user