From 682d5e0a1a8e432fc689cdcd2fde4c755378c1ce Mon Sep 17 00:00:00 2001 From: kosako Date: Sat, 11 Jul 2026 18:14:32 +0900 Subject: [PATCH] =?UTF-8?q?server/shared:=20safeFilePart=20=E3=81=A8=20sta?= =?UTF-8?q?tus=20=E5=AE=9A=E6=95=B0=E3=82=92=E5=8D=98=E4=B8=80=E3=82=BD?= =?UTF-8?q?=E3=83=BC=E3=82=B9=E5=8C=96=E3=81=97=20build.js=20=E3=81=AB=20e?= =?UTF-8?q?xport=20=E3=82=AC=E3=83=BC=E3=83=89=E3=82=92=E8=B6=B3=E3=81=99?= =?UTF-8?q?=20(#109=20R-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 挙動を変えないリファクタリング(Issue #109 スライス R-1)。 - safeFilePart(receive.js と widget index.js にバイト同一で重複)を shared/format.js へ移設し、server 側を shared 参照に切替(widget 側の 切替は R-3 で実施)。shared-format.test.js にテスト追加 - status 定数を単一ソース化: store.js の VALID_STATUSES を FEEDBACK_STATUSES に改名して export し、receive.js のローカル定義を 削除して store から require。正規化の丸め挙動(不正値→"new")は不変 - scripts/build.js に複数宣言子 export(export const A = 1, B = 2;)の 検出 fail を追加(静かな export 漏れを「明示 fail」方針に揃える。 現ソース該当 0 件のため dist のロジックは不変) - dist を再生成(shared/format への safeFilePart 追加分のみ) 実装: Codex(Claude 作成のブリーフに基づく)。検証: npm run check 全 113 テスト pass。 Co-Authored-By: Codex --- dist/patchloop-widget.js | 9 ++++++++- scripts/build.js | 30 +++++++++++++++++++++++++++++- server/receive.js | 12 ++---------- server/store.js | 7 ++++--- shared/format.js | 7 +++++++ test/shared-format.test.js | 10 ++++++++++ 6 files changed, 60 insertions(+), 15 deletions(-) diff --git a/dist/patchloop-widget.js b/dist/patchloop-widget.js index 92b5a35..2d95586 100644 --- a/dist/patchloop-widget.js +++ b/dist/patchloop-widget.js @@ -370,6 +370,13 @@ const __pl_shared_format = (() => { // link hardening (safeLinkUrl / mdLinkUrl) and the screenshot status texts // stay in their respective owners because their semantics differ per side. +function safeFilePart(value) { + return String(value || "feedback") + .replace(/[^a-zA-Z0-9_-]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 80) || "feedback"; +} + function truncateText(value, max) { const text = String(value ?? ""); return text.length > max ? `${text.slice(0, max)}…` : text; @@ -421,7 +428,7 @@ function formatTarget(target) { return `${target.kind || "point"} at ${present(target.clientX)},${present(target.clientY)}`; } -return { truncateText, present, escapeHtml, escapeXml, slackEscape, formatSlackCode, formatSlackLink, formatViewport, formatTarget }; +return { safeFilePart, truncateText, present, escapeHtml, escapeXml, slackEscape, formatSlackCode, formatSlackLink, formatViewport, formatTarget }; })(); const { pointFromClient, rectFromPoints, rectContainsArea, pointFromStoredTarget, rectFromStoredArea, round, numberOrNull } = __pl_widget_src_geometry; const { pointAnchorOffsets, areaAnchorOffsets, roundedAnchor, geometryFromAnchor, viewportDiffersFromCreation } = __pl_widget_src_anchoring; diff --git a/scripts/build.js b/scripts/build.js index d058dde..ef6aec1 100644 --- a/scripts/build.js +++ b/scripts/build.js @@ -42,6 +42,30 @@ function parseBindingList(raw, context) { }); } +function hasTopLevelComma(line) { + const depths = { "(": 0, "[": 0, "{": 0 }; + const closing = { ")": "(", "]": "[", "}": "{" }; + let quote = null; + let escaped = false; + + for (const char of line) { + if (quote) { + if (escaped) escaped = false; + else if (char === "\\") escaped = true; + else if (char === quote) quote = null; + continue; + } + if (char === '"' || char === "'" || char === "`") { + quote = char; + continue; + } + if (char in depths) depths[char] += 1; + else if (char in closing) depths[closing[char]] -= 1; + else if (char === "," && Object.values(depths).every((depth) => depth === 0)) return true; + } + return false; +} + // Parses one module into { imports, exports, bodyLines }. Import statements // are removed from the body; `export` keywords are stripped in place. function parseModule(filePath) { @@ -75,7 +99,11 @@ function parseModule(filePath) { }); return; } - const declMatch = EXPORT_DECL_RE.exec(line) || EXPORT_VAR_RE.exec(line); + const varMatch = EXPORT_VAR_RE.exec(line); + if (varMatch && hasTopLevelComma(line)) { + fail(`${where}: multiple declarators in an export declaration are not supported`); + } + const declMatch = EXPORT_DECL_RE.exec(line) || varMatch; if (!declMatch) { fail(`${where}: only named export declarations and export lists are supported`); } diff --git a/server/receive.js b/server/receive.js index 002cf43..abdb04a 100644 --- a/server/receive.js +++ b/server/receive.js @@ -6,8 +6,8 @@ const fs = require("fs"); const path = require("path"); const crypto = require("crypto"); -const { truncateText, present, escapeHtml, slackEscape, formatSlackCode, formatSlackLink, formatViewport, formatTarget } = require("../shared/format.js"); -const { createStore } = require("./store.js"); +const { safeFilePart, truncateText, present, escapeHtml, slackEscape, formatSlackCode, formatSlackLink, formatViewport, formatTarget } = require("../shared/format.js"); +const { createStore, FEEDBACK_STATUSES } = require("./store.js"); const CONFIG_PATH = process.env.PATCHLOOP_RECEIVER_CONFIG || path.join(__dirname, "receiver.config.json"); const config = loadConfig(CONFIG_PATH); @@ -95,7 +95,6 @@ const IMPORT_BUNDLE_KIND = "patchloop-feedback-bundle"; // Both are accepted so files exported before the batch-download switch still // import. const SUPPORTED_IMPORT_BUNDLE_VERSIONS = new Set([1, 2]); -const FEEDBACK_STATUSES = ["new", "accepted", "fixed", "ignored"]; // Default applied to payloads received before the widget sent schemaVersion, // so every stored item carries a version going forward. const DEFAULT_SCHEMA_VERSION = 1; @@ -1496,13 +1495,6 @@ function contentTypeForPath(filePath) { return "application/octet-stream"; } -function safeFilePart(value) { - return String(value || "feedback") - .replace(/[^a-zA-Z0-9_-]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 80) || "feedback"; -} - function httpError(message, statusCode) { const error = new Error(message); error.statusCode = statusCode; diff --git a/server/store.js b/server/store.js index c3febf3..baf1fad 100644 --- a/server/store.js +++ b/server/store.js @@ -15,6 +15,7 @@ // delete(id) -> item|null returns the removed item (for screenshot cleanup) // count() -> number // close() +// Exported FEEDBACK_STATUSES is the shared status allowlist used by the receiver. // // A new backend (e.g. createMysqlStore) just needs to implement this shape and // be wired into createStore() below; the receiver code stays unchanged. @@ -38,10 +39,10 @@ function isUniqueViolation(error) { return /UNIQUE constraint failed/i.test(error && error.message); } -const VALID_STATUSES = ["new", "accepted", "fixed", "ignored"]; +const FEEDBACK_STATUSES = ["new", "accepted", "fixed", "ignored"]; function normalizeStatus(value) { - return VALID_STATUSES.includes(value) ? value : "new"; + return FEEDBACK_STATUSES.includes(value) ? value : "new"; } // Columns extracted from each feedback object for indexed filtering. The full @@ -218,4 +219,4 @@ function createStore(config = {}) { throw new Error(`Unknown store backend: ${backend}`); } -module.exports = { createStore }; +module.exports = { createStore, FEEDBACK_STATUSES }; diff --git a/shared/format.js b/shared/format.js index 45235dc..b5afce0 100644 --- a/shared/format.js +++ b/shared/format.js @@ -4,6 +4,13 @@ // link hardening (safeLinkUrl / mdLinkUrl) and the screenshot status texts // stay in their respective owners because their semantics differ per side. +export function safeFilePart(value) { + return String(value || "feedback") + .replace(/[^a-zA-Z0-9_-]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 80) || "feedback"; +} + export function truncateText(value, max) { const text = String(value ?? ""); return text.length > max ? `${text.slice(0, max)}…` : text; diff --git a/test/shared-format.test.js b/test/shared-format.test.js index 6dd1909..835b183 100644 --- a/test/shared-format.test.js +++ b/test/shared-format.test.js @@ -4,6 +4,7 @@ const assert = require("node:assert/strict"); const test = require("node:test"); const { + safeFilePart, truncateText, present, escapeHtml, @@ -15,6 +16,15 @@ const { formatTarget } = require("../shared/format.js"); +test("safeFilePart creates a bounded filesystem-safe component", () => { + assert.equal(safeFilePart("feedback_123"), "feedback_123"); + assert.equal(safeFilePart(" --hello, world!!-- "), "hello-world"); + assert.equal(safeFilePart(""), "feedback"); + assert.equal(safeFilePart(null), "feedback"); + assert.equal(safeFilePart(false), "feedback"); + assert.equal(safeFilePart("x".repeat(81)), "x".repeat(80)); +}); + test("truncateText coerces and appends an ellipsis past the limit", () => { assert.equal(truncateText("hello", 10), "hello"); assert.equal(truncateText("hello world", 5), "hello…");