Changeset c52dc82 in Klonkt


Ignore:
Timestamp:
07/28/2026 08:37:08 AM (6 weeks ago)
Author:
Robin Genis <roboburr@…>
Branches:
main
Children:
3f32994
Parents:
0101d0a
Message:

Thumbnails ontbraken: providerlijst eruit, en grote pagina-s worden gelezen i.p.v. geweigerd

Twee bugs die elkaar maskeerden, allebei van mij.

DE PROVIDERLIJST WAS HET PROBLEEM, NIET DE OPLOSSING. Een YouTube-link matchte de
hardcoded lijst, kortsloot voor oEmbed, en kwam eruit als een kaart zonder titel
en zonder thumbnail. Er werd dus niets opgeslagen. YouTube levert gewoon oEmbed
en og:image, net als de rest, dus de generieke weg doet het beter dan het
speciale geval. De lijst is weg: geen whitelist meer om te onderhouden, en of een
embed getoond mag worden is een guardian-besluit, geen kwestie van welke host het
is.

DE BODY-CAP SLOEG DE KOP ERAF. Een pagina werd geweigerd als hij groot was.
YouTube propt ~665kB inline script voor zijn og:image en sluit <head> pas op
673kB, dus we knipten net voor de tags af en hielden niets over. Nu lezen we een
pagina vanaf het BEGIN en stoppen zodra we de tags hebben (of </head>), met 1MB
als achtervang. Een normale pagina kost daardoor nog steeds een paar tientallen
kB. Het scannen kijkt alleen naar het nieuwe stuk plus wat overlap, anders wordt
een pagina van 1MB kwadratisch werk.

Echt getest, niet aangenomen: youtube.com en youtu.be leveren nu titel +
thumbnail (oembed, ~2s), linuxguides ook, boiert.eu via opengraph in 144ms.

Changed files:
src/services/EmbedResolver.js

  • providerstap verwijderd uit de keten (AudioEmbedService blijft voor de web-spelers)
  • safeHead: streamt de kop, stopt bij og:image of </head>, cap 1MB
  • safeJsonText: JSON moet heel zijn, dus daar blijft weigeren juist

src/services/ActivityPubService.js

  • self-heal 16 -> 17, en nu ook rijen die eerder niets opleverden opnieuw proberen

test/embed-resolver.test.js

  • regressietest: een videohost is geen speciaal geval en krijgt een echte kaart
  • regressietest: een grote pagina wordt afgekapt, niet geweigerd

remarks: 214 tests groen.

-robo
Co-Authored-By: Claude Opus 4.8 <noreply@…>

Files:
3 edited

Legend:

Unmodified
Added
Removed

  • src/services/ActivityPubService.js

    r0101d0a rc52dc82  
    28492849// during a flux window, e.g. a fleet-wide update), and drops notes that are gone
    28502850// (404/410). Bump SELFHEAL_VERSION only on a release that warrants a re-sync.
    2851 const SELFHEAL_VERSION = 16; // v16: resolve external link previews (embed_json) for posts that predate the embed pipeline
     2851const SELFHEAL_VERSION = 17; // v17: re-resolve link previews now that OpenGraph works and big pages are read instead of refused
    28522852async function fetchNoteAP(url) {
    28532853  try {
    … …  
    31903190        // quote (a quote already IS the card) and no embed yet, so this costs
    31913191        // one page fetch per candidate and never repeats.
    3192         if (!r.quote_json && !r.embed_json) {
     3192        if (!r.quote_json) {
    31933193          const ej = await resolveExternalEmbed(html || r.content).catch(() => null);
    31943194          if (ej) { try { db.prepare('UPDATE ap_timeline SET embed_json = ? WHERE id = ?').run(ej, r.id); } catch { /* ignore */ } }

  • src/services/EmbedResolver.js

    r0101d0a rc52dc82  
    77//      author gets addressed, and the permission model applies. Never resolved
    88//      over oEmbed, because oEmbed has none of that.
    9 //   2. Known provider      → the existing player (YouTube/Spotify/Bandcamp/…).
    10 //   3. oEmbed discovery    → the generic path, and the preferred implementation
    11 //      for everything outside the fediverse.
    12 //   4. Otherwise           → a plain link.
     9//   2. oEmbed, else OpenGraph → one page fetch, and no list of providers to
     10//      maintain. Whether an embed may be shown at all is a guardian decision
     11//      (the gate), not a question of which host it came from.
     12//   3. Otherwise           → a plain link.
    1313//
    1414// Everything returns the SAME normalised shape, so one renderer draws them all
    … …  
    136136  }
    137137
    138   // 2. A provider we already play ourselves.
    139   if (io.provider) {
    140     const p = io.provider(url);
    141     if (p) return { kind: 'provider', url, provider: p.provider, id: p.id || null, media: [] };
    142   }
    143 
    144   // 3. oEmbed, then OpenGraph. One page fetch serves both: oEmbed is the richer
     138  // 2. oEmbed, then OpenGraph. One page fetch serves both: oEmbed is the richer
    145139  //    protocol, OpenGraph is the one most of the web actually ships.
     140  //
     141  //    There is deliberately NO list of known providers here. A hardcoded list
     142  //    is a whitelist you have to keep maintaining, and it was actively harmful:
     143  //    a YouTube link matched the list, short-circuited before oEmbed, and came
     144  //    out as a card with no title and no thumbnail, so nothing was stored at
     145  //    all. YouTube serves both oEmbed and og:image like everyone else, so the
     146  //    generic path handles it better than the special case did.
    146147  if (io.getPage) {
    147148    const page = await io.getPage(url).catch(() => null);
    … …  
    178179// so a hostile URL in a post cannot make the server probe an internal host.
    179180
    180 const MAX_BODY = 512_000;   // an oEmbed page/endpoint is small; refuse the rest
     181const MAX_JSON = 512_000;   // an oEmbed/AP payload must parse whole, so cap and refuse
     182// Of a web page we only ever need the <head>. The cap has to clear the worst
     183// real case rather than the tidy one: YouTube ships ~665kB of inline script
     184// before its og:image and closes <head> at ~673kB, and at 512kB we cut the page
     185// off just short of the tags and produced nothing. We stop as soon as the tags
     186// are in hand, so a normal page still costs a few dozen kB.
     187const MAX_HEAD = 1_048_576;
    181188const UA = 'Mozilla/5.0 (compatible; Klonkt/1.0; +https://klonkt.com)';
    182189
    183 async function safeText(safeFetch, url, accept, extra = {}) {
     190/** A whole small document, refused when it is too big to be one. */
     191async function safeJsonText(safeFetch, url, accept) {
    184192  try {
    185     const r = await safeFetch(url, { headers: { Accept: accept, ...extra } });
     193    const r = await safeFetch(url, { headers: { Accept: accept } });
    186194    if (!r.ok) return null;
    187     if (Number(r.headers.get('content-length') || 0) > MAX_BODY) return null;
     195    if (Number(r.headers.get('content-length') || 0) > MAX_JSON) return null;
    188196    const body = await r.text();
    189     return body.length > MAX_BODY ? body.slice(0, MAX_BODY) : body;
     197    return body.length > MAX_JSON ? null : body;   // truncated JSON is useless
     198  } catch { return null; }
     199}
     200
     201/**
     202 * The START of a web page, streamed and cut off at MAX_HEAD.
     203 *
     204 * Refusing a page for being large was wrong: YouTube's watch page is megabytes,
     205 * so it was rejected outright and never produced a thumbnail, even though its
     206 * og:image sits in the first few kilobytes like everyone else's. We only ever
     207 * read the <head>, so read that much and stop pulling. The cap still protects
     208 * us from someone streaming us an endless body.
     209 */
     210async function safeHead(safeFetch, url, extra = {}) {
     211  try {
     212    const r = await safeFetch(url, { headers: { Accept: 'text/html,application/xhtml+xml', ...extra } });
     213    if (!r.ok) return null;
     214    if (!r.body || typeof r.body.getReader !== 'function') {
     215      const body = await r.text();                       // no stream (or a test double)
     216      return body.length > MAX_HEAD ? body.slice(0, MAX_HEAD) : body;
     217    }
     218    const reader = r.body.getReader();
     219    const dec = new TextDecoder('utf-8');
     220    const parts = [];
     221    let len = 0;
     222    let tail = '';        // carry a little context so a tag split across chunks still matches
     223    let done_ = false;
     224    while (!done_) {
     225      const { done, value } = await reader.read();
     226      if (done) break;
     227      const chunk = dec.decode(value, { stream: true });
     228      parts.push(chunk);
     229      len += chunk.length;
     230      // Scan only the new chunk (plus overlap), not the whole buffer: testing
     231      // the full string every read turns a 1MB page into quadratic work.
     232      const window = tail + chunk;
     233      if (len >= MAX_HEAD || /<\/head>/i.test(window) || /og:image/i.test(window)) done_ = true;
     234      tail = chunk.slice(-512);
     235    }
     236    try { await reader.cancel(); } catch { /* already closed */ }
     237    return parts.join('').slice(0, MAX_HEAD);
    190238  } catch { return null; }
    191239}
    … …  
    199247    provider: detectProvider ? (u) => { try { return detectProvider(u); } catch { return null; } } : null,
    200248    getAP: async (u) => {
    201       const body = await safeText(safeFetch, u, 'application/activity+json, application/ld+json');
     249      const body = await safeJsonText(safeFetch, u, 'application/activity+json, application/ld+json');
    202250      if (!body) return null;
    203251      try { return JSON.parse(body); } catch { return null; }   // an HTML page is simply not AP
    … …  
    205253    // Plenty of sites only hand out their OpenGraph tags to something that
    206254    // looks like a browser, so the page fetch identifies itself.
    207     getPage: (u) => safeText(safeFetch, u, 'text/html', { 'User-Agent': UA }),
     255    getPage: (u) => safeHead(safeFetch, u, { 'User-Agent': UA }),
    208256    getJSON: async (u) => {
    209       const body = await safeText(safeFetch, u, 'application/json');
     257      const body = await safeJsonText(safeFetch, u, 'application/json');
    210258      if (!body) return null;
    211259      try { return JSON.parse(body); } catch { return null; }

  • test/embed-resolver.test.js

    r0101d0a rc52dc82  
    5252});
    5353
    54 test('a known provider beats oEmbed but loses to AP', async () => {
     54// Regression: a hardcoded provider list used to short-circuit here and return a
     55// card with no title and no thumbnail, so a YouTube link ended up storing
     56// nothing at all. There is no provider list any more; every non-fediverse URL
     57// takes the generic path, which is exactly what gives it a thumbnail.
     58test('a video host is not special-cased and still gets a real card', async () => {
    5559  const r = await resolveEmbed('https://youtu.be/abcdefghijk', io({
    56     provider: () => ({ provider: 'youtube', id: 'abcdefghijk' }),
     60    provider: () => ({ provider: 'youtube', id: 'abcdefghijk' }),   // ignored on purpose
    5761    getPage: async () => OEMBED_PAGE,
    5862    getJSON: async () => OEMBED_JSON,
    5963  }));
    60   assert.equal(r.kind, 'provider');
    61   assert.equal(r.provider, 'youtube');
     64  assert.equal(r.kind, 'oembed');
     65  assert.equal(r.title, 'A talk');
     66  assert.ok(r.media[0].url, 'and it has a thumbnail, which the old path never produced');
    6267});
    6368
    … …  
    115120  };
    116121  const io = liveIO({ safeFetch: fakeFetch, detectProvider: () => null });
    117   assert.equal(await io.getPage('https://x/huge'), null, 'oversized body refused');
     122  // A JSON payload must parse whole, so an oversized one is refused outright.
     123  assert.equal(await io.getJSON('https://x/huge'), null, 'oversized JSON refused');
    118124  assert.equal(await io.getAP('https://x/boom'), null, 'a refused fetch is not an error');
    119125  assert.deepEqual(await io.getAP('https://x/ok'), { type: 'Note', id: 'https://s/1' });
    … …  
    163169  assert.equal(r.url, 'https://lg.example/artikel');
    164170});
     171
     172// Regression, the one that kept YouTube blank: a page is read from the START and
     173// cut off, never refused for being large. Refusing it meant no thumbnail at all
     174// for exactly the sites people share most.
     175test('a huge page is truncated, not rejected', async () => {
     176  const { liveIO } = await import('../src/services/EmbedResolver.js');
     177  const big = '<html><head>' + 'x'.repeat(5000) + '<meta property="og:title" content="T">'
     178    + '<meta property="og:image" content="https://x/i.png"></head></html>';
     179  const fakeFetch = async () => ({ ok: true, headers: { get: () => String(9_000_000) }, text: async () => big });
     180  const io = liveIO({ safeFetch: fakeFetch, detectProvider: () => null });
     181  const page = await io.getPage('https://x/huge');
     182  assert.ok(page && page.includes('og:image'), 'the head survives the cap');
     183});
Note: See TracChangeset for help on using the changeset viewer.