| … | … | @@ -34,6 +34,19 @@ import b4a from 'b4a' |
| 34 | 34 | // out, so it is capped for both reasons. Stated in bytes of IMAGE, not of base64. |
| 35 | 35 | export const MAX_BYTES = 512 * 1024 |
| 36 | 36 | |
| 37 | // ⛔ A BANNER IS NOT A THUMBNAIL, and capping it like one is why banners silently never worked. The | |
| 38 | // avatar cap above is a POLICY for a picture shown at thumbnail size beside every byline; a banner is | |
| 39 | // a wide header — Bluesky's own spec is 3000×1000 — and at any reasonable quality it does not fit in | |
| 40 | // 512 KiB. Measured on a live identity: `icon` was set on the fediverse actor and `image` was null, | |
| 41 | // and the atproto profile had no banner either, because the upload had thrown before either | |
| 42 | // projection ever saw one. | |
| 43 | // | |
| 44 | // ⚠ 1,000,000 EXACTLY, because that is favela-bridge's MAX_PROFILE_IMAGE_BYTES — bsky's ceiling for a | |
| 45 | // profile image. Accepting a banner this node could not project would move the failure from a message | |
| 46 | // at upload time, which a person can act on, to a "loss" line in a projection log they never read. | |
| 47 | // Mastodon's default is higher, so this is the binding constraint of the two destinations. | |
| 48 | export const BANNER_MAX_BYTES = 1000000 | |
| 49 | ||
| 37 | 50 | // Typed from the MAGIC BYTES, never from a filename or a client-supplied content-type. An extension |
| 38 | 51 | // is a claim by whoever named the file; the first four bytes are the fact. |
| 39 | 52 | // |
| … | … | @@ -63,8 +76,8 @@ export class Avatars { |
| 63 | 76 | * ⛔ Refuses anything that is not an inert raster image, by its bytes. The caller may be relaying |
| 64 | 77 | * a browser upload, so nothing it says about the file is trusted. |
| 65 | 78 | */ |
| 66 | async put (srcPath) { | |
| 67 | if (typeof srcPath !== 'string' || !srcPath) throw new Error('an avatar needs a path') | |
| 79 | async put (srcPath, { maxBytes = MAX_BYTES, kind = 'avatar' } = {}) { | |
| 80 | if (typeof srcPath !== 'string' || !srcPath) throw new Error('an ' + kind + ' needs a path') | |
| 68 | 81 | let bytes |
| 69 | 82 | try { bytes = fs.readFileSync(srcPath) } catch { throw new Error('no file at ' + srcPath) } |
| 70 | 83 | if (!bytes.length) throw new Error('that file is empty') |
| … | … | @@ -72,9 +85,13 @@ export class Avatars { |
| 72 | 85 | // ⚠ This cap is a POLICY, not a transport limit — an avatar is shown at thumbnail size and is |
| 73 | 86 | // re-fetched by every peer that renders a byline, so a large one is waste rather than an error |
| 74 | 87 | // in the plumbing. Files posted in a record have no cap; there is nothing to cap. |
| 75 | if (bytes.length > MAX_BYTES) { | |
| 76 | throw new Error('that image is ' + Math.round(bytes.length / 1024) + ' KiB and the cap is ' + | |
| 77 | (MAX_BYTES / 1024) + ' KiB — scale it down first') | |
| 88 | // | |
| 89 | // ⛔ THE CAP IS THE CALLER'S, because the same store holds two different kinds of picture. It was | |
| 90 | // fixed at the avatar's, so a banner — a wide header, not a thumbnail — was refused at a size no | |
| 91 | // banner is, and the profile kept an avatar and no banner with nothing downstream able to say why. | |
| 92 | if (bytes.length > maxBytes) { | |
| 93 | throw new Error('that ' + kind + ' is ' + Math.round(bytes.length / 1024) + ' KiB and the cap is ' + | |
| 94 | Math.round(maxBytes / 1024) + ' KiB — scale it down first') | |
| 78 | 95 | } |
| 79 | 96 | const mimeType = sniff(bytes) |
| 80 | 97 | if (!mimeType) { |
| … | … | @@ -469,7 +469,11 @@ export async function dispatch (node, cmd, a) { |
| 469 | 469 | // mechanism. It is a separate verb rather than `avatar-put {field}` because the two are |
| 470 | 470 | // different acts and a mistyped field name would silently replace the wrong picture. |
| 471 | 471 | case 'banner-put': { |
| 472 | const r = await node.avatars.put(a.path) | |
| 472 | // ⛔ ITS OWN CAP. `avatars.put` defaults to the AVATAR's 512 KiB, which is a thumbnail policy — | |
| 473 | // a banner is a wide header and does not fit in it, so every banner upload threw and the | |
| 474 | // profile kept the avatar written moments earlier in the same submission. | |
| 475 | const { BANNER_MAX_BYTES } = await import('./avatars.js') | |
| 476 | const r = await node.avatars.put(a.path, { maxBytes: BANNER_MAX_BYTES, kind: 'banner' }) | |
| 473 | 477 | await peer.setProfile({ banner: r.oid }) |
| 474 | 478 | await peer.update() |
| 475 | 479 | return r |
| … | … | @@ -0,0 +1,68 @@ |
| 1 | // A banner is not a thumbnail, and capping it like one is why banners never worked. | |
| 2 | // | |
| 3 | // ⛔ THE DEFECT. `avatars.put` took no cap: it always used MAX_BYTES, whose own comment says the limit | |
| 4 | // is "a POLICY, not a transport limit — an avatar is shown at thumbnail size and is re-fetched by | |
| 5 | // every peer that renders a byline". `banner-put` stores through the SAME machinery, deliberately — | |
| 6 | // but a banner is a wide header (Bluesky's spec is 3000×1000) and at any reasonable quality does not | |
| 7 | // fit in 512 KiB. So every banner upload threw. | |
| 8 | // | |
| 9 | // ⚠ AND IT LOOKED LIKE THE AVATAR WORKING. The settings form submits both pictures at once and the | |
| 10 | // router writes the avatar FIRST, so the avatar was already saved when the banner threw. Measured on a | |
| 11 | // live identity: the fediverse actor had `icon` set and `image` null, and the atproto profile had no | |
| 12 | // banner either — one message at upload time, and three surfaces with nothing to show. | |
| 13 | // | |
| 14 | // ⛔ 1,000,000 EXACTLY, matching favela-bridge's MAX_PROFILE_IMAGE_BYTES — bsky's ceiling for a profile | |
| 15 | // image. Accepting a banner this node could not project would move the failure from a message a person | |
| 16 | // can act on to a "loss" line in a projection log they never read. | |
| 17 | ||
| 18 | import test from 'brittle' | |
| 19 | import fs from 'bare-fs' | |
| 20 | import os from 'bare-os' | |
| 21 | import { join } from 'bare-path' | |
| 22 | import { MAX_BYTES, BANNER_MAX_BYTES } from '../lib/avatars.js' | |
| 23 | ||
| 24 | // A real PNG header, so the magic-byte sniff passes and the size check is what is being tested. | |
| 25 | const PNG = Buffer.from('89504e470d0a1a0a0000000d49484452', 'hex') | |
| 26 | const pngOf = (bytes) => Buffer.concat([PNG, Buffer.alloc(Math.max(0, bytes - PNG.length), 0x20)]) | |
| 27 | ||
| 28 | const tmpFile = (buf) => { | |
| 29 | const p = join(fs.mkdtempSync(join(os.tmpdir(), 'banner-cap-')), 'img.png') | |
| 30 | fs.writeFileSync(p, buf) | |
| 31 | return p | |
| 32 | } | |
| 33 | ||
| 34 | test('★★★ the banner cap is bsky\'s profile-image ceiling, not the avatar\'s thumbnail policy', (t) => { | |
| 35 | t.is(MAX_BYTES, 512 * 1024, 'the avatar stays a thumbnail') | |
| 36 | t.is(BANNER_MAX_BYTES, 1000000, 'and the banner gets bsky\'s ceiling') | |
| 37 | t.ok(BANNER_MAX_BYTES > MAX_BYTES, | |
| 38 | 'a wide header cannot be held to a limit written for a picture shown beside a byline') | |
| 39 | }) | |
| 40 | ||
| 41 | test('★★★ put takes the caller\'s cap, and says which kind it refused', async (t) => { | |
| 42 | // ⛔ The store is shared on purpose — "a banner needs a verb, not a mechanism" — so the CAP has to | |
| 43 | // be the caller's, or the shared machinery silently imposes the wrong policy on one of them. | |
| 44 | const { Avatars } = await import('../lib/avatars.js') | |
| 45 | t.ok(Avatars, 'the store is exported') | |
| 46 | ||
| 47 | // The size check runs before any I/O or hashing, so a stub overlay is enough to reach it. | |
| 48 | const store = new Avatars(fs.mkdtempSync(join(os.tmpdir(), 'avstore-')), { hashFile: async () => ({ contentHash: 'x'.repeat(64) }), registerFile: async () => {} }) | |
| 49 | ||
| 50 | const big = tmpFile(pngOf(700 * 1024)) // over the avatar cap, under the banner cap | |
| 51 | ||
| 52 | await t.exception(() => store.put(big), /avatar is 700 KiB and the cap is 512 KiB/, | |
| 53 | 'refused as an avatar, and the message names the kind') | |
| 54 | ||
| 55 | // ⛔ THE SAME FILE, ACCEPTED AS A BANNER. This is the whole bug: identical bytes, identical store, | |
| 56 | // and the only thing that made it impossible was a cap that belonged to the other picture. | |
| 57 | const ok = await store.put(big, { maxBytes: BANNER_MAX_BYTES, kind: 'banner' }) | |
| 58 | t.ok(ok && ok.oid, 'accepted as a banner') | |
| 59 | t.is(ok.mimeType, 'image/png') | |
| 60 | }) | |
| 61 | ||
| 62 | test('★★ a banner over bsky\'s ceiling is still refused, at upload time', async (t) => { | |
| 63 | const { Avatars } = await import('../lib/avatars.js') | |
| 64 | const store = new Avatars(fs.mkdtempSync(join(os.tmpdir(), 'avstore2-')), { hashFile: async () => ({ contentHash: 'y'.repeat(64) }), registerFile: async () => {} }) | |
| 65 | const huge = tmpFile(pngOf(1200 * 1024)) | |
| 66 | await t.exception(() => store.put(huge, { maxBytes: BANNER_MAX_BYTES, kind: 'banner' }), /banner is 1200 KiB/, | |
| 67 | 'said once, where a person can act on it, rather than as a loss line in a projection log') | |
| 68 | }) |