fix: address code review findings

- Fix WebSocket reconnection logic in App.svelte (was a no-op)
- Remove spoofable x-forwarded-for fallback, use cf-connecting-ip only
- Wrap Durable Object broadcast in try/catch to prevent place failures
- Cache OffscreenCanvas to avoid re-allocation every render frame
This commit is contained in:
tiennm99 committed 2026-04-16 16:27:35 +07:00
1 parent 2ccf1c9779
commit b3b4916263
5 files changed
+32 -18

No files matched your search

+5
View File
@@ -0,0 +1,5 @@
{
"enabledPlugins": {
"frontend-design@claude-plugins-official": true
}
}
+9 -6
View File
@@ -13,8 +13,8 @@
/** @type {CanvasRenderer} */ /** @type {CanvasRenderer} */
let canvasRenderer; let canvasRenderer;
// WebSocket connection for real-time updates // WebSocket connection with auto-reconnect
$effect(() => { function connectWebSocket() {
const protocol = location.protocol === 'https:' ? 'wss:' : 'ws:'; const protocol = location.protocol === 'https:' ? 'wss:' : 'ws:';
const ws = new WebSocket(`${protocol}//${location.host}/api/ws`); const ws = new WebSocket(`${protocol}//${location.host}/api/ws`);
@@ -27,11 +27,14 @@
} catch { /* ignore parse errors */ } } catch { /* ignore parse errors */ }
}; };
ws.onclose = () => { ws.onclose = () => setTimeout(connectWebSocket, 1000);
// Reconnect after 1s ws.onerror = () => ws.close();
setTimeout(() => { /* effect re-runs on reactive dep change */ }, 1000);
};
return ws;
}
$effect(() => {
const ws = connectWebSocket();
return () => ws.close(); return () => ws.close();
}); });
</script> </script>
+5 -2
View File
@@ -12,6 +12,10 @@
let lastMouse = { x: 0, y: 0 }; let lastMouse = { x: 0, y: 0 };
let loading = $state(true); let loading = $state(true);
// Cached offscreen canvas — avoids re-allocation every render
const offscreen = new OffscreenCanvas(CANVAS_WIDTH, CANVAS_HEIGHT);
const offCtx = offscreen.getContext('2d');
/** Render ImageData onto visible canvas with current zoom/pan */ /** Render ImageData onto visible canvas with current zoom/pan */
function render() { function render() {
if (!canvasEl || !imageData) return; if (!canvasEl || !imageData) return;
@@ -21,8 +25,7 @@
ctx.fillStyle = '#1a1a1a'; ctx.fillStyle = '#1a1a1a';
ctx.fillRect(0, 0, canvasEl.width, canvasEl.height); ctx.fillRect(0, 0, canvasEl.width, canvasEl.height);
const offscreen = new OffscreenCanvas(CANVAS_WIDTH, CANVAS_HEIGHT); offCtx.putImageData(imageData, 0, 0);
offscreen.getContext('2d').putImageData(imageData, 0, 0);
ctx.save(); ctx.save();
ctx.translate(pan.x, pan.y); ctx.translate(pan.x, pan.y);
+2 -3
View File
@@ -5,9 +5,8 @@
* @returns {string} user id prefixed with "anon:" * @returns {string} user id prefixed with "anon:"
*/ */
export function getUserId(request) { export function getUserId(request) {
const ip = request.headers.get('cf-connecting-ip') // CF-Connecting-IP is set by Cloudflare and cannot be spoofed
|| request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() const ip = request.headers.get('cf-connecting-ip') || '127.0.0.1';
|| '127.0.0.1';
// Simple hash for privacy // Simple hash for privacy
let hash = 0; let hash = 0;
+11 -7
View File
@@ -63,13 +63,17 @@ app.post('/api/place', async (c) => {
// Write pixels to canvas // Write pixels to canvas
await setPixels(c.env, pixels); await setPixels(c.env, pixels);
// Broadcast to all connected WebSocket clients // Broadcast to all connected WebSocket clients (non-blocking)
const roomId = c.env.CANVAS_ROOM.idFromName('main'); try {
const room = c.env.CANVAS_ROOM.get(roomId); const roomId = c.env.CANVAS_ROOM.idFromName('main');
await room.fetch(new Request('http://internal/broadcast', { const room = c.env.CANVAS_ROOM.get(roomId);
method: 'POST', await room.fetch(new Request('http://internal/broadcast', {
body: JSON.stringify(pixels), method: 'POST',
})); body: JSON.stringify(pixels),
}));
} catch (err) {
console.error('Broadcast failed:', err);
}
return c.json({ ok: true, credits: remaining }); return c.json({ ok: true, credits: remaining });
}); });