Skip to content

Commit 14f573a

Browse files
fix: address Copilot review comments
- Add URL validation to prevent "https://pyplots.ai/None" when spec is None - Add html.escape for query params to prevent XSS in og:image URLs - Add FileNotFoundError handling for missing static og-image.png - Add comprehensive tests for analytics module (detect_platform, track_og_image) - Add tests for new og:image endpoints (home.png, catalog.png) - Fix platform count: 27 platforms (not 25) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent e016527 commit 14f573a

6 files changed

Lines changed: 252 additions & 6 deletions

File tree

api/analytics.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
PLAUSIBLE_ENDPOINT = "https://plausible.io/api/event"
1919
DOMAIN = "pyplots.ai"
2020

21-
# All platforms from nginx.conf bot detection (25 total)
21+
# All platforms from nginx.conf bot detection (27 total)
2222
PLATFORM_PATTERNS = {
2323
# Social Media
2424
"twitter": "twitterbot",
@@ -118,10 +118,13 @@ def track_og_image(
118118
url = "https://pyplots.ai/"
119119
elif page == "catalog":
120120
url = "https://pyplots.ai/catalog"
121-
elif library:
121+
elif spec is not None and library:
122122
url = f"https://pyplots.ai/{spec}/{library}"
123-
else:
123+
elif spec is not None:
124124
url = f"https://pyplots.ai/{spec}"
125+
else:
126+
# Fallback: missing spec for a spec-based page
127+
url = "https://pyplots.ai/"
125128

126129
props: dict[str, str] = {"page": page, "platform": platform}
127130
if spec:

api/routers/og_images.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ def _get_static_og_image() -> bytes:
2424
global _STATIC_OG_IMAGE
2525
if _STATIC_OG_IMAGE is None:
2626
path = Path(__file__).parent.parent.parent / "app" / "public" / "og-image.png"
27-
_STATIC_OG_IMAGE = path.read_bytes()
27+
try:
28+
_STATIC_OG_IMAGE = path.read_bytes()
29+
except FileNotFoundError as exc:
30+
raise HTTPException(status_code=500, detail="Static OG image not found") from exc
2831
return _STATIC_OG_IMAGE
2932

3033

api/routers/seo.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ async def seo_home(request: Request):
9797
Passes query params (e.g., ?lib=plotly&dom=statistics) to og:image URL for tracking.
9898
"""
9999
# Pass filter params to og:image URL for tracking shared filtered URLs
100-
query_string = str(request.query_params) if request.query_params else ""
100+
# Use html.escape to prevent XSS via query params
101+
query_string = html.escape(str(request.query_params), quote=True) if request.query_params else ""
101102
image_url = f"{DEFAULT_HOME_IMAGE}?{query_string}" if query_string else DEFAULT_HOME_IMAGE
102103
page_url = f"https://pyplots.ai/?{query_string}" if query_string else "https://pyplots.ai/"
103104

docs/architecture/plausible.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ Bot requests page → nginx detects bot → SEO proxy serves HTML with og:image
152152
| `library` | Library ID | Only for detail pages |
153153
| `filter_*` | Filter value | Dynamic props for filtered URLs (e.g., `filter_lib`, `filter_dom`) |
154154

155-
### Platform Detection (25 platforms)
155+
### Platform Detection (27 platforms)
156156

157157
**Social Media**: twitter, facebook, linkedin, pinterest, reddit, tumblr, mastodon
158158

tests/unit/api/test_analytics.py

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
"""Tests for server-side Plausible analytics tracking."""
2+
3+
from unittest.mock import AsyncMock, MagicMock, patch
4+
5+
import pytest
6+
7+
from api.analytics import PLATFORM_PATTERNS, detect_platform, track_og_image
8+
9+
10+
class TestDetectPlatform:
11+
"""Tests for platform detection from User-Agent."""
12+
13+
def test_detects_twitter(self) -> None:
14+
"""Should detect Twitter bot."""
15+
assert detect_platform("Twitterbot/1.0") == "twitter"
16+
17+
def test_detects_whatsapp(self) -> None:
18+
"""Should detect WhatsApp."""
19+
assert detect_platform("WhatsApp/2.21.4.22") == "whatsapp"
20+
21+
def test_detects_facebook(self) -> None:
22+
"""Should detect Facebook."""
23+
assert detect_platform("facebookexternalhit/1.1") == "facebook"
24+
25+
def test_detects_linkedin(self) -> None:
26+
"""Should detect LinkedIn."""
27+
assert detect_platform("LinkedInBot/1.0") == "linkedin"
28+
29+
def test_detects_slack(self) -> None:
30+
"""Should detect Slack."""
31+
assert detect_platform("Slackbot-LinkExpanding 1.0") == "slack"
32+
33+
def test_detects_discord(self) -> None:
34+
"""Should detect Discord."""
35+
assert detect_platform("Mozilla/5.0 (compatible; Discordbot/2.0)") == "discord"
36+
37+
def test_detects_telegram(self) -> None:
38+
"""Should detect Telegram."""
39+
assert detect_platform("TelegramBot/1.0") == "telegram"
40+
41+
def test_detects_teams(self) -> None:
42+
"""Should detect Microsoft Teams."""
43+
assert detect_platform("Mozilla/5.0 Microsoft Teams") == "teams"
44+
45+
def test_detects_google(self) -> None:
46+
"""Should detect Googlebot."""
47+
assert detect_platform("Mozilla/5.0 (compatible; Googlebot/2.1)") == "google"
48+
49+
def test_returns_unknown_for_regular_browser(self) -> None:
50+
"""Should return unknown for regular browsers."""
51+
assert detect_platform("Mozilla/5.0 (Windows NT 10.0; Win64; x64) Chrome/91.0") == "unknown"
52+
53+
def test_returns_unknown_for_empty_string(self) -> None:
54+
"""Should return unknown for empty User-Agent."""
55+
assert detect_platform("") == "unknown"
56+
57+
def test_case_insensitive(self) -> None:
58+
"""Should match case-insensitively."""
59+
assert detect_platform("TWITTERBOT/1.0") == "twitter"
60+
assert detect_platform("twitterbot/1.0") == "twitter"
61+
62+
def test_all_platforms_have_patterns(self) -> None:
63+
"""Should have 27 platform patterns defined."""
64+
assert len(PLATFORM_PATTERNS) == 27
65+
66+
67+
class TestTrackOgImage:
68+
"""Tests for og:image tracking function."""
69+
70+
@pytest.fixture
71+
def mock_request(self) -> MagicMock:
72+
"""Create a mock FastAPI request."""
73+
request = MagicMock()
74+
request.headers = {"user-agent": "Twitterbot/1.0", "x-forwarded-for": "1.2.3.4"}
75+
request.client = MagicMock()
76+
request.client.host = "127.0.0.1"
77+
return request
78+
79+
def test_creates_async_task(self, mock_request: MagicMock) -> None:
80+
"""Should create background task without blocking."""
81+
with patch("api.analytics.asyncio.create_task") as mock_create_task:
82+
track_og_image(mock_request, page="home")
83+
mock_create_task.assert_called_once()
84+
85+
def test_home_page_url(self, mock_request: MagicMock) -> None:
86+
"""Should build correct URL for home page."""
87+
with patch("api.analytics.asyncio.create_task") as mock_create_task:
88+
track_og_image(mock_request, page="home")
89+
call_args = mock_create_task.call_args[0][0]
90+
# The coroutine should be called with home URL
91+
assert call_args is not None
92+
93+
def test_catalog_page_url(self, mock_request: MagicMock) -> None:
94+
"""Should build correct URL for catalog page."""
95+
with patch("api.analytics._send_plausible_event", new_callable=AsyncMock):
96+
with patch("api.analytics.asyncio.create_task") as mock_create_task:
97+
track_og_image(mock_request, page="catalog")
98+
mock_create_task.assert_called_once()
99+
100+
def test_spec_overview_url(self, mock_request: MagicMock) -> None:
101+
"""Should build correct URL for spec overview."""
102+
with patch("api.analytics.asyncio.create_task"):
103+
# Should not raise even with spec_overview page
104+
track_og_image(mock_request, page="spec_overview", spec="scatter-basic")
105+
106+
def test_spec_detail_url(self, mock_request: MagicMock) -> None:
107+
"""Should build correct URL for spec detail."""
108+
with patch("api.analytics.asyncio.create_task"):
109+
track_og_image(mock_request, page="spec_detail", spec="scatter-basic", library="matplotlib")
110+
111+
def test_fallback_url_when_spec_none(self, mock_request: MagicMock) -> None:
112+
"""Should fallback to home URL when spec is None for spec-based page."""
113+
with patch("api.analytics.asyncio.create_task"):
114+
# Should not raise - falls back to home URL
115+
track_og_image(mock_request, page="spec_overview", spec=None)
116+
117+
def test_includes_filter_props(self, mock_request: MagicMock) -> None:
118+
"""Should include filter parameters in props."""
119+
with patch("api.analytics.asyncio.create_task"):
120+
track_og_image(mock_request, page="home", filters={"lib": "plotly", "dom": "statistics"})
121+
122+
def test_uses_x_forwarded_for(self) -> None:
123+
"""Should use X-Forwarded-For header for client IP."""
124+
request = MagicMock()
125+
request.headers = {"user-agent": "Twitterbot/1.0", "x-forwarded-for": "5.6.7.8"}
126+
request.client = None
127+
128+
with patch("api.analytics.asyncio.create_task"):
129+
track_og_image(request, page="home")
130+
131+
def test_fallback_to_client_host(self) -> None:
132+
"""Should fallback to client.host when X-Forwarded-For not present."""
133+
request = MagicMock()
134+
request.headers = {"user-agent": "Twitterbot/1.0"}
135+
request.client = MagicMock()
136+
request.client.host = "10.0.0.1"
137+
138+
with patch("api.analytics.asyncio.create_task"):
139+
track_og_image(request, page="home")
140+
141+
def test_handles_missing_client(self) -> None:
142+
"""Should handle missing client gracefully."""
143+
request = MagicMock()
144+
request.headers = {"user-agent": "Twitterbot/1.0"}
145+
request.client = None
146+
147+
with patch("api.analytics.asyncio.create_task"):
148+
track_og_image(request, page="home")
149+
150+
151+
class TestSendPlausibleEvent:
152+
"""Tests for Plausible API call."""
153+
154+
@pytest.mark.asyncio
155+
async def test_sends_correct_payload(self) -> None:
156+
"""Should send correct payload to Plausible."""
157+
from api.analytics import _send_plausible_event
158+
159+
with patch("api.analytics.httpx.AsyncClient") as mock_client_class:
160+
mock_client = AsyncMock()
161+
mock_client_class.return_value.__aenter__.return_value = mock_client
162+
163+
await _send_plausible_event(
164+
user_agent="Twitterbot/1.0",
165+
client_ip="1.2.3.4",
166+
name="og_image_view",
167+
url="https://pyplots.ai/",
168+
props={"page": "home", "platform": "twitter"},
169+
)
170+
171+
mock_client.post.assert_called_once()
172+
call_kwargs = mock_client.post.call_args[1]
173+
assert call_kwargs["json"]["name"] == "og_image_view"
174+
assert call_kwargs["json"]["domain"] == "pyplots.ai"
175+
176+
@pytest.mark.asyncio
177+
async def test_handles_network_error(self) -> None:
178+
"""Should handle network errors gracefully."""
179+
from api.analytics import _send_plausible_event
180+
181+
with patch("api.analytics.httpx.AsyncClient") as mock_client_class:
182+
mock_client = AsyncMock()
183+
mock_client.post.side_effect = Exception("Network error")
184+
mock_client_class.return_value.__aenter__.return_value = mock_client
185+
186+
# Should not raise
187+
await _send_plausible_event(
188+
user_agent="Twitterbot/1.0",
189+
client_ip="1.2.3.4",
190+
name="og_image_view",
191+
url="https://pyplots.ai/",
192+
props={},
193+
)

tests/unit/api/test_routers.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,52 @@ def test_seo_spec_implementation_fallback_image(self, db_client, mock_spec) -> N
560560
class TestOgImagesRouter:
561561
"""Tests for OG image generation endpoints."""
562562

563+
def test_get_home_og_image(self, client: TestClient) -> None:
564+
"""Should return static og:image for home page."""
565+
with patch("api.routers.og_images.track_og_image"):
566+
with patch("api.routers.og_images._get_static_og_image", return_value=b"fake-image"):
567+
response = client.get("/og/home.png")
568+
assert response.status_code == 200
569+
assert response.headers["content-type"] == "image/png"
570+
assert "max-age=86400" in response.headers["cache-control"]
571+
572+
def test_get_home_og_image_with_filters(self, client: TestClient) -> None:
573+
"""Should pass filter params to tracking."""
574+
with patch("api.routers.og_images.track_og_image") as mock_track:
575+
with patch("api.routers.og_images._get_static_og_image", return_value=b"fake-image"):
576+
response = client.get("/og/home.png?lib=plotly&dom=statistics")
577+
assert response.status_code == 200
578+
mock_track.assert_called_once()
579+
call_kwargs = mock_track.call_args[1]
580+
assert call_kwargs["page"] == "home"
581+
assert call_kwargs["filters"] == {"lib": "plotly", "dom": "statistics"}
582+
583+
def test_get_catalog_og_image(self, client: TestClient) -> None:
584+
"""Should return static og:image for catalog page."""
585+
with patch("api.routers.og_images.track_og_image") as mock_track:
586+
with patch("api.routers.og_images._get_static_og_image", return_value=b"fake-image"):
587+
response = client.get("/og/catalog.png")
588+
assert response.status_code == 200
589+
assert response.headers["content-type"] == "image/png"
590+
mock_track.assert_called_once()
591+
call_kwargs = mock_track.call_args[1]
592+
assert call_kwargs["page"] == "catalog"
593+
594+
def test_get_static_og_image_file_not_found(self, client: TestClient) -> None:
595+
"""Should return 500 when static image file not found."""
596+
import api.routers.og_images as og_module
597+
598+
# Reset cached image
599+
og_module._STATIC_OG_IMAGE = None
600+
601+
with patch("api.routers.og_images.track_og_image"):
602+
with patch("pathlib.Path.read_bytes", side_effect=FileNotFoundError("not found")):
603+
response = client.get("/og/home.png")
604+
assert response.status_code == 500
605+
606+
# Reset for other tests
607+
og_module._STATIC_OG_IMAGE = None
608+
563609
def test_get_branded_impl_image_no_db(self, client: TestClient) -> None:
564610
"""Should return 503 when DB not available."""
565611
with patch(DB_CONFIG_PATCH, return_value=False):

0 commit comments

Comments
 (0)