Accept IRI-valued quote URL properties

[Palcimer]

Summary

In Fedify 2.3.6, vocabulary decoding threw TypeError: Invalid URL when a JSON-LD context declared _misskey_quote or quoteUri with "@type": "@id". Such terms expand to {"@id": …} nodes, but the fedify:url scalar type read @value only.
Updated the fedify:url decoder to read whichever of the two the node carries, and widened its dataCheck() to accept both shapes. Also added a regression test that parses a Note whose context declares _misskey_quote and quoteUri with "@type": "@id".

Assisted-by: Claude Code:claude-opus-5

Related issue

• closes IRI-valued quote URL aliases are dropped during JSON-LD parsing #1015

Changes

• Updated the fedify:url decoder to read @id when @value is absent.
• Updated the fedify:url datacheck() to accept a node carrying either @value or @id.
• Added a regression test that parses a Note whose context declares _misskey_quote and quoteUri with "@type": "@id".

Benefits

• Applications can now parse objects from senders whose context declares _misskey_quote or quoteUri with "@type": "@id", which previously failed with TypeError: Invalid URL.

Checklist

• Did you add a changelog entry to the CHANGES.md?
• Did you write a regression test to reproduce the bug (if it's a bug fix)?
• Did you run mise test on your machine?

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e526438e-6f5e-4bfb-9a96-cc94a01c4146

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
packages/vocab-tools/src/type.ts	16.66%	5 Missing ⚠️

Files with missing lines	Coverage Δ
packages/vocab-tools/src/type.ts	84.00% <16.66%> (-0.30%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dahlia]

Thanks for tracking this down. The diagnosis is right, and the regression test is a real one: I reverted type.ts, regenerated vocab.ts and confirmed it fails with the exact error #1015 reports. Reading @id as a fallback also matches the fedify:gatewayUrl type that already exists on main, so this will merge forward cleanly.

One thing to settle before it lands. Parsing still throws TypeError: Invalid URL for several shapes a real sender can produce, and one of them, an inlined quote object carrying no id, fails the whole Note with the same error the issue reports. A quote URL we cannot parse should be dropped instead. I left a verified patch for that on the decoder, together with at:// handling so the two @id paths agree. The remaining comments are about the changelog entry.

The pull request also conflicts now that 2.0.28 has shipped, so please rebase onto the current 2.0-maintenance. CHANGES.md is the only conflict: the “To be released” section you appended to has been released, and changes.d/ upstream is empty with next.txt at 2.0.29. Drop the hand-written hunk, keep your fragments, and run sacho sync to materialize them into the 2.0.29 section.

[dahlia]

The shape here is right, and it matches a precedent already on main: the fedify:gatewayUrl scalar type there reads @id or @value in exactly this way, so this will merge forward cleanly.

What it does not do yet is stop the crash the issue is about. I regenerated vocab.ts with this patch applied and fed Note.fromJsonLd() a range of values under a context declaring _misskey_quote with "@type": "@id":

value	result with this patch
"https://example.com/q"	https://example.com/q
{"type": "Note", "id": "https://example.com/q2"}	https://example.com/q2
{"type": "Note", "content": "hi"}	throws TypeError: Invalid URL: 'undefined'
"_:b0"	throws TypeError: Invalid URL: '_:b0'
""	throws TypeError: Invalid URL: './'
"notes/rel"	throws TypeError: Invalid URL: 'notes/rel'
"at://did:plc:abc/app.bsky.feed.post/xyz"	throws TypeError: Invalid URL

Row three is the same error text, from the same property, that #1015 reports. A sender that inlines the quoted object without an id still fails the entire Note, and a blank node identifier does the same. A quote URL we cannot parse should be dropped, not taken down the object around it.

The reason dataCheck() cannot protect against this today is in packages/vocab-tools/src/codec.ts: getDecoders(), which emits the guard, runs only when property.range.length > 1 (line 465), and fedify:url is a sole range in all four properties that use it. The single-range path at line 444 calls getDecoder() unguarded, so there is no undefined to skip on.

The last row deserves its own note. codec.ts lines 428 to 438 already carry an at:// special case for the non-scalar @id path, so as written the two @id paths disagree about ATProto URIs.

Making the decoder tolerant needs a small change in the generator. Dropping the single-range special case altogether, the way main does it, does not work on this branch: the dataCheck ? decoder : undefined ternary widens narrow literal unions, and fedify:proofPurpose then fails type checking with TS2345. An opt-in flag avoids that. This is the version I verified:

--- a/packages/vocab-tools/src/class.ts
+++ b/packages/vocab-tools/src/class.ts
@@ -154,6 +154,17 @@ export async function* generateClasses(
     isTemporalInstant,
 } from "@fedify/vocab-runtime/temporal";\n`;
   yield `
+function canParseIri(iri: string): boolean {
+  return URL.canParse(iri) || iri.startsWith("at://");
+}
+
+function parseIri(iri: string): URL {
+  return !URL.canParse(iri) && iri.startsWith("at://")
+    ? new URL("at://" + encodeURIComponent(iri.substring(5)))
+    : new URL(iri);
+}
+`;
+  yield `
 function isValidLanguageTag(language: string): boolean {
--- a/packages/vocab-tools/src/type.ts
+++ b/packages/vocab-tools/src/type.ts
@@ -17,6 +17,12 @@ interface ScalarType {
   dataCheck(variable: string): string;
   decoder(variable: string, baseUrlVar: string): string;
+  /**
+   * Whether a value that fails `dataCheck()` should be skipped instead of
+   * being handed to `decoder()`.  Set this for types decoded from untrusted
+   * remote input, where one malformed value must not fail the whole object.
+   */
+  skipUnparsable?: boolean;
 }
@@ -309,14 +315,15 @@ const scalarTypes: Record<string, ScalarType> = {
     dataCheck(v) {
       return `typeof ${v} === "object" &&
-      (("@value" in ${v} && typeof ${v}["@value"] === "string" &&
-        ${v}["@value"] !== "" && ${v}["@value"] !== "/") ||
-      ("@id" in ${v} && typeof ${v}["@id"] === "string" &&
-        ${v}["@id"] !== "" && ${v}["@id"] !== "/"))`;
+      ((typeof ${v}["@value"] === "string" && canParseIri(${v}["@value"])) ||
+       (typeof ${v}["@id"] === "string" && canParseIri(${v}["@id"])))`;
     },
     decoder(v) {
-      return `new URL(typeof ${v}["@value"] === "string" ? ${v}["@value"] : ${v}["@id"])`;
+      return `parseIri(
+        typeof ${v}["@value"] === "string" ? ${v}["@value"] : ${v}["@id"]
+      )`;
     },
+    skipUnparsable: true,
   },
@@ -597,6 +604,10 @@ export function getDecoder(
+export function skipsUnparsable(typeUri: string): boolean {
+  return scalarTypes[typeUri]?.skipUnparsable ?? false;
+}
+
 export function getDataCheck(
--- a/packages/vocab-tools/src/codec.ts
+++ b/packages/vocab-tools/src/codec.ts
@@ -6,11 +6,13 @@ import {
   areAllScalarTypes,
   emitOverride,
   getAllProperties,
+  getDataCheck,
   getDecoder,
   getDecoders,
   getEncoders,
   getSubtypes,
   isCompactableType,
+  skipsUnparsable,
 } from "./type.ts";
@@ -441,7 +443,13 @@ export async function* generateDecoder(
     yield `
       const decoded =
     `;
+    const lenient = property.range.length == 1 &&
+      skipsUnparsable(property.range[0]);
     if (property.range.length == 1) {
+      if (lenient) {
+        yield getDataCheck(property.range[0], types, "v");
+        yield " ? ";
+      }
       yield getDecoder(
         property.range[0],
@@ -449,6 +457,7 @@ export async function* generateDecoder(
         "options",
         `(values["@id"] == null ? options.baseUrl : new URL(values["@id"]))`,
       );
+      if (lenient) yield " : undefined";
     } else {
@@ -462,7 +471,7 @@ export async function* generateDecoder(
     yield `
       ;
     `;
-    if (property.range.length > 1) {
+    if (property.range.length > 1 || lenient) {
       yield `
       if (typeof decoded === "undefined") {

With that applied, every throwing row above becomes null, the at:// row parses to at://did%3Aplc%3Aabc%2Fapp.bsky.feed.post%2Fxyz (the same value codec.ts line 433 produces for the other @id path), and all 22,030 tests in packages/vocab pass. The vocab-tools snapshots need regenerating.

Sharing parseIri() this way also lets codec.ts lines 428 to 438 drop their inline copy of the at:// handling, and it lines the branch up with the parseIri() that @fedify/vocab-runtime already exports on main.

[dahlia]

Keeping dataCheck() in step with decoder() is the right instinct, so this should stay. Two things to know about it though.

It generates nothing today. The guard is emitted only for properties with more than one range, and fedify:url is a sole range in all four properties that use it, namely quoteUrl on Article, ChatMessage, Note and Question. That is why the snapshot diff in this pull request touches the decoder line four times and no dataCheck() line at all. Worth knowing mostly so the changelog does not advertise it as something users can observe.

Second, fedify:gatewayUrl on main reads @id first and falls back to @value, while this reads them the other way round. The two are mutually exclusive in practice so nothing changes, but matching the existing order keeps the two definitions readable side by side.

If the decoder becomes tolerant the way I suggest below, this guard starts being emitted and its condition becomes load-bearing. It would then need to reject more than the empty string and /: both _:b0 and a relative IRI pass those checks and still fail new URL().

[dahlia]

This is a real regression test. I reverted type.ts to its base revision, forced a regeneration and confirmed it fails with TypeError: Invalid URL: 'undefined', which is the error #1015 reports.

Two gaps worth closing:

• quoteUrl itself is never declared with "@type": "@id", only the two aliases are. The primary property goes through the same decoder, so covering it costs one line.
• Nothing mixes the two forms, for instance a @value-shaped quoteUrl alongside an @id-shaped _misskey_quote. That is what pins the precedence order the existing Note.quoteUrl test establishes.

Once unparsable values are dropped instead of thrown, this is also the right place to assert it: a _misskey_quote expanding to a node with no @id should leave quoteUrl at null rather than fail fromJsonLd().

[dahlia]

This describes the generator rather than the change. fedify:url and dataCheck() are vocab-tools internals that no user of @fedify/vocab ever sees, and the second sentence describes something that currently generates no code at all. Entries here should say what broke for users and when.

Two smaller things. Neighbouring bug fix entries open with “Fixed”, not “Updated”. And the marker should carry the pull request number alongside the issue, the way the @fedify/redis entry above carries [[#1028], [#1034]].

Something along these lines:

 -  Fixed `Note.quoteUrl`, and the same property on `Article`, `ChatMessage`,
    and `Question`, being dropped or throwing `TypeError: Invalid URL` when
    the sender's JSON-LD context declared `_misskey_quote` or `quoteUri` with
    `"@type": "@id"`.  Such terms expand to a node object carrying `@id`
    rather than `@value`, and the parser read `@value` only.  Notes from
    Misskey-compatible servers are affected.
    [[#1015], [#1043] by Jang Hanarae]

Note the two spaces after each sentence, which the surrounding entries use. Edit changes.d/vocab/iri-valued-quote-url.md and run sacho sync rather than editing this file by hand.

[dahlia]

Pinning the links in front matter is the right call, but #1043 is missing. Once the entry cites the pull request as well as the issue, this needs both:

links:
  '#1015': https://github.com/fedify-dev/fedify/issues/1015
  '#1043': https://github.com/fedify-dev/fedify/pull/1043

Separately, this change spans two published packages and only one of them has a fragment. The behaviour users observe lands in @fedify/vocab, but the source change is in packages/vocab-tools/, which ships as @fedify/vocab-tools, so changes.d/vocab-tools/ should carry its own entry describing the generator change. Run sacho sync afterwards to materialize both sections into CHANGES.md.