Skip to content

Refuse unparseable https preview URLs instead of crashing validate - #243

Closed
bfs2021 wants to merge 1 commit into
TMHSDigital:mainfrom
bfs2021:fix/preview-invalid-url-crash
Closed

bfs2021 wants to merge 1 commit into
TMHSDigital:mainfrom
bfs2021:fix/preview-invalid-url-crash

Conversation

@bfs2021

@bfs2021 bfs2021 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Problem

npm run validate crashes with a stack trace when an entry's preview: looks like an https URL but cannot be parsed (for example preview: https://[bad), instead of reporting a validation error. Fixes #233.

Root cause

checkPreviewField evaluated safeImageUrl(v) === new URL(v).href on one line. For an unparseable URL safeImageUrl returns null, but the second operand still calls new URL(v), which throws ERR_INVALID_URL and aborts the whole validate run before any other check can report.

Fix

Parse v defensively once (try/catch around new URL) and only compare against safeImageUrl(v) when the URL actually parsed. An unparseable URL now falls through to the normal preview must be an https:// image URL on a public host error, and behaviour for every parseable value is unchanged (same comparison as before).

Testing

  • Two new cases in site/checks.test.mjs: unparseable https URL (https://[bad) and URL with userinfo — both rejected with the public-host message. Before the fix the unparseable case crashed the suite (TypeError: Invalid URL at checks.mjs:1066); after it the suite is green: 242 assertions across 34 checks.
  • node site/lib/lib.test.mjs: 986 assertions OK.
  • npm run validate over the real catalog completes without a crash.

PR drafted with an agent; the diff and tests above were verified locally.

checkPreviewField called new URL(v) on values safeImageUrl had already
rejected as unparseable, so a typo like https://[bad aborted the whole
validate run with a stack trace instead of reporting the field error.
Parse defensively and report the normal public-host error.
TMHSDigital added a commit that referenced this pull request Nov 6, 2026
The fixes these pull requests made had already landed on main, so their
branches conflict; these are the parts main did not have:

- data.schema.json: list items must be non-empty strings (minLength 1),
  from #244. schema-check.mjs gains minLength (by character) so the
  build's own schema check can enforce it.
- checks.test.mjs: a preview URL with an unclosed IPv6 bracket, and one
  carrying credentials, are refused rather than thrown, from #243.
- site/README.md: a paragraph on opensearch.xml, from #242.

Co-authored-by: harshkanhat <kanhatharsh@gmail.com>
Co-authored-by: bfs2021 <101915083+bfs2021@users.noreply.github.com>
Co-authored-by: Ritesh Yadav <riteshyadav33302111@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LpkeXQPaUhxjgqdqnk6BrJ
@TMHSDigital

Copy link
Copy Markdown
Owner

Thank you! #233 was fixed on main in 4b7fd13 with the same approach as yours (catch the new URL() throw, then compare against safeImageUrl), so this branch now conflicts and I'm closing it as superseded.

Your two extra test cases, the unclosed IPv6 bracket and a URL carrying credentials, weren't covered on main, so I added them (a3cd8b0, with you as co-author).

@TMHSDigital TMHSDigital closed this Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

validate crashes with a stack trace on an unparseable https preview URL

2 participants