From 91a5ff4b571c9a791e5a5323405a495188144362 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 1 May 2026 19:18:53 +0000 Subject: [PATCH 01/17] fix(api): wire QR size param, normalize URLs in updateLink, use enable() in enableLink, deduplicate VALID_RANGES - qr.ts: size query param was parsed from the route schema but never read from the request or forwarded to renderQrSvg; now extracted and passed - link-management: updateLink called normalizeUrl on create but not on update, so trailing slashes and bare question marks survived round-trips - link-management: enableLink used LinkRepository.update({expires_at:null}) instead of the purpose-built LinkRepository.enable(), adding a redundant DB read and bypassing enable-specific logic - analytics.ts, bundles.ts: VALID_RANGES was a hand-written literal that could diverge from TIMELINE_RANGES in schemas.ts; now derived from it Regression tests added for QR size forwarding and URL normalization in updateLink (trailing slash, bare question mark, null label clear). https://claude.ai/code/session_019UcHThUMEX13UdmykzAeMS --- src/__tests__/service/link-service.test.ts | 38 ++++++++++++++++++++++ src/__tests__/unit/qr.test.ts | 24 +++++++++++++- src/api/analytics.ts | 3 +- src/api/bundles.ts | 3 +- src/api/qr.ts | 4 ++- src/services/link-management.ts | 3 +- 6 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/__tests__/service/link-service.test.ts b/src/__tests__/service/link-service.test.ts index bda1d9d..0ba7362 100644 --- a/src/__tests__/service/link-service.test.ts +++ b/src/__tests__/service/link-service.test.ts @@ -300,3 +300,41 @@ describe("URL normalization in createLink", () => { expect(result.data.url).toBe("https://example.com/path"); }); }); + +describe("URL normalization in updateLink", () => { + it("strips trailing slash from updated URL", async () => { + const created = await createLink(env as any, { url: "https://example.com" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + + const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/new-path/" }); + expect(updated.ok).toBe(true); + if (updated.ok) { + expect(updated.data.url).toBe("https://example.com/new-path"); + } + }); + + it("strips trailing question mark from updated URL", async () => { + const created = await createLink(env as any, { url: "https://example.com" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + + const updated = await updateLink(env as any, created.data.id, { url: "https://example.com/page?" }); + expect(updated.ok).toBe(true); + if (updated.ok) { + expect(updated.data.url).toBe("https://example.com/page"); + } + }); + + it("can clear the label by passing null", async () => { + const created = await createLink(env as any, { url: "https://example.com", label: "Old Label" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + + const updated = await updateLink(env as any, created.data.id, { label: null }); + expect(updated.ok).toBe(true); + if (updated.ok) { + expect(updated.data.label).toBeNull(); + } + }); +}); diff --git a/src/__tests__/unit/qr.test.ts b/src/__tests__/unit/qr.test.ts index 790c33d..3e00c64 100644 --- a/src/__tests__/unit/qr.test.ts +++ b/src/__tests__/unit/qr.test.ts @@ -2,7 +2,29 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, it, expect } from "vitest"; -import { makeQR } from "../../qr"; +import { makeQR, renderQrSvg } from "../../qr"; + +function extractSvgWidth(svg: string): number { + const m = svg.match(/width="([^"]+)"/); + return m ? parseFloat(m[1]) : NaN; +} + +describe("renderQrSvg", () => { + it("respects the size option: larger size produces larger SVG", () => { + const svg200 = renderQrSvg("https://example.com", { size: 200 }); + const svg400 = renderQrSvg("https://example.com", { size: 400 }); + expect(svg200).not.toBeNull(); + expect(svg400).not.toBeNull(); + expect(extractSvgWidth(svg200!)).toBeCloseTo(200, 0); + expect(extractSvgWidth(svg400!)).toBeCloseTo(400, 0); + }); + + it("uses 220 as default size when no size provided", () => { + const svg = renderQrSvg("https://example.com"); + expect(svg).not.toBeNull(); + expect(extractSvgWidth(svg!)).toBeCloseTo(220, 0); + }); +}); describe("makeQR", () => { it("returns a non-empty square boolean matrix for short input", () => { diff --git a/src/api/analytics.ts b/src/api/analytics.ts index b9355e7..4af4a35 100644 --- a/src/api/analytics.ts +++ b/src/api/analytics.ts @@ -9,8 +9,9 @@ import { } from "../services/link-management"; import { resolveClickFilters } from "../services/admin-management"; import { fromServiceResult } from "./response"; +import { TIMELINE_RANGES } from "./schemas"; -const VALID_RANGES = new Set(["24h", "7d", "30d", "90d", "1y", "all"]); +const VALID_RANGES = new Set(TIMELINE_RANGES); function parseRange(rangeParam: string | null | undefined): TimelineRange | undefined { return VALID_RANGES.has(rangeParam as TimelineRange) ? (rangeParam as TimelineRange) : undefined; diff --git a/src/api/bundles.ts b/src/api/bundles.ts index f86526f..be5acf6 100644 --- a/src/api/bundles.ts +++ b/src/api/bundles.ts @@ -32,10 +32,11 @@ import { IdParamSchema, LinkSchema, RangeQuerySchema, + TIMELINE_RANGES, UpdateBundleBodySchema, paramHook, } from "./schemas"; -const VALID_RANGES = new Set(["24h", "7d", "30d", "90d", "1y", "all"]); +const VALID_RANGES = new Set(TIMELINE_RANGES); function parseRange(raw: string | undefined, fallback: TimelineRange = "30d"): TimelineRange { return VALID_RANGES.has(raw as TimelineRange) ? (raw as TimelineRange) : fallback; diff --git a/src/api/qr.ts b/src/api/qr.ts index 48f5934..750ed36 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -13,6 +13,8 @@ export async function handleLinkQr(request: Request, env: Env, linkId: number): const link = result.data; const url = new URL(request.url); const requestedSlug = url.searchParams.get("slug"); + const sizeParam = url.searchParams.get("size"); + const size = sizeParam ? parseInt(sizeParam, 10) : undefined; const slug = requestedSlug ? link.slugs.find((s) => s.slug === requestedSlug) @@ -22,7 +24,7 @@ export async function handleLinkQr(request: Request, env: Env, linkId: number): const origin = url.origin; const qrUrl = `${origin}/${slug.slug}?utm_medium=qr`; - const svg = renderQrSvg(qrUrl); + const svg = renderQrSvg(qrUrl, { size }); if (!svg) return json({ error: "Failed to generate QR code" }, 500); diff --git a/src/services/link-management.ts b/src/services/link-management.ts index cb25159..0315d23 100644 --- a/src/services/link-management.ts +++ b/src/services/link-management.ts @@ -129,6 +129,7 @@ export async function updateLink( } catch { return fail(400, "url must be a valid URL"); } + body.url = normalizeUrl(body.url); } const link = await LinkRepository.update(env.DB, id, body); @@ -170,7 +171,7 @@ export async function enableLink(env: Env, id: number, identity: string): Promis const link = await LinkRepository.getById(env.DB, id); if (!link) return fail(404, "Link not found"); if (link.created_by !== identity) return fail(403, "Only the link owner can enable this link"); - const enabled = await LinkRepository.update(env.DB, id, { expires_at: null }); + const enabled = await LinkRepository.enable(env.DB, id); await Promise.all( enabled!.slugs.map((s) => From d19edf2cc01f27bc8cad0626b83910a55857ea73 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 1 May 2026 19:18:59 +0000 Subject: [PATCH 02/17] fix(sdk/typescript): qr() size param type string -> number The API schema declares size as an integer. The SDK accepted string, forcing callers to pre-stringify. Changed to number; the method now converts to string internally before appending to the query string. Existing test updated; null-clearing tests added for links.update (label, expiresAt) and bundles.update (description). https://claude.ai/code/session_019UcHThUMEX13UdmykzAeMS --- sdk/typescript/src/resources/links.ts | 4 ++-- sdk/typescript/tests/client.test.ts | 31 +++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/sdk/typescript/src/resources/links.ts b/sdk/typescript/src/resources/links.ts index 7cd7158..871f781 100644 --- a/sdk/typescript/src/resources/links.ts +++ b/sdk/typescript/src/resources/links.ts @@ -70,10 +70,10 @@ export class LinksResource { } /** Get QR code SVG for a link. Returns the SVG string. */ - qr(id: number, options: { slug?: string; size?: string } = {}): Promise { + qr(id: number, options: { slug?: string; size?: number } = {}): Promise { return this.http.requestText("GET", `/_/api/links/${id}/qr`, { slug: options.slug, - size: options.size, + size: options.size !== undefined ? String(options.size) : undefined, }); } diff --git a/sdk/typescript/tests/client.test.ts b/sdk/typescript/tests/client.test.ts index d867614..43cf1e3 100644 --- a/sdk/typescript/tests/client.test.ts +++ b/sdk/typescript/tests/client.test.ts @@ -254,6 +254,24 @@ describe("links.update", () => { expect(url).toBe(`${BASE}/_/api/links/1`); expect(init.method).toBe("PUT"); }); + + it("sends label: null to clear the label", async () => { + mockFetch(200, stubLink); + await client().links.update(1, { label: null }); + const { init } = lastCall(); + const body = JSON.parse(init.body as string) as Record; + expect(body).toHaveProperty("label"); + expect(body["label"]).toBeNull(); + }); + + it("sends expires_at: null to clear expiry", async () => { + mockFetch(200, stubLink); + await client().links.update(1, { expiresAt: null }); + const { init } = lastCall(); + const body = JSON.parse(init.body as string) as Record; + expect(body).toHaveProperty("expires_at"); + expect(body["expires_at"]).toBeNull(); + }); }); describe("links.disable", () => { @@ -357,9 +375,9 @@ describe("links.qr", () => { expect(lastCall().url).toBe(`${BASE}/_/api/links/5/qr`); }); - it("appends slug and size when provided", async () => { + it("appends slug and size when provided, converting number to string", async () => { mockFetch(200, "", "image/svg+xml"); - await client().links.qr(5, { slug: "promo", size: "256" }); + await client().links.qr(5, { slug: "promo", size: 256 }); expect(lastCall().url).toBe(`${BASE}/_/api/links/5/qr?slug=promo&size=256`); }); }); @@ -515,6 +533,15 @@ describe("bundles.update", () => { expect(url).toBe(`${BASE}/_/api/bundles/42`); expect(init.method).toBe("PUT"); }); + + it("sends description: null to clear the description", async () => { + mockFetch(200, stubBundle); + await client().bundles.update(42, { description: null }); + const { init } = lastCall(); + const body = JSON.parse(init.body as string) as Record; + expect(body).toHaveProperty("description"); + expect(body["description"]).toBeNull(); + }); }); describe("bundles.delete", () => { From 7932123e086dcf501c7d1debfca88d36c0dcf685 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 1 May 2026 19:19:06 +0000 Subject: [PATCH 03/17] fix(sdk/python): sentinel pattern for nullable fields in update() links.update() and bundles.update() used None as both the default and the explicit null signal for optional nullable fields (label, expires_at, description, icon). A caller omitting label got the same wire payload as one explicitly passing label=None, so fields could not be cleared without always sending them. Introduced UNSET sentinel in _base.py (singleton). Optional nullable params default to UNSET; only keys explicitly provided by the caller are included in the request body. Passing None sends the field as JSON null, clearing the value on the server. Sync and async resources updated. Regression tests added for both clients. https://claude.ai/code/session_019UcHThUMEX13UdmykzAeMS --- sdk/python/src/shrtnr/_base.py | 17 +++++++ sdk/python/src/shrtnr/resources/bundles.py | 29 ++++++++---- sdk/python/src/shrtnr/resources/links.py | 29 ++++++++---- sdk/python/tests/test_async_client.py | 26 +++++++++++ sdk/python/tests/test_client.py | 53 ++++++++++++++++++++++ 5 files changed, 134 insertions(+), 20 deletions(-) diff --git a/sdk/python/src/shrtnr/_base.py b/sdk/python/src/shrtnr/_base.py index 2599be4..36e7c65 100644 --- a/sdk/python/src/shrtnr/_base.py +++ b/sdk/python/src/shrtnr/_base.py @@ -20,6 +20,23 @@ DEFAULT_TIMEOUT = 30.0 +class _UnsetType: + """Sentinel distinguishing 'not provided' from an explicit ``None``.""" + + _instance: _UnsetType | None = None + + def __new__(cls) -> _UnsetType: + if cls._instance is None: + cls._instance = super().__new__(cls) + return cls._instance + + def __repr__(self) -> str: + return "UNSET" + + +UNSET: Any = _UnsetType() + + def _build_auth_headers(api_key: str) -> dict[str, str]: return {"Authorization": f"Bearer {api_key}"} diff --git a/sdk/python/src/shrtnr/resources/bundles.py b/sdk/python/src/shrtnr/resources/bundles.py index b5a739f..6d121d9 100644 --- a/sdk/python/src/shrtnr/resources/bundles.py +++ b/sdk/python/src/shrtnr/resources/bundles.py @@ -15,6 +15,7 @@ import httpx from .._base import ( + UNSET, _build_auth_headers, _build_url, parse_json_response, @@ -97,17 +98,21 @@ def update( id: int, *, name: str | None = None, - description: str | None = None, - icon: str | None = None, + description: str | None = UNSET, + icon: str | None = UNSET, accent: BundleAccent | None = None, ) -> Bundle: - """Update a bundle's name, description, icon, or accent.""" + """Update a bundle's name, description, icon, or accent. + + Pass ``None`` for *description* or *icon* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if name is not None: body["name"] = name - if description is not None: + if description is not UNSET: body["description"] = description - if icon is not None: + if icon is not UNSET: body["icon"] = icon if accent is not None: body["accent"] = accent @@ -219,17 +224,21 @@ async def update( id: int, *, name: str | None = None, - description: str | None = None, - icon: str | None = None, + description: str | None = UNSET, + icon: str | None = UNSET, accent: BundleAccent | None = None, ) -> Bundle: - """Update a bundle's name, description, icon, or accent.""" + """Update a bundle's name, description, icon, or accent. + + Pass ``None`` for *description* or *icon* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if name is not None: body["name"] = name - if description is not None: + if description is not UNSET: body["description"] = description - if icon is not None: + if icon is not UNSET: body["icon"] = icon if accent is not None: body["accent"] = accent diff --git a/sdk/python/src/shrtnr/resources/links.py b/sdk/python/src/shrtnr/resources/links.py index d55767d..a50c835 100644 --- a/sdk/python/src/shrtnr/resources/links.py +++ b/sdk/python/src/shrtnr/resources/links.py @@ -15,6 +15,7 @@ import httpx from .._base import ( + UNSET, _build_auth_headers, _build_url, parse_json_response, @@ -102,16 +103,20 @@ def update( id: int, *, url: str | None = None, - label: str | None = None, - expires_at: int | None = None, + label: str | None = UNSET, + expires_at: int | None = UNSET, ) -> Link: - """Update a link's URL, label, or expiry.""" + """Update a link's URL, label, or expiry. + + Pass ``None`` for *label* or *expires_at* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if url is not None: body["url"] = url - if label is not None: + if label is not UNSET: body["label"] = label - if expires_at is not None: + if expires_at is not UNSET: body["expires_at"] = expires_at req_url = self._url(f"/_/api/links/{id}") return Link.from_dict( @@ -233,16 +238,20 @@ async def update( id: int, *, url: str | None = None, - label: str | None = None, - expires_at: int | None = None, + label: str | None = UNSET, + expires_at: int | None = UNSET, ) -> Link: - """Update a link's URL, label, or expiry.""" + """Update a link's URL, label, or expiry. + + Pass ``None`` for *label* or *expires_at* to clear the field. + Omitting a parameter leaves the field unchanged. + """ body: dict[str, Any] = {} if url is not None: body["url"] = url - if label is not None: + if label is not UNSET: body["label"] = label - if expires_at is not None: + if expires_at is not UNSET: body["expires_at"] = expires_at req_url = self._url(f"/_/api/links/{id}") return Link.from_dict( diff --git a/sdk/python/tests/test_async_client.py b/sdk/python/tests/test_async_client.py index b2ef2c4..cc64ffa 100644 --- a/sdk/python/tests/test_async_client.py +++ b/sdk/python/tests/test_async_client.py @@ -118,6 +118,19 @@ async def test_async_links_update(client: AsyncShrtnr) -> None: assert link.id == 1 +@respx.mock +async def test_async_links_update_clears_label_with_none(client: AsyncShrtnr) -> None: + import json as _json + + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict()), + ) + await client.links.update(1, label=None) + body = _json.loads(route.calls[0].request.content) + assert "label" in body + assert body["label"] is None + + @respx.mock async def test_async_links_disable(client: AsyncShrtnr) -> None: route = respx.post(f"{BASE_URL}/_/api/links/1/disable").mock( @@ -274,6 +287,19 @@ async def test_async_bundles_update(client: AsyncShrtnr) -> None: assert b.id == 42 +@respx.mock +async def test_async_bundles_update_clears_description_with_none(client: AsyncShrtnr) -> None: + import json as _json + + route = respx.put(f"{BASE_URL}/_/api/bundles/42").mock( + return_value=httpx.Response(200, json=make_bundle_dict()), + ) + await client.bundles.update(42, description=None) + body = _json.loads(route.calls[0].request.content) + assert "description" in body + assert body["description"] is None + + @respx.mock async def test_async_bundles_delete(client: AsyncShrtnr) -> None: respx.delete(f"{BASE_URL}/_/api/bundles/42").mock( diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index a90704c..0a22991 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -182,6 +182,38 @@ def test_links_update(client: Shrtnr) -> None: assert body == {"url": "https://new.com"} +@respx.mock +def test_links_update_clears_label_with_none(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict(link_id=1)), + ) + client.links.update(1, label=None) + body = json.loads(route.calls[0].request.content) + assert "label" in body + assert body["label"] is None + + +@respx.mock +def test_links_update_omits_label_when_not_provided(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict(link_id=1)), + ) + client.links.update(1, url="https://new.com") + body = json.loads(route.calls[0].request.content) + assert "label" not in body + + +@respx.mock +def test_links_update_clears_expires_at_with_none(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/links/1").mock( + return_value=httpx.Response(200, json=make_link_dict(link_id=1)), + ) + client.links.update(1, expires_at=None) + body = json.loads(route.calls[0].request.content) + assert "expires_at" in body + assert body["expires_at"] is None + + # ---- links.disable ---- @@ -455,6 +487,27 @@ def test_bundles_update(client: Shrtnr) -> None: assert body == {"description": "edited"} +@respx.mock +def test_bundles_update_clears_description_with_none(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/bundles/42").mock( + return_value=httpx.Response(200, json=make_bundle_dict()), + ) + client.bundles.update(42, description=None) + body = json.loads(route.calls[0].request.content) + assert "description" in body + assert body["description"] is None + + +@respx.mock +def test_bundles_update_omits_description_when_not_provided(client: Shrtnr) -> None: + route = respx.put(f"{BASE_URL}/_/api/bundles/42").mock( + return_value=httpx.Response(200, json=make_bundle_dict()), + ) + client.bundles.update(42, name="New Name") + body = json.loads(route.calls[0].request.content) + assert "description" not in body + + # ---- bundles.delete ---- From 63d2d6e489f4748aa268eab88e99efe2668ea3a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 1 May 2026 19:19:12 +0000 Subject: [PATCH 04/17] fix(sdk/dart): sentinel pattern for nullable fields in update() Same issue as the Python SDK: links.update() and bundles.update() could not distinguish an omitted label/expiresAt from an explicit null. Used a private const Object _unset = Object() sentinel (identical() check) so omitted params are excluded from the body and null params serialize as JSON null. Regression tests added for null-clearing (label, description) and for confirming omitted fields are absent from the request body. https://claude.ai/code/session_019UcHThUMEX13UdmykzAeMS --- sdk/dart/lib/src/resources/bundles.dart | 14 ++++++++---- sdk/dart/lib/src/resources/links.dart | 14 ++++++++---- sdk/dart/test/client_test.dart | 30 +++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/sdk/dart/lib/src/resources/bundles.dart b/sdk/dart/lib/src/resources/bundles.dart index 71144ef..1b3da43 100644 --- a/sdk/dart/lib/src/resources/bundles.dart +++ b/sdk/dart/lib/src/resources/bundles.dart @@ -4,6 +4,9 @@ import '../base_client.dart'; import '../models.dart'; +// Sentinel distinguishing "not provided" from an explicit null. +const Object _unset = Object(); + /// Methods for the `/api/bundles` and related endpoints. class BundlesResource { /// Creates a bundles resource backed by [_http]. @@ -53,17 +56,20 @@ class BundlesResource { } /// Update a bundle's name, description, icon, or accent. + /// + /// Pass `null` for [description] or [icon] to clear the field. + /// Omitting a parameter leaves the field unchanged. Future update( int id, { String? name, - String? description, - String? icon, + Object? description = _unset, + Object? icon = _unset, BundleAccent? accent, }) async { final body = {}; if (name != null) body['name'] = name; - if (description != null) body['description'] = description; - if (icon != null) body['icon'] = icon; + if (!identical(description, _unset)) body['description'] = description; + if (!identical(icon, _unset)) body['icon'] = icon; if (accent != null) body['accent'] = accent.wireValue; final json = await _http.requestJson('PUT', '/_/api/bundles/$id', body: body); diff --git a/sdk/dart/lib/src/resources/links.dart b/sdk/dart/lib/src/resources/links.dart index 3435602..3dce75b 100644 --- a/sdk/dart/lib/src/resources/links.dart +++ b/sdk/dart/lib/src/resources/links.dart @@ -4,6 +4,9 @@ import '../base_client.dart'; import '../models.dart'; +// Sentinel distinguishing "not provided" from an explicit null. +const Object _unset = Object(); + /// Methods for the `/api/links` and related endpoints. class LinksResource { /// Creates a links resource backed by [_http]. @@ -51,16 +54,19 @@ class LinksResource { } /// Update a link's URL, label, or expiry. + /// + /// Pass `null` for [label] or [expiresAt] to clear the field. + /// Omitting a parameter leaves the field unchanged. Future update( int id, { String? url, - String? label, - int? expiresAt, + Object? label = _unset, + Object? expiresAt = _unset, }) async { final body = {}; if (url != null) body['url'] = url; - if (label != null) body['label'] = label; - if (expiresAt != null) body['expires_at'] = expiresAt; + if (!identical(label, _unset)) body['label'] = label; + if (!identical(expiresAt, _unset)) body['expires_at'] = expiresAt; final json = await _http.requestJson('PUT', '/_/api/links/$id', body: body); return Link.fromJson(json! as Map); } diff --git a/sdk/dart/test/client_test.dart b/sdk/dart/test/client_test.dart index 147ebb3..1e02078 100644 --- a/sdk/dart/test/client_test.dart +++ b/sdk/dart/test/client_test.dart @@ -316,6 +316,21 @@ void main() { final body = jsonDecode(m.capture.request!.body) as Map; expect(body['url'], 'https://new.com'); }); + + test('sends label: null to clear the label', () async { + final m = _mock(status: 200, body: _linkJson()); + await m.client.links.update(1, label: null); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body.containsKey('label'), isTrue); + expect(body['label'], isNull); + }); + + test('omits label when not provided', () async { + final m = _mock(status: 200, body: _linkJson()); + await m.client.links.update(1, url: 'https://new.com'); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body.containsKey('label'), isFalse); + }); }); // ---- 7. links.disable ---- @@ -649,6 +664,21 @@ void main() { final body = jsonDecode(m.capture.request!.body) as Map; expect(body['name'], 'Updated'); }); + + test('sends description: null to clear the description', () async { + final m = _mock(status: 200, body: _bundleJson()); + await m.client.bundles.update(42, description: null); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body.containsKey('description'), isTrue); + expect(body['description'], isNull); + }); + + test('omits description when not provided', () async { + final m = _mock(status: 200, body: _bundleJson()); + await m.client.bundles.update(42, name: 'Only Name'); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body.containsKey('description'), isFalse); + }); }); // ---- 23. bundles.delete ---- From 711a998fd924c0bafeaae271fdf501966ec0d01e Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 12:25:14 +0800 Subject: [PATCH 05/17] constants: harmonize slug-length cap at 128 via MAX_SLUG_LENGTH The service-layer validateSlugLength() capped at 128 while CustomSlugStringSchema capped at 64 and slug_length capped at 16, so callers could request a custom slug that the schema accepted but the service rejected (or vice versa). One global MAX_SLUG_LENGTH constant in src/constants.ts is now the single source of truth, applied to validateSlugLength, CustomSlugStringSchema, slug_length in CreateLinkBodySchema, and the MCP create_link tool input. --- src/api/schemas.ts | 5 +++-- src/constants.ts | 1 + src/mcp/server.ts | 3 ++- src/slugs.ts | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/api/schemas.ts b/src/api/schemas.ts index 1f4e466..c3e8316 100644 --- a/src/api/schemas.ts +++ b/src/api/schemas.ts @@ -4,6 +4,7 @@ import { z } from "@hono/zod-openapi"; import type { Hook } from "@hono/zod-openapi"; import { formatZodError } from "./response"; +import { MIN_SLUG_LENGTH, MAX_SLUG_LENGTH } from "../constants"; // ---- Common ---- @@ -62,7 +63,7 @@ export const CreateLinkBodySchema = z .object({ url: z.string().url().max(2048), label: z.string().optional(), - slug_length: z.number().int().min(3).max(16).optional(), + slug_length: z.number().int().min(MIN_SLUG_LENGTH).max(MAX_SLUG_LENGTH).optional(), expires_at: z.number().int().nonnegative().optional(), allow_duplicate: z.boolean().optional(), }) @@ -86,7 +87,7 @@ export const UpdateLinkBodySchema = z export const CustomSlugStringSchema = z .string() .min(1) - .max(64) + .max(MAX_SLUG_LENGTH) .regex(/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/i); export const AddSlugBodySchema = z diff --git a/src/constants.ts b/src/constants.ts index 130205f..151bacb 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -2,4 +2,5 @@ // SPDX-License-Identifier: Apache-2.0 export const MIN_SLUG_LENGTH = 3; +export const MAX_SLUG_LENGTH = 128; export const DEFAULT_SLUG_LENGTH = MIN_SLUG_LENGTH; diff --git a/src/mcp/server.ts b/src/mcp/server.ts index ec3e1b5..aae0288 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -62,6 +62,7 @@ import { CustomSlugStringSchema, TIMELINE_RANGES, } from "../api/schemas"; +import { MIN_SLUG_LENGTH, MAX_SLUG_LENGTH, DEFAULT_SLUG_LENGTH } from "../constants"; import pkg from "../../package.json"; type ToolResult = { @@ -216,7 +217,7 @@ export class ShrtnrMCP extends McpAgent, Props> { inputSchema: { url: z.string().url().describe("Destination URL to shorten"), label: z.string().optional().describe("Human-readable label for the link"), - slug_length: z.number().int().min(3).max(16).optional().describe("Length of the random slug (default: 3)"), + slug_length: z.number().int().min(MIN_SLUG_LENGTH).max(MAX_SLUG_LENGTH).optional().describe(`Length of the random slug (default: ${DEFAULT_SLUG_LENGTH})`), custom_slug: z .union([CustomSlugStringSchema, z.array(CustomSlugStringSchema)]) .optional() diff --git a/src/slugs.ts b/src/slugs.ts index 56e25ce..224c0da 100644 --- a/src/slugs.ts +++ b/src/slugs.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { SlugRepository } from "./db"; -import { MIN_SLUG_LENGTH } from "./constants"; +import { MIN_SLUG_LENGTH, MAX_SLUG_LENGTH } from "./constants"; // Unambiguous lowercase characters: removed l, o, 0, 1 to avoid confusion export const RANDOM_CHARSET = "abcdefghijkmnpqrstuvwxyz23456789"; @@ -43,6 +43,6 @@ export function validateCustomSlug(slug: string): string | null { export function validateSlugLength(length: number): string | null { if (!Number.isInteger(length) || length < MIN_SLUG_LENGTH) return `Slug length must be an integer >= ${MIN_SLUG_LENGTH}`; - if (length > 128) return "Slug length must be an integer <= 128"; + if (length > MAX_SLUG_LENGTH) return `Slug length must be an integer <= ${MAX_SLUG_LENGTH}`; return null; } From 8d6f3e46cf7108f430d7082521f0f980db6086a0 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 12:25:26 +0800 Subject: [PATCH 06/17] api/qr: reject non-positive and out-of-range size Route schema previously accepted size="0" (regex /^\d+$/), and the handler parsed it into 0, which renderQrSvg silently turned into a 0-by-0 SVG. Schema now coerces to integer and bounds 1..2048, so Hono's validation hook returns 400 on bad input. Handler defensively re-validates size for callers that come in via the admin route (no Zod) and renderQrSvg short-circuits to null on a non-positive or non-integer size as a final safety net. Tests cover 0, -5, abc, 2049 (rejected), 1, 220, 300, 2048 (accepted). --- src/__tests__/handler/api-links.test.ts | 61 +++++++++++++++++++++++++ src/__tests__/unit/qr.test.ts | 13 ++++++ src/api/links.ts | 4 +- src/api/qr.ts | 21 ++++++--- src/qr.ts | 6 ++- 5 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/__tests__/handler/api-links.test.ts b/src/__tests__/handler/api-links.test.ts index 8ba0ca8..501e1dd 100644 --- a/src/__tests__/handler/api-links.test.ts +++ b/src/__tests__/handler/api-links.test.ts @@ -1180,3 +1180,64 @@ describe("Link+slug access model (design): anyone reads, anyone adds slugs, only expect(enable.status).toBe(403); }); }); + +describe("GET /_/api/links/:id/qr size validation", () => { + async function createTestLink(): Promise { + const res = await SELF.fetch( + authed("/_/admin/api/links", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com" }), + }) + ); + expect(res.status).toBe(201); + const body = await res.json() as { id: number }; + return body.id; + } + + it("rejects size=0 with 400", async () => { + const id = await createTestLink(); + const key = await seedApiKey(env.DB, "read"); + const res = await SELF.fetch(new Request(`https://shrtnr.test/_/api/links/${id}/qr?size=0`, { + headers: { Authorization: `Bearer ${key}` }, + })); + expect(res.status).toBe(400); + }); + + it("rejects negative size with 400", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=-5`)); + expect(res.status).toBe(400); + }); + + it("rejects non-numeric size with 400", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=abc`)); + expect(res.status).toBe(400); + }); + + it("rejects size above 2048 with 400 on the OpenAPI route", async () => { + const id = await createTestLink(); + const key = await seedApiKey(env.DB, "read"); + const res = await SELF.fetch(new Request(`https://shrtnr.test/_/api/links/${id}/qr?size=2049`, { + headers: { Authorization: `Bearer ${key}` }, + })); + expect(res.status).toBe(400); + }); + + it("accepts size=300 and returns SVG sized to 300", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=300`)); + expect(res.status).toBe(200); + const svg = await res.text(); + expect(svg).toMatch(/width="300"/); + }); + + it("accepts no size and returns SVG sized to default 220", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr`)); + expect(res.status).toBe(200); + const svg = await res.text(); + expect(svg).toMatch(/width="220"/); + }); +}); diff --git a/src/__tests__/unit/qr.test.ts b/src/__tests__/unit/qr.test.ts index 3e00c64..00e546a 100644 --- a/src/__tests__/unit/qr.test.ts +++ b/src/__tests__/unit/qr.test.ts @@ -24,6 +24,19 @@ describe("renderQrSvg", () => { expect(svg).not.toBeNull(); expect(extractSvgWidth(svg!)).toBeCloseTo(220, 0); }); + + it("returns null for non-positive or non-integer sizes", () => { + expect(renderQrSvg("https://example.com", { size: 0 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: -5 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: 1.5 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: NaN })).toBeNull(); + }); + + it("accepts size = 1 (smallest valid)", () => { + const svg = renderQrSvg("https://example.com", { size: 1 }); + expect(svg).not.toBeNull(); + expect(extractSvgWidth(svg!)).toBeCloseTo(1, 0); + }); }); describe("makeQR", () => { diff --git a/src/api/links.ts b/src/api/links.ts index 7180d18..eb7bcbe 100644 --- a/src/api/links.ts +++ b/src/api/links.ts @@ -348,8 +348,8 @@ const linkQrRoute = createRoute({ query: z.object({ slug: z.string().regex(/^[a-zA-Z0-9_-]+$/).optional() .openapi({ description: "Optional specific slug. Defaults to the link's primary slug." }), - size: z.string().regex(/^\d+$/).optional() - .openapi({ description: "PNG dimensions in pixels (square). Default per server config." }), + size: z.coerce.number().int().min(1).max(2048).optional() + .openapi({ description: "QR pixel dimensions (square). Integer 1-2048; defaults to 220." }), }), }, responses: { diff --git a/src/api/qr.ts b/src/api/qr.ts index 750ed36..5a8ebce 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -7,23 +7,30 @@ import { renderQrSvg } from "../qr"; import { json } from "./response"; export async function handleLinkQr(request: Request, env: Env, linkId: number): Promise { + const url = new URL(request.url); + const requestedSlug = url.searchParams.get("slug") ?? undefined; + const sizeParam = url.searchParams.get("size"); + + let size: number | undefined; + if (sizeParam !== null) { + const parsed = Number(sizeParam); + if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < 1) { + return json({ error: "size must be a positive integer" }, 400); + } + size = parsed; + } + const result = await getLink(env, linkId); if (!result.ok) return json({ error: result.error }, result.status); const link = result.data; - const url = new URL(request.url); - const requestedSlug = url.searchParams.get("slug"); - const sizeParam = url.searchParams.get("size"); - const size = sizeParam ? parseInt(sizeParam, 10) : undefined; - const slug = requestedSlug ? link.slugs.find((s) => s.slug === requestedSlug) : link.slugs.find((s) => s.is_custom) ?? link.slugs[0]; if (!slug) return json({ error: "Slug not found" }, 404); - const origin = url.origin; - const qrUrl = `${origin}/${slug.slug}?utm_medium=qr`; + const qrUrl = `${url.origin}/${slug.slug}?utm_medium=qr`; const svg = renderQrSvg(qrUrl, { size }); if (!svg) return json({ error: "Failed to generate QR code" }, 500); diff --git a/src/qr.ts b/src/qr.ts index b64c33c..12b03d6 100644 --- a/src/qr.ts +++ b/src/qr.ts @@ -197,12 +197,16 @@ function rsEncode(data: number[], numEcc: number): number[] { /** * Render a QR code as an SVG string. - * Returns null if the text cannot be encoded. + * Returns null if the text cannot be encoded or the size is invalid. */ export function renderQrSvg( text: string, options: { size?: number; fg?: string; bg?: string } = {}, ): string | null { + if (options.size !== undefined && (!Number.isInteger(options.size) || options.size < 1)) { + return null; + } + const matrix = makeQR(text); if (!matrix) return null; From 1cf70131ad258df2c60ddf871fcb5ee56cf51094 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 12:25:33 +0800 Subject: [PATCH 07/17] sdk: bump x-spec-hash for slug-length and QR size schema changes CustomSlugStringSchema max moved 64 -> 128, slug_length max moved 16 -> 128, and QR size moved from a regex string to a coerced integer with min=1/max=2048. None of these change the SDK surface: existing methods still take string slugs and integer sizes, and the SDKs do not enforce server-side bounds locally. Hash bumped to record review per CLAUDE.md spec-hash workflow. --- sdk/dart/pubspec.yaml | 2 +- sdk/python/pyproject.toml | 2 +- sdk/typescript/package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/dart/pubspec.yaml b/sdk/dart/pubspec.yaml index 829a5e5..44b07aa 100644 --- a/sdk/dart/pubspec.yaml +++ b/sdk/dart/pubspec.yaml @@ -4,7 +4,7 @@ version: 1.0.1 homepage: https://oddb.it/shrtnr-website-pub repository: https://github.com/oddbit/shrtnr issue_tracker: https://github.com/oddbit/shrtnr/issues -# x-spec-hash: sha256:3b6e9163bac619b9bbef7ba774b2cd06a9a968f1223b85ba302a018c4bee3b57 +# x-spec-hash: sha256:0d54031654375b3a269cd23d05d66ff44a4b10a894d09b55ed2c1c66405c25b8 # Stored as a comment rather than a top-level key to avoid a pana score deduction for unknown manifest fields. topics: diff --git a/sdk/python/pyproject.toml b/sdk/python/pyproject.toml index 2c79562..c5806bb 100644 --- a/sdk/python/pyproject.toml +++ b/sdk/python/pyproject.toml @@ -92,7 +92,7 @@ warn_redundant_casts = true warn_unreachable = true [tool.shrtnr] -spec_hash = "sha256:3b6e9163bac619b9bbef7ba774b2cd06a9a968f1223b85ba302a018c4bee3b57" +spec_hash = "sha256:0d54031654375b3a269cd23d05d66ff44a4b10a894d09b55ed2c1c66405c25b8" [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 320944b..1471c5c 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -51,5 +51,5 @@ "click-analytics", "custom-slug" ], - "x-spec-hash": "sha256:3b6e9163bac619b9bbef7ba774b2cd06a9a968f1223b85ba302a018c4bee3b57" + "x-spec-hash": "sha256:0d54031654375b3a269cd23d05d66ff44a4b10a894d09b55ed2c1c66405c25b8" } From f4145bd21864d6d720b34a23db45cb4d350c2726 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 12:30:07 +0800 Subject: [PATCH 08/17] sdk/dart: switch update() to take a model and add copyWith Replaces the Object?-with-sentinel update(int id, {label, expiresAt}) signature with update(Link link) / update(Bundle bundle). Callers now read a record, clone-and-edit via copyWith, and pass the modified value back. The omit-vs-clear ambiguity moves into copyWith, where it is encapsulated in the model class via a private _Unset sentinel that external callers cannot construct or match. This restores compile-time type checking on label/expiresAt/description/icon that the previous Object? widening removed, and aligns with idiomatic Dart practice (immutable models with copyWith). Bumps Dart SDK to 1.1.0; the breaking change is intentional and documented in CHANGELOG.md. The Python and TypeScript SDKs keep their existing partial-update shapes; each language now implements the same set/clear/omit feature in its own idiom. --- sdk/dart/CHANGELOG.md | 17 +++ sdk/dart/lib/src/models.dart | 112 ++++++++++++++++++ sdk/dart/lib/src/resources/bundles.dart | 33 +++--- sdk/dart/lib/src/resources/links.dart | 30 +++-- sdk/dart/pubspec.yaml | 2 +- sdk/dart/test/client_test.dart | 144 +++++++++++++++++++++--- 6 files changed, 285 insertions(+), 53 deletions(-) diff --git a/sdk/dart/CHANGELOG.md b/sdk/dart/CHANGELOG.md index 2822b51..3130d1a 100644 --- a/sdk/dart/CHANGELOG.md +++ b/sdk/dart/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## 1.1.0 (2026-05-07) + +**Breaking change to `update()` methods.** `LinksResource.update` and `BundlesResource.update` now take a `Link` / `Bundle` object instead of an id with optional named parameters. To edit a record, call `copyWith` on the value returned by `get()` (or any other read), then pass it to `update()`. + +```dart +// 1.0 +await client.links.update(42, label: null); + +// 1.1 +final link = await client.links.get(42); +await client.links.update(link.copyWith(label: null)); +``` + +`Link`, `Bundle`, and `BundleWithSummary` now expose a `copyWith` method. Pass `null` to a nullable field to clear it; omit the parameter to preserve the current value. The disambiguation between "omit" and "explicit null" is encapsulated in `copyWith` via a private sentinel, so callers never see it. `update()` always sends the full writable payload, which sidesteps the omit-vs-clear ambiguity at the wire level. + +The Python and TypeScript SDKs keep their existing partial-update shapes; each language implements the same set/clear/omit feature in its own idiom. + ## 1.0.1 (2026-04-30) Packaging and documentation only. No public surface changes. diff --git a/sdk/dart/lib/src/models.dart b/sdk/dart/lib/src/models.dart index 4bddde8..b026f85 100644 --- a/sdk/dart/lib/src/models.dart +++ b/sdk/dart/lib/src/models.dart @@ -3,6 +3,17 @@ import 'package:meta/meta.dart'; +// ---- copyWith sentinel ---- + +// Distinguishes "argument not passed" from "argument explicitly null" inside +// copyWith. A private named class is used (not `const Object()`) so the +// sentinel cannot be matched by any value an external caller could construct. +class _Unset { + const _Unset(); +} + +const _Unset _unset = _Unset(); + // ---- Enum types ---- /// Accent color applied to a bundle. @@ -240,6 +251,36 @@ class Link { /// Click count change as a percentage versus the previous equivalent period. /// Absent when comparison data is unavailable. final double? deltaPct; + + /// Returns a copy with selected fields replaced. + /// + /// Pass `null` to a nullable field to clear it. Omit the parameter to + /// preserve the current value. + Link copyWith({ + int? id, + String? url, + Object? label = _unset, + int? createdAt, + Object? expiresAt = _unset, + Object? createdVia = _unset, + String? createdBy, + List? slugs, + int? totalClicks, + Object? deltaPct = _unset, + }) { + return Link( + id: id ?? this.id, + url: url ?? this.url, + label: identical(label, _unset) ? this.label : label as String?, + createdAt: createdAt ?? this.createdAt, + expiresAt: identical(expiresAt, _unset) ? this.expiresAt : expiresAt as int?, + createdVia: identical(createdVia, _unset) ? this.createdVia : createdVia as String?, + createdBy: createdBy ?? this.createdBy, + slugs: slugs ?? this.slugs, + totalClicks: totalClicks ?? this.totalClicks, + deltaPct: identical(deltaPct, _unset) ? this.deltaPct : deltaPct as double?, + ); + } } /// A collection of links grouped to show combined engagement. @@ -302,6 +343,36 @@ class Bundle { /// Unix seconds when the bundle was last modified. final int updatedAt; + + /// Returns a copy with selected fields replaced. + /// + /// Pass `null` to a nullable field to clear it. Omit the parameter to + /// preserve the current value. + Bundle copyWith({ + int? id, + String? name, + Object? description = _unset, + Object? icon = _unset, + BundleAccent? accent, + Object? archivedAt = _unset, + Object? createdVia = _unset, + String? createdBy, + int? createdAt, + int? updatedAt, + }) { + return Bundle( + id: id ?? this.id, + name: name ?? this.name, + description: identical(description, _unset) ? this.description : description as String?, + icon: identical(icon, _unset) ? this.icon : icon as String?, + accent: accent ?? this.accent, + archivedAt: identical(archivedAt, _unset) ? this.archivedAt : archivedAt as int?, + createdVia: identical(createdVia, _unset) ? this.createdVia : createdVia as String?, + createdBy: createdBy ?? this.createdBy, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + ); + } } /// Bundle enriched with range-scoped summary data. @@ -367,6 +438,47 @@ class BundleWithSummary extends Bundle { /// Top member links by click count. final List topLinks; + + /// Returns a copy with selected fields replaced. + /// + /// Pass `null` to a nullable field to clear it. Omit the parameter to + /// preserve the current value. + @override + BundleWithSummary copyWith({ + int? id, + String? name, + Object? description = _unset, + Object? icon = _unset, + BundleAccent? accent, + Object? archivedAt = _unset, + Object? createdVia = _unset, + String? createdBy, + int? createdAt, + int? updatedAt, + int? linkCount, + int? totalClicks, + Object? deltaPct = _unset, + List? sparkline, + List? topLinks, + }) { + return BundleWithSummary( + id: id ?? this.id, + name: name ?? this.name, + description: identical(description, _unset) ? this.description : description as String?, + icon: identical(icon, _unset) ? this.icon : icon as String?, + accent: accent ?? this.accent, + archivedAt: identical(archivedAt, _unset) ? this.archivedAt : archivedAt as int?, + createdVia: identical(createdVia, _unset) ? this.createdVia : createdVia as String?, + createdBy: createdBy ?? this.createdBy, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + linkCount: linkCount ?? this.linkCount, + totalClicks: totalClicks ?? this.totalClicks, + deltaPct: identical(deltaPct, _unset) ? this.deltaPct : deltaPct as double?, + sparkline: sparkline ?? this.sparkline, + topLinks: topLinks ?? this.topLinks, + ); + } } /// Top-link entry preview shown in a [BundleWithSummary]. diff --git a/sdk/dart/lib/src/resources/bundles.dart b/sdk/dart/lib/src/resources/bundles.dart index 1b3da43..2f511ad 100644 --- a/sdk/dart/lib/src/resources/bundles.dart +++ b/sdk/dart/lib/src/resources/bundles.dart @@ -4,9 +4,6 @@ import '../base_client.dart'; import '../models.dart'; -// Sentinel distinguishing "not provided" from an explicit null. -const Object _unset = Object(); - /// Methods for the `/api/bundles` and related endpoints. class BundlesResource { /// Creates a bundles resource backed by [_http]. @@ -55,24 +52,22 @@ class BundlesResource { return Bundle.fromJson(json! as Map); } - /// Update a bundle's name, description, icon, or accent. + /// Update a bundle with the values held by [bundle]. /// - /// Pass `null` for [description] or [icon] to clear the field. - /// Omitting a parameter leaves the field unchanged. - Future update( - int id, { - String? name, - Object? description = _unset, - Object? icon = _unset, - BundleAccent? accent, - }) async { - final body = {}; - if (name != null) body['name'] = name; - if (!identical(description, _unset)) body['description'] = description; - if (!identical(icon, _unset)) body['icon'] = icon; - if (accent != null) body['accent'] = accent.wireValue; + /// The writable fields ([Bundle.name], [Bundle.description], [Bundle.icon], + /// [Bundle.accent]) are always sent on the wire; pass a [Bundle] produced + /// by [Bundle.copyWith] with the desired changes (use `null` on a nullable + /// field to clear it). [BundleWithSummary] is also accepted because it + /// extends [Bundle]; its summary fields are ignored by the server. + Future update(Bundle bundle) async { + final body = { + 'name': bundle.name, + 'description': bundle.description, + 'icon': bundle.icon, + 'accent': bundle.accent.wireValue, + }; final json = - await _http.requestJson('PUT', '/_/api/bundles/$id', body: body); + await _http.requestJson('PUT', '/_/api/bundles/${bundle.id}', body: body); return Bundle.fromJson(json! as Map); } diff --git a/sdk/dart/lib/src/resources/links.dart b/sdk/dart/lib/src/resources/links.dart index 3dce75b..9d9da94 100644 --- a/sdk/dart/lib/src/resources/links.dart +++ b/sdk/dart/lib/src/resources/links.dart @@ -4,9 +4,6 @@ import '../base_client.dart'; import '../models.dart'; -// Sentinel distinguishing "not provided" from an explicit null. -const Object _unset = Object(); - /// Methods for the `/api/links` and related endpoints. class LinksResource { /// Creates a links resource backed by [_http]. @@ -53,21 +50,20 @@ class LinksResource { return Link.fromJson(json! as Map); } - /// Update a link's URL, label, or expiry. + /// Update a link with the values held by [link]. /// - /// Pass `null` for [label] or [expiresAt] to clear the field. - /// Omitting a parameter leaves the field unchanged. - Future update( - int id, { - String? url, - Object? label = _unset, - Object? expiresAt = _unset, - }) async { - final body = {}; - if (url != null) body['url'] = url; - if (!identical(label, _unset)) body['label'] = label; - if (!identical(expiresAt, _unset)) body['expires_at'] = expiresAt; - final json = await _http.requestJson('PUT', '/_/api/links/$id', body: body); + /// The writable fields ([Link.url], [Link.label], [Link.expiresAt]) are + /// always sent on the wire; pass a [Link] produced by [Link.copyWith] with + /// the desired changes (use `null` on a nullable field to clear it). + /// Read-only fields like [Link.id] are used for routing; server-managed + /// fields like slugs and click counts are ignored by the server. + Future update(Link link) async { + final body = { + 'url': link.url, + 'label': link.label, + 'expires_at': link.expiresAt, + }; + final json = await _http.requestJson('PUT', '/_/api/links/${link.id}', body: body); return Link.fromJson(json! as Map); } diff --git a/sdk/dart/pubspec.yaml b/sdk/dart/pubspec.yaml index 44b07aa..b023906 100644 --- a/sdk/dart/pubspec.yaml +++ b/sdk/dart/pubspec.yaml @@ -1,6 +1,6 @@ name: shrtnr description: Dart client for the shrtnr URL shortener API. Create short links, manage custom slugs, and read click analytics. -version: 1.0.1 +version: 1.1.0 homepage: https://oddb.it/shrtnr-website-pub repository: https://github.com/oddbit/shrtnr issue_tracker: https://github.com/oddbit/shrtnr/issues diff --git a/sdk/dart/test/client_test.dart b/sdk/dart/test/client_test.dart index 1e02078..d4d6b2a 100644 --- a/sdk/dart/test/client_test.dart +++ b/sdk/dart/test/client_test.dart @@ -308,28 +308,78 @@ void main() { // ---- 6. links.update ---- group('links.update', () { - test('PUTs /_/api/links/:id with patch body', () async { + test('PUTs /_/api/links/:id with full writable payload', () async { final m = _mock(status: 200, body: _linkJson(url: 'https://new.com')); - await m.client.links.update(1, url: 'https://new.com'); + final original = Link.fromJson(_linkJson(id: 1, url: 'https://old.com')); + await m.client.links.update(original.copyWith(url: 'https://new.com')); expect(m.capture.request!.url.toString(), '$_base/_/api/links/1'); expect(m.capture.request!.method, 'PUT'); final body = jsonDecode(m.capture.request!.body) as Map; expect(body['url'], 'https://new.com'); + // All writable fields are present in the wire payload. + expect(body.containsKey('label'), isTrue); + expect(body.containsKey('expires_at'), isTrue); }); - test('sends label: null to clear the label', () async { + test('copyWith(label: null) sends label: null to clear', () async { final m = _mock(status: 200, body: _linkJson()); - await m.client.links.update(1, label: null); + final original = Link.fromJson({ + ..._linkJson(id: 1), + 'label': 'old label', + }); + await m.client.links.update(original.copyWith(label: null)); final body = jsonDecode(m.capture.request!.body) as Map; - expect(body.containsKey('label'), isTrue); expect(body['label'], isNull); }); - test('omits label when not provided', () async { + test('copyWith preserves unchanged label so the server keeps the old value', + () async { final m = _mock(status: 200, body: _linkJson()); - await m.client.links.update(1, url: 'https://new.com'); + final original = Link.fromJson({ + ..._linkJson(id: 1), + 'label': 'preserve me', + }); + await m.client.links.update(original.copyWith(url: 'https://new.com')); final body = jsonDecode(m.capture.request!.body) as Map; - expect(body.containsKey('label'), isFalse); + expect(body['label'], 'preserve me'); + }); + }); + + // ---- 6a. Link.copyWith semantics ---- + + group('Link.copyWith', () { + test('omitted parameters preserve the existing value', () { + final link = Link.fromJson({ + ..._linkJson(id: 5, url: 'https://orig.com'), + 'label': 'keep', + 'expires_at': 9999999, + }); + final copy = link.copyWith(); + expect(copy.id, 5); + expect(copy.url, 'https://orig.com'); + expect(copy.label, 'keep'); + expect(copy.expiresAt, 9999999); + }); + + test('passing null clears nullable fields', () { + final link = Link.fromJson({ + ..._linkJson(id: 5), + 'label': 'old', + 'expires_at': 9999999, + }); + final copy = link.copyWith(label: null, expiresAt: null); + expect(copy.label, isNull); + expect(copy.expiresAt, isNull); + }); + + test('passing a value overrides only that field', () { + final link = Link.fromJson({ + ..._linkJson(id: 5, url: 'https://orig.com'), + 'label': 'keep', + }); + final copy = link.copyWith(url: 'https://new.com'); + expect(copy.url, 'https://new.com'); + expect(copy.label, 'keep'); }); }); @@ -656,28 +706,90 @@ void main() { // ---- 22. bundles.update ---- group('bundles.update', () { - test('PUTs /_/api/bundles/:id with patch body', () async { + test('PUTs /_/api/bundles/:id with full writable payload', () async { final m = _mock(status: 200, body: _bundleJson(name: 'Updated')); - await m.client.bundles.update(42, name: 'Updated'); + final original = Bundle.fromJson(_bundleJson(id: 42, name: 'Old')); + await m.client.bundles.update(original.copyWith(name: 'Updated')); expect(m.capture.request!.url.toString(), '$_base/_/api/bundles/42'); expect(m.capture.request!.method, 'PUT'); final body = jsonDecode(m.capture.request!.body) as Map; expect(body['name'], 'Updated'); + // Full writable payload: all four fields present. + expect(body.containsKey('description'), isTrue); + expect(body.containsKey('icon'), isTrue); + expect(body['accent'], 'orange'); }); - test('sends description: null to clear the description', () async { + test('copyWith(description: null) sends description: null to clear', () async { final m = _mock(status: 200, body: _bundleJson()); - await m.client.bundles.update(42, description: null); + final original = Bundle.fromJson({ + ..._bundleJson(id: 42), + 'description': 'old desc', + }); + await m.client.bundles.update(original.copyWith(description: null)); final body = jsonDecode(m.capture.request!.body) as Map; - expect(body.containsKey('description'), isTrue); expect(body['description'], isNull); }); - test('omits description when not provided', () async { + test('copyWith preserves the description so the server keeps the old value', + () async { final m = _mock(status: 200, body: _bundleJson()); - await m.client.bundles.update(42, name: 'Only Name'); + final original = Bundle.fromJson({ + ..._bundleJson(id: 42), + 'description': 'preserve me', + }); + await m.client.bundles.update(original.copyWith(name: 'Renamed')); final body = jsonDecode(m.capture.request!.body) as Map; - expect(body.containsKey('description'), isFalse); + expect(body['description'], 'preserve me'); + }); + + test('accepts a BundleWithSummary (covariant)', () async { + final m = _mock(status: 200, body: _bundleJson(name: 'Updated')); + final summary = BundleWithSummary.fromJson(_bundleWithSummaryJson(id: 42)); + await m.client.bundles.update(summary.copyWith(name: 'Updated')); + final body = jsonDecode(m.capture.request!.body) as Map; + expect(body['name'], 'Updated'); + // Summary fields like link_count are not part of the wire payload. + expect(body.containsKey('link_count'), isFalse); + }); + }); + + // ---- 22a. Bundle.copyWith semantics ---- + + group('Bundle.copyWith', () { + test('omitted parameters preserve the existing value', () { + final bundle = Bundle.fromJson({ + ..._bundleJson(id: 42, name: 'orig', accent: 'blue'), + 'description': 'desc', + 'icon': 'star', + }); + final copy = bundle.copyWith(); + expect(copy.id, 42); + expect(copy.name, 'orig'); + expect(copy.description, 'desc'); + expect(copy.icon, 'star'); + expect(copy.accent, BundleAccent.blue); + }); + + test('passing null clears nullable fields', () { + final bundle = Bundle.fromJson({ + ..._bundleJson(id: 42), + 'description': 'old', + 'icon': 'star', + }); + final copy = bundle.copyWith(description: null, icon: null); + expect(copy.description, isNull); + expect(copy.icon, isNull); + }); + + test('BundleWithSummary.copyWith returns BundleWithSummary preserving summary', + () { + final summary = BundleWithSummary.fromJson(_bundleWithSummaryJson(id: 42)); + final copy = summary.copyWith(name: 'Renamed'); + expect(copy, isA()); + expect(copy.name, 'Renamed'); + expect(copy.linkCount, 3); + expect(copy.totalClicks, 100); }); }); From 1c9dc034086af1f22772bc6210db0c9dca143505 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 12:34:13 +0800 Subject: [PATCH 09/17] test/api-schemas: track new MAX_SLUG_LENGTH boundary The CustomSlugStringSchema parameterized test had a 65-char rejection case pinned to the old 64-char cap. Replace with the new 128/129 boundary so the test continues to enforce "reject anything over the documented maximum" against the unified MAX_SLUG_LENGTH = 128 constant. --- src/__tests__/unit/api-schemas.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/__tests__/unit/api-schemas.test.ts b/src/__tests__/unit/api-schemas.test.ts index 0eb2089..8484ce1 100644 --- a/src/__tests__/unit/api-schemas.test.ts +++ b/src/__tests__/unit/api-schemas.test.ts @@ -25,7 +25,8 @@ describe("CustomSlugStringSchema", () => { ["trailing-", false], ["with space", false], ["sl/ash", false], - ["a".repeat(65), false], + ["a".repeat(128), true], + ["a".repeat(129), false], ])("safeParse(%j) -> ok=%s", (value, expectedOk) => { const result = CustomSlugStringSchema.safeParse(value); expect(result.success).toBe(expectedOk); From 7423f55919b57df9d49e3457866259819cf0551d Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:03:04 +0800 Subject: [PATCH 10/17] test(handler/redirect-kv): label POST to suppress autoLabelLink fetch The "populates KV when a link is created" test triggered the createLink handler's autoLabelLink waitUntil branch, which does a real fetch() with a 5s AbortSignal against the test URL. On CI that fetch hangs long enough to push the next beforeEach past its 10s hookTimeout, failing "populates KV when a custom slug is added" deterministically. Passing a label gates out the autoLabelLink branch and keeps the assertion focused on KV write-through. --- src/__tests__/handler/redirect-kv.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/__tests__/handler/redirect-kv.test.ts b/src/__tests__/handler/redirect-kv.test.ts index 0cc6a62..1c72352 100644 --- a/src/__tests__/handler/redirect-kv.test.ts +++ b/src/__tests__/handler/redirect-kv.test.ts @@ -94,11 +94,12 @@ describe("Redirect with KV cache", () => { describe("KV write-through on mutations", () => { it("populates KV when a link is created", async () => { + // label suppresses autoLabelLink's outbound fetch; without it the next beforeEach times out on CI. const res = await SELF.fetch( new Request("https://shrtnr.test/_/admin/api/links", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ url: "https://new-link.com/page" }), + body: JSON.stringify({ url: "https://new-link.com/page", label: "preset" }), }), ); expect(res.status).toBe(201); From fbac310e6f5d0b67150efe45ac4852b6acad739b Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:05:07 +0800 Subject: [PATCH 11/17] api/qr: hoist size bounds to constants, enforce max on both routes The QR size upper bound (2048) was only enforced via the OpenAPI Zod schema on /_/api/links/:id/qr. The admin route /_/admin/api/links/:id/qr goes through the same handler but skips Zod, so a caller could ask for arbitrarily large SVGs and force the renderer to do unbounded work. Hoists MIN_QR_SIZE / MAX_QR_SIZE / DEFAULT_QR_SIZE into src/constants.ts and threads them through: - src/api/qr.ts handler validation (now rejects > MAX_QR_SIZE) - src/api/links.ts Zod schema and OpenAPI description - src/qr.ts renderer min check and default OpenAPI description templated from the same constants so it cannot drift; resulting string is byte-identical so spec hash unchanged. Regression test covers the admin route 400 on size=2049. --- src/__tests__/handler/api-links.test.ts | 6 ++++++ src/api/links.ts | 5 +++-- src/api/qr.ts | 5 +++-- src/constants.ts | 4 ++++ src/qr.ts | 6 ++++-- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/__tests__/handler/api-links.test.ts b/src/__tests__/handler/api-links.test.ts index 501e1dd..2dba119 100644 --- a/src/__tests__/handler/api-links.test.ts +++ b/src/__tests__/handler/api-links.test.ts @@ -1225,6 +1225,12 @@ describe("GET /_/api/links/:id/qr size validation", () => { expect(res.status).toBe(400); }); + it("rejects size above 2048 with 400 on the admin route", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=2049`)); + expect(res.status).toBe(400); + }); + it("accepts size=300 and returns SVG sized to 300", async () => { const id = await createTestLink(); const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=300`)); diff --git a/src/api/links.ts b/src/api/links.ts index eb7bcbe..10f0891 100644 --- a/src/api/links.ts +++ b/src/api/links.ts @@ -25,6 +25,7 @@ import { handlePublicLinkAnalytics, handlePublicLinkTimeline } from "./analytics import { fetchPageTitle } from "../title-fetch"; import { fromServiceResult, json } from "./response"; import { requireScope } from "./scope"; +import { DEFAULT_QR_SIZE, MAX_QR_SIZE, MIN_QR_SIZE } from "../constants"; import type { Env, TimelineRange } from "../types"; import { AddSlugBodySchema, @@ -348,8 +349,8 @@ const linkQrRoute = createRoute({ query: z.object({ slug: z.string().regex(/^[a-zA-Z0-9_-]+$/).optional() .openapi({ description: "Optional specific slug. Defaults to the link's primary slug." }), - size: z.coerce.number().int().min(1).max(2048).optional() - .openapi({ description: "QR pixel dimensions (square). Integer 1-2048; defaults to 220." }), + size: z.coerce.number().int().min(MIN_QR_SIZE).max(MAX_QR_SIZE).optional() + .openapi({ description: `QR pixel dimensions (square). Integer ${MIN_QR_SIZE}-${MAX_QR_SIZE}; defaults to ${DEFAULT_QR_SIZE}.` }), }), }, responses: { diff --git a/src/api/qr.ts b/src/api/qr.ts index 5a8ebce..2fc127d 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -1,6 +1,7 @@ // Copyright 2026 Oddbit (https://oddbit.id) // SPDX-License-Identifier: Apache-2.0 +import { MAX_QR_SIZE, MIN_QR_SIZE } from "../constants"; import { Env } from "../types"; import { getLink } from "../services/link-management"; import { renderQrSvg } from "../qr"; @@ -14,8 +15,8 @@ export async function handleLinkQr(request: Request, env: Env, linkId: number): let size: number | undefined; if (sizeParam !== null) { const parsed = Number(sizeParam); - if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < 1) { - return json({ error: "size must be a positive integer" }, 400); + if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < MIN_QR_SIZE || parsed > MAX_QR_SIZE) { + return json({ error: `size must be an integer between ${MIN_QR_SIZE} and ${MAX_QR_SIZE}` }, 400); } size = parsed; } diff --git a/src/constants.ts b/src/constants.ts index 151bacb..edfb7a6 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -4,3 +4,7 @@ export const MIN_SLUG_LENGTH = 3; export const MAX_SLUG_LENGTH = 128; export const DEFAULT_SLUG_LENGTH = MIN_SLUG_LENGTH; + +export const MIN_QR_SIZE = 1; +export const MAX_QR_SIZE = 2048; +export const DEFAULT_QR_SIZE = 220; diff --git a/src/qr.ts b/src/qr.ts index 12b03d6..b433b46 100644 --- a/src/qr.ts +++ b/src/qr.ts @@ -1,6 +1,8 @@ // Copyright 2026 Oddbit (https://oddbit.id) // SPDX-License-Identifier: Apache-2.0 +import { DEFAULT_QR_SIZE, MIN_QR_SIZE } from "./constants"; + /** * Generate a QR code matrix for the given text. * Returns a 2D boolean grid (true = dark module) or null if the text is too long. @@ -203,7 +205,7 @@ export function renderQrSvg( text: string, options: { size?: number; fg?: string; bg?: string } = {}, ): string | null { - if (options.size !== undefined && (!Number.isInteger(options.size) || options.size < 1)) { + if (options.size !== undefined && (!Number.isInteger(options.size) || options.size < MIN_QR_SIZE)) { return null; } @@ -213,7 +215,7 @@ export function renderQrSvg( const modules = matrix.length; const margin = 4; const total = modules + margin * 2; - const cellSize = (options.size ?? 220) / total; + const cellSize = (options.size ?? DEFAULT_QR_SIZE) / total; const svgSize = total * cellSize; const fg = options.fg ?? "#001110"; const bg = options.bg ?? "#ffffff"; From b596f848cf516fd444485791f5f2f4b3171c2957 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:05:22 +0800 Subject: [PATCH 12/17] api/qr,mcp: default to primary slug, not first custom The default slug-pick when no ?slug= query param is provided was link.slugs.find(s => s.is_custom) ?? link.slugs[0]. After SlugRepository .setPrimary or after disabling/removing a primary custom slug, the "primary" slug can be the auto-generated one, while a custom slug sits non-primary alongside it. The find(is_custom) pick returned that non-primary custom slug, contradicting the documented "Defaults to the link's primary slug" behavior. Same pattern lived in two places, both fixed: - src/api/qr.ts handleLinkQr default-slug pick - src/mcp/server.ts get_link_qr default-slug pick (also corrects the parameter description from "custom slug or primary" to "primary slug") Regression test: link with auto-slug primary + non-primary custom slug must produce the same QR for /qr and /qr?slug=. --- src/__tests__/handler/api-links.test.ts | 35 +++++++++++++++++++++++++ src/api/qr.ts | 2 +- src/mcp/server.ts | 4 +-- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/__tests__/handler/api-links.test.ts b/src/__tests__/handler/api-links.test.ts index 2dba119..468f387 100644 --- a/src/__tests__/handler/api-links.test.ts +++ b/src/__tests__/handler/api-links.test.ts @@ -1246,4 +1246,39 @@ describe("GET /_/api/links/:id/qr size validation", () => { const svg = await res.text(); expect(svg).toMatch(/width="220"/); }); + + it("defaults to the link's primary slug, not just any custom slug", async () => { + // Setup: link with auto-slug ("primary"), plus a custom slug that is + // explicitly NOT primary. Default-pick must pick the auto-slug. + const createRes = await SELF.fetch( + authed("/_/admin/api/links", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com/primary-pick" }), + }) + ); + expect(createRes.status).toBe(201); + const created = await createRes.json() as { id: number; slugs: { slug: string }[] }; + const id = created.id; + const autoSlug = created.slugs[0].slug; + + const addRes = await SELF.fetch(authed(`/_/admin/api/links/${id}/slugs`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ slug: "my-custom-pick" }), + })); + expect(addRes.status).toBe(201); + + // addCustom flipped primary to "my-custom-pick"; flip it back so the + // test exercises the bug shape: primary != custom. + await env.DB.prepare("UPDATE slugs SET is_primary = (slug = ?) WHERE link_id = ?") + .bind(autoSlug, id) + .run(); + + const noSlug = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr`)); + const explicitPrimary = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?slug=${autoSlug}`)); + expect(noSlug.status).toBe(200); + expect(explicitPrimary.status).toBe(200); + expect(await noSlug.text()).toBe(await explicitPrimary.text()); + }); }); diff --git a/src/api/qr.ts b/src/api/qr.ts index 2fc127d..804c969 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -27,7 +27,7 @@ export async function handleLinkQr(request: Request, env: Env, linkId: number): const link = result.data; const slug = requestedSlug ? link.slugs.find((s) => s.slug === requestedSlug) - : link.slugs.find((s) => s.is_custom) ?? link.slugs[0]; + : link.slugs.find((s) => s.is_primary) ?? link.slugs[0]; if (!slug) return json({ error: "Slug not found" }, 404); diff --git a/src/mcp/server.ts b/src/mcp/server.ts index aae0288..4fba5f9 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -469,7 +469,7 @@ export class ShrtnrMCP extends McpAgent, Props> { description: "Get a QR code SVG for a short link. The QR encodes the short URL with a ?qr tracking parameter.", inputSchema: { link_id: z.number().int().positive().describe("Numeric ID of the link"), - slug: z.string().optional().describe("Specific slug to use (defaults to custom slug or primary)"), + slug: z.string().optional().describe("Specific slug to use (defaults to the link's primary slug)"), base_url: z.string().url().describe("Base URL of the shrtnr instance, e.g. https://oddb.it"), }, annotations: { title: "Link QR code", ...READ_ONLY }, @@ -481,7 +481,7 @@ export class ShrtnrMCP extends McpAgent, Props> { const target = requestedSlug ? link.slugs.find((s) => s.slug === requestedSlug) - : (link.slugs.find((s) => s.is_custom) ?? link.slugs[0]); + : (link.slugs.find((s) => s.is_primary) ?? link.slugs[0]); if (!target) return fail("Slug not found"); From c51d7fcad913e058f827b076c601fd47e0eb138a Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:05:33 +0800 Subject: [PATCH 13/17] sdk/dart: bump 1.1.0 -> 2.0.0 for breaking update() signature Under SemVer, a breaking API change on a 1.x package should bump the major. The 1.1.0 entry already documented LinksResource.update and BundlesResource.update as taking a Link/Bundle object instead of an id plus named parameters. Renames the in-progress entry to 2.0.0 and bumps pubspec.yaml accordingly. Spec hash is unchanged so no parity work required. --- sdk/dart/CHANGELOG.md | 2 +- sdk/dart/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/dart/CHANGELOG.md b/sdk/dart/CHANGELOG.md index 3130d1a..fdef98e 100644 --- a/sdk/dart/CHANGELOG.md +++ b/sdk/dart/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 1.1.0 (2026-05-07) +## 2.0.0 (2026-05-07) **Breaking change to `update()` methods.** `LinksResource.update` and `BundlesResource.update` now take a `Link` / `Bundle` object instead of an id with optional named parameters. To edit a record, call `copyWith` on the value returned by `get()` (or any other read), then pass it to `update()`. diff --git a/sdk/dart/pubspec.yaml b/sdk/dart/pubspec.yaml index b023906..168c76d 100644 --- a/sdk/dart/pubspec.yaml +++ b/sdk/dart/pubspec.yaml @@ -1,6 +1,6 @@ name: shrtnr description: Dart client for the shrtnr URL shortener API. Create short links, manage custom slugs, and read click analytics. -version: 1.1.0 +version: 2.0.0 homepage: https://oddb.it/shrtnr-website-pub repository: https://github.com/oddbit/shrtnr issue_tracker: https://github.com/oddbit/shrtnr/issues From 0f02798f2ca7c0a3be8345ebab7bd86ea08a65ff Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:25:11 +0800 Subject: [PATCH 14/17] qr: enforce MAX_QR_SIZE at the renderer too renderQrSvg already validated MIN_QR_SIZE but not the upper bound, relying entirely on the HTTP-facing handler to cap size. The function is exported and called from src/api/qr.ts and src/mcp/server.ts; if a new caller forgets to pre-validate, an oversized size silently passes through to QR generation and produces an arbitrarily large SVG. Adds the > MAX_QR_SIZE branch to the existing nullable-return path. Unit tests cover 2049 and 100000 (rejected) and 2048 (accepted at the boundary). --- src/__tests__/unit/qr.test.ts | 11 +++++++++++ src/qr.ts | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/__tests__/unit/qr.test.ts b/src/__tests__/unit/qr.test.ts index 00e546a..6f136b8 100644 --- a/src/__tests__/unit/qr.test.ts +++ b/src/__tests__/unit/qr.test.ts @@ -32,11 +32,22 @@ describe("renderQrSvg", () => { expect(renderQrSvg("https://example.com", { size: NaN })).toBeNull(); }); + it("returns null for sizes above MAX_QR_SIZE (2048)", () => { + expect(renderQrSvg("https://example.com", { size: 2049 })).toBeNull(); + expect(renderQrSvg("https://example.com", { size: 100000 })).toBeNull(); + }); + it("accepts size = 1 (smallest valid)", () => { const svg = renderQrSvg("https://example.com", { size: 1 }); expect(svg).not.toBeNull(); expect(extractSvgWidth(svg!)).toBeCloseTo(1, 0); }); + + it("accepts size = 2048 (largest valid)", () => { + const svg = renderQrSvg("https://example.com", { size: 2048 }); + expect(svg).not.toBeNull(); + expect(extractSvgWidth(svg!)).toBeCloseTo(2048, 0); + }); }); describe("makeQR", () => { diff --git a/src/qr.ts b/src/qr.ts index b433b46..504017c 100644 --- a/src/qr.ts +++ b/src/qr.ts @@ -1,7 +1,7 @@ // Copyright 2026 Oddbit (https://oddbit.id) // SPDX-License-Identifier: Apache-2.0 -import { DEFAULT_QR_SIZE, MIN_QR_SIZE } from "./constants"; +import { DEFAULT_QR_SIZE, MAX_QR_SIZE, MIN_QR_SIZE } from "./constants"; /** * Generate a QR code matrix for the given text. @@ -205,7 +205,7 @@ export function renderQrSvg( text: string, options: { size?: number; fg?: string; bg?: string } = {}, ): string | null { - if (options.size !== undefined && (!Number.isInteger(options.size) || options.size < MIN_QR_SIZE)) { + if (options.size !== undefined && (!Number.isInteger(options.size) || options.size < MIN_QR_SIZE || options.size > MAX_QR_SIZE)) { return null; } From 73ef262f5c752020627a846161238dce3964dc89 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:25:20 +0800 Subject: [PATCH 15/17] api/qr: reject empty ?slug= with 400 on the admin route The OpenAPI route rejects empty slugs via Zod regex /^[a-zA-Z0-9_-]+$/. The admin route bypasses Zod and goes straight to handleLinkQr, where searchParams.get("slug") returns "" for ?slug=. "" is falsy, so the handler silently fell back to the primary slug default and returned a QR for a slug the caller didn't ask for. That is inconsistent with the OpenAPI route's 400 and surprising on its own merits. Adds an explicit empty-string check that mirrors the OpenAPI behavior. Other malformed slugs (spaces, special chars) still pass through to the slug lookup and return 404 if not found; matching Zod's full regex on the admin route is out of scope. --- src/__tests__/handler/api-links.test.ts | 6 ++++++ src/api/qr.ts | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/__tests__/handler/api-links.test.ts b/src/__tests__/handler/api-links.test.ts index 468f387..559af3f 100644 --- a/src/__tests__/handler/api-links.test.ts +++ b/src/__tests__/handler/api-links.test.ts @@ -1231,6 +1231,12 @@ describe("GET /_/api/links/:id/qr size validation", () => { expect(res.status).toBe(400); }); + it("rejects empty slug with 400 on the admin route", async () => { + const id = await createTestLink(); + const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?slug=`)); + expect(res.status).toBe(400); + }); + it("accepts size=300 and returns SVG sized to 300", async () => { const id = await createTestLink(); const res = await SELF.fetch(authed(`/_/admin/api/links/${id}/qr?size=300`)); diff --git a/src/api/qr.ts b/src/api/qr.ts index 804c969..15af509 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -12,6 +12,10 @@ export async function handleLinkQr(request: Request, env: Env, linkId: number): const requestedSlug = url.searchParams.get("slug") ?? undefined; const sizeParam = url.searchParams.get("size"); + if (requestedSlug === "") { + return json({ error: "slug must not be empty" }, 400); + } + let size: number | undefined; if (sizeParam !== null) { const parsed = Number(sizeParam); From df19b9a73fec789448746836382ff0975e0469e9 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:25:28 +0800 Subject: [PATCH 16/17] services/links: map enable() race-condition null to 404 LinkRepository.enable() returns null if the link no longer exists when the existence check fires (i.e., a concurrent delete won the race between the service's getById and the repository's UPDATE). enableLink asserted non-null with `!` and would crash on `enabled!.slugs`, surfacing as a 500. Maps the null return to fail(404, "Link not found") and removes the non-null assertions on the subsequent reads, matching how the rest of the service handles "link not found" responses. disableLink has the same shape; left for a separate change rather than expanding this PR's scope. --- src/services/link-management.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/services/link-management.ts b/src/services/link-management.ts index 0315d23..264710b 100644 --- a/src/services/link-management.ts +++ b/src/services/link-management.ts @@ -172,18 +172,20 @@ export async function enableLink(env: Env, id: number, identity: string): Promis if (!link) return fail(404, "Link not found"); if (link.created_by !== identity) return fail(403, "Only the link owner can enable this link"); const enabled = await LinkRepository.enable(env.DB, id); + // null on race: link deleted between getById and the UPDATE. + if (!enabled) return fail(404, "Link not found"); await Promise.all( - enabled!.slugs.map((s) => + enabled.slugs.map((s) => SlugCache.put(env.SLUG_KV, s.slug, { - url: enabled!.url, + url: enabled.url, disabled_at: s.disabled_at, expires_at: null, }), ), ); - return ok(enabled!); + return ok(enabled); } export async function deleteLink(env: Env, id: number, identity: string): Promise> { From 888d18e195c6cd785fb20eeec12a545f539edb49 Mon Sep 17 00:00:00 2001 From: Dennis Alund Date: Thu, 7 May 2026 13:25:32 +0800 Subject: [PATCH 17/17] sdk/dart: docs polish for 2.0.0 - CHANGELOG: example comments said "// 1.1" but the release is now 2.0.0 after the SemVer bump; relabel as "// 1.x" and "// 2.0". - Resource docstrings on update(): replace "ignored by the server" with language that reflects the actual behavior. UpdateLinkBody and UpdateBundleBody schemas are .strict() server-side and would reject unknown fields; the SDK simply doesn't include them in the payload. --- sdk/dart/CHANGELOG.md | 4 ++-- sdk/dart/lib/src/resources/bundles.dart | 4 +++- sdk/dart/lib/src/resources/links.dart | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sdk/dart/CHANGELOG.md b/sdk/dart/CHANGELOG.md index fdef98e..d67d8f3 100644 --- a/sdk/dart/CHANGELOG.md +++ b/sdk/dart/CHANGELOG.md @@ -5,10 +5,10 @@ **Breaking change to `update()` methods.** `LinksResource.update` and `BundlesResource.update` now take a `Link` / `Bundle` object instead of an id with optional named parameters. To edit a record, call `copyWith` on the value returned by `get()` (or any other read), then pass it to `update()`. ```dart -// 1.0 +// 1.x await client.links.update(42, label: null); -// 1.1 +// 2.0 final link = await client.links.get(42); await client.links.update(link.copyWith(label: null)); ``` diff --git a/sdk/dart/lib/src/resources/bundles.dart b/sdk/dart/lib/src/resources/bundles.dart index 2f511ad..f698769 100644 --- a/sdk/dart/lib/src/resources/bundles.dart +++ b/sdk/dart/lib/src/resources/bundles.dart @@ -58,7 +58,9 @@ class BundlesResource { /// [Bundle.accent]) are always sent on the wire; pass a [Bundle] produced /// by [Bundle.copyWith] with the desired changes (use `null` on a nullable /// field to clear it). [BundleWithSummary] is also accepted because it - /// extends [Bundle]; its summary fields are ignored by the server. + /// extends [Bundle]; only the writable fields above are put on the wire, + /// so the summary fields never reach the server (whose update schema is + /// strict and would reject them). Future update(Bundle bundle) async { final body = { 'name': bundle.name, diff --git a/sdk/dart/lib/src/resources/links.dart b/sdk/dart/lib/src/resources/links.dart index 9d9da94..56eda96 100644 --- a/sdk/dart/lib/src/resources/links.dart +++ b/sdk/dart/lib/src/resources/links.dart @@ -56,7 +56,8 @@ class LinksResource { /// always sent on the wire; pass a [Link] produced by [Link.copyWith] with /// the desired changes (use `null` on a nullable field to clear it). /// Read-only fields like [Link.id] are used for routing; server-managed - /// fields like slugs and click counts are ignored by the server. + /// fields like slugs and click counts are not included in the request body + /// (the server's update schema is strict and would reject them). Future update(Link link) async { final body = { 'url': link.url,