diff --git a/apps/build-docs/src/build-docs.ts b/apps/build-docs/src/build-docs.ts index 8cd8800e820..1305eb92546 100644 --- a/apps/build-docs/src/build-docs.ts +++ b/apps/build-docs/src/build-docs.ts @@ -44,7 +44,8 @@ const stubImageProvider: ImageProvider = { }, compressImage: async () => ({ compressed: false, reason: "unsupported-platform" }), planCompression: async () => ({ skip: "unsupported-platform" as const, decodeCost: null }), - compressionConcurrency: () => 1 + compressionConcurrency: () => 1, + resizeForPreview: async () => ({ resized: false, reason: "unsupported-platform" as const }) }; import { ZipArchive } from "archiver"; import { execSync } from "child_process"; diff --git a/apps/server/src/routes/routes.ts b/apps/server/src/routes/routes.ts index 0db575877a2..5975a81f35c 100644 --- a/apps/server/src/routes/routes.ts +++ b/apps/server/src/routes/routes.ts @@ -24,7 +24,6 @@ import etapiTokensApiRoutes from "./api/etapi_tokens.js"; import filesRoute from "./api/files.js"; import fontsRoute from "./api/fonts.js"; // API routes -import linkEmbedRoute from "./api/link_embed.js"; import llmChatRoute from "./api/llm_chat.js"; import llmSpecialNotesRoute from "./api/llm_special_notes.js"; import loginApiRoute from "./api/login.js"; @@ -201,7 +200,6 @@ function register(app: express.Application) { // POST rather than GET: the URL would otherwise sit in the query string of every access-log // line (Trilium's own, and any reverse proxy in front of it), and a pasted URL can carry a // one-time token or a signed signature. The body is not logged. - asyncApiRoute(PST, "/api/link-embed/metadata", linkEmbedRoute.getMetadata); asyncApiRoute(PST, "/api/onenote-import/device-login", onenoteImportRoute.deviceLogin); asyncApiRoute(PST, "/api/onenote-import/device-poll", onenoteImportRoute.devicePoll); diff --git a/apps/server/src/services/image_codec.ts b/apps/server/src/services/image_codec.ts index c825b8678dc..63c74895063 100644 --- a/apps/server/src/services/image_codec.ts +++ b/apps/server/src/services/image_codec.ts @@ -12,7 +12,7 @@ import { IMAGE_COMPRESSIBLE_FORMATS, type ImageCompressionSkipReason } from "@triliumnext/commons"; import { type InspectedImage, inspectImage } from "@triliumnext/core/src/services/image_inspect.js"; -import type { ImageCompressionOutcome, ImageCompressionRequest, ImageFormat } from "@triliumnext/core/src/services/image_provider.js"; +import type { ImageCompressionOutcome, ImageCompressionRequest, ImageFormat, PreviewResizeOutcome, PreviewResizeRequest } from "@triliumnext/core/src/services/image_provider.js"; import { estimateJpegQuality } from "@triliumnext/core/src/services/jpeg_quality.js"; import imageType from "image-type"; import isAnimated from "is-animated"; @@ -442,3 +442,49 @@ export async function compressImageBytes( return { compressed: true, buffer: result, format: toJpeg ? JPEG_FORMAT : PNG_FORMAT }; } +/** + * Scales a link preview's cover image down to `maxEdge` and re-encodes it. + * + * The bytes make a round trip — downloaded and stored in the same breath — so the reduction happens + * here rather than being left to the generic compression pass: a 5MB `og:image` has no business + * being carried through a pipeline sized for the user's own photographs to become a thumbnail + * nobody will see above a couple of hundred pixels. + * + * Transparency survives by re-encoding to PNG only where the picture actually has non-opaque pixels; + * an opaque one becomes a JPEG, which is several times smaller. + * + * Answers `undecodable` rather than throwing when Jimp cannot read the bytes — it bundles decoders + * for PNG/JPEG/GIF/BMP/TIFF only, so a WebP or an AVIF lands there, as does an error page served + * where a picture should have been. Saying so is enough; what a preview does without its picture is + * not this function's business. + */ +export async function resizePreviewImage( + bytes: Uint8Array, + { maxEdge, jpegQuality }: PreviewResizeRequest, + log: CodecLog = () => {} +): Promise { + try { + const image = await decodeImage(bytes); + + // Only ever down: scaleToFit() would happily enlarge a smaller picture. + if (image.bitmap.width > maxEdge || image.bitmap.height > maxEdge) { + image.scaleToFit({ w: maxEdge, h: maxEdge }); + } + + // hasAlpha() inspects the pixels rather than just the channel, so an opaque PNG still takes + // the JPEG path. An animated GIF or WebP collapses to its first frame, which is all a + // thumbnail wanted of it. + const encoded = image.hasAlpha() + ? await image.getBuffer("image/png") + : await image.getBuffer("image/jpeg", { quality: jpegQuality }); + + return { resized: true, bytes: new Uint8Array(encoded) }; + } catch (e: unknown) { + // The address is deliberately left out of the line: it is the user's private browsing, and a + // pasted link can carry a one-time token in its path or query. + log(`Could not decode a link preview image: ${e}`, true); + + return { resized: false, reason: "undecodable" }; + } +} + diff --git a/apps/server/src/services/image_codec_preview.spec.ts b/apps/server/src/services/image_codec_preview.spec.ts new file mode 100644 index 00000000000..1f31a953b23 --- /dev/null +++ b/apps/server/src/services/image_codec_preview.spec.ts @@ -0,0 +1,75 @@ +import { Jimp } from "jimp"; +import { describe, expect, it } from "vitest"; + +import { resizePreviewImage } from "./image_codec.js"; + +const REQUEST = { maxEdge: 256, jpegQuality: 75 }; + +async function png(width: number, height: number, color = 0xff0000ff) { + return new Uint8Array(await new Jimp({ width, height, color }).getBuffer("image/png")); +} + +async function decoded(bytes: Uint8Array) { + return await Jimp.fromBuffer(Buffer.from(bytes)); +} + +describe("resizePreviewImage", () => { + it("scales a large picture down to the longest edge asked for, keeping its proportions", async () => { + const result = await resizePreviewImage(await png(1000, 500), REQUEST); + + expect(result.resized).toBe(true); + if (!result.resized) return; + + const image = await decoded(result.bytes); + expect(image.bitmap.width).toBe(256); + expect(image.bitmap.height).toBe(128); + }); + + it("leaves a picture already smaller than the ceiling at the size it came", async () => { + const result = await resizePreviewImage(await png(64, 48), REQUEST); + + expect(result.resized).toBe(true); + if (!result.resized) return; + + const image = await decoded(result.bytes); + expect(image.bitmap.width).toBe(64); + expect(image.bitmap.height).toBe(48); + }); + + /** + * The choice of encoding is by what the pixels actually are, not by what the format allows: a + * PNG with nothing transparent in it is several times smaller as a JPEG, and a card thumbnail + * is not where that saving should be passed up. + */ + it("re-encodes an opaque picture to JPEG and a transparent one to PNG", async () => { + const opaque = await resizePreviewImage(await png(300, 300, 0x336699ff), REQUEST); + const transparent = await resizePreviewImage(await png(300, 300, 0x33669900), REQUEST); + + expect(opaque.resized && (await decoded(opaque.bytes)).mime).toBe("image/jpeg"); + expect(transparent.resized && (await decoded(transparent.bytes)).mime).toBe("image/png"); + }); + + it("says so rather than throwing when the bytes are not a picture it can read", async () => { + // An error page served where a picture should have been — the case this has to survive. + const html = new TextEncoder().encode("404"); + + await expect(resizePreviewImage(html, REQUEST)).resolves.toEqual({ + resized: false, + reason: "undecodable" + }); + }); + + it("passes the reason to the log it was given, and manages without one", async () => { + const lines: string[] = []; + await resizePreviewImage(new Uint8Array([ 1, 2, 3 ]), REQUEST, (message) => lines.push(message)); + + expect(lines).toHaveLength(1); + expect(lines[0]).toMatch(/Could not decode a link preview image/); + // The address never appears: a pasted link can carry a one-time token in its query. + expect(lines[0]).not.toMatch(/http/); + + await expect(resizePreviewImage(new Uint8Array([ 1, 2, 3 ]), REQUEST)).resolves.toMatchObject({ + resized: false + }); + }); +}); diff --git a/apps/server/src/services/image_compression.spec.ts b/apps/server/src/services/image_compression.spec.ts index 8c64f8d8adb..6192d51fec3 100644 --- a/apps/server/src/services/image_compression.spec.ts +++ b/apps/server/src/services/image_compression.spec.ts @@ -533,10 +533,7 @@ describe("compression parameters", () => { // The header pass is an optimization of the read, never a precondition for it — so when it // cannot answer, the run must go the long way round rather than report a failure. initImageProvider({ - getImageType: (buffer) => real.getImageType(buffer), - processImage: (buffer, name, shrink) => real.processImage(buffer, name, shrink), - compressImage: (buffer, compressionRequest) => real.compressImage(buffer, compressionRequest), - compressionConcurrency: () => real.compressionConcurrency(), + ...real, planCompression: () => Promise.reject(new Error("planner unavailable")) }); @@ -567,9 +564,7 @@ describe("compression parameters", () => { let compressedSoFar = 0; initImageProvider({ - getImageType: (buffer) => real.getImageType(buffer), - processImage: (buffer, name, shrink) => real.processImage(buffer, name, shrink), - planCompression: (header, req) => real.planCompression(header, req), + ...real, compressionConcurrency: () => 1, compressImage: async (buffer, req) => { const outcome = await real.compressImage(buffer, req); @@ -898,10 +893,7 @@ async function whileCompressing(interfere: () => void, request: () => Promise const real = getImageProvider(); initImageProvider({ - getImageType: (buffer) => real.getImageType(buffer), - processImage: (buffer, originalName, shrink) => real.processImage(buffer, originalName, shrink), - planCompression: (header, compressionRequest) => real.planCompression(header, compressionRequest), - compressionConcurrency: () => real.compressionConcurrency(), + ...real, compressImage: (buffer, compressionRequest) => { interfere(); return real.compressImage(buffer, compressionRequest); diff --git a/apps/server/src/services/image_provider.ts b/apps/server/src/services/image_provider.ts index c9902ff39c3..fe6d3993b0c 100644 --- a/apps/server/src/services/image_provider.ts +++ b/apps/server/src/services/image_provider.ts @@ -7,7 +7,7 @@ */ import { getLog, imageCompressionService, options as optionService } from "@triliumnext/core"; -import type { ImageCompressionOutcome, ImageCompressionPlan, ImageCompressionRequest, ImageFormat, ImageProvider, ProcessedImage } from "@triliumnext/core/src/services/image_provider.js"; +import type { ImageCompressionOutcome, ImageCompressionPlan, ImageCompressionRequest, ImageFormat, ImageProvider, PreviewResizeOutcome, PreviewResizeRequest, ProcessedImage } from "@triliumnext/core/src/services/image_provider.js"; import { createConcurrencyGate } from "./concurrency_gate.js"; import { @@ -16,7 +16,8 @@ import { decodeCostOf, detectSvg, getImageTypeFromBuffer, - planFromBytes + planFromBytes, + resizePreviewImage } from "./image_codec.js"; import { compressInWorker, compressionConcurrency } from "./image_worker_pool.js"; @@ -124,5 +125,9 @@ export const serverImageProvider: ImageProvider = { return offThread ?? compressImageBytes(buffer, request, toBackendLog); }, - compressionConcurrency + compressionConcurrency, + + async resizeForPreview(bytes: Uint8Array, request: PreviewResizeRequest): Promise { + return await resizePreviewImage(bytes, request, toBackendLog); + } }; diff --git a/apps/server/src/services/request.ts b/apps/server/src/services/request.ts index 84a75b55f2d..c24e332391b 100644 --- a/apps/server/src/services/request.ts +++ b/apps/server/src/services/request.ts @@ -1,9 +1,10 @@ "use strict"; -import { type CookieJar, type ExecOpts, getLog, type RequestProvider, sync_options as syncOptions } from "@triliumnext/core"; +import { type CookieJar, type ExecOpts, type FetchedResource, type FetchResourceOpts, getLog, type RequestProvider, sync_options as syncOptions } from "@triliumnext/core"; +import { readCappedResponse } from "@triliumnext/core/src/services/request.js"; import url from "url"; -import { createPinnedLookup, validateHostResolution, validateUrl } from "./safe_fetch.js"; +import { createPinnedLookup, safeFetch, validateHostResolution, validateUrl } from "./safe_fetch.js"; // this service provides abstraction over node's HTTP/HTTPS modules. // Subclasses (e.g. apps/desktop's ElectronRequestProvider) can override @@ -193,6 +194,20 @@ export default class NodeRequestProvider implements RequestProvider { }); } + /** + * Fetches a third-party resource, hardened the same way {@link getImage} is: the address is + * vetted, the name resolved, the private ranges refused, and the connection pinned to the + * addresses that were actually checked, so a second lookup cannot answer differently. + * + * This is the runtime where all of that is possible, and where it is most needed — the network + * a server can see is not one the author of a note is entitled to reach through it. + */ + async fetchResource(resourceUrl: string, opts: FetchResourceOpts): Promise { + const response = await safeFetch(resourceUrl, { headers: opts.headers }); + + return await readCappedResponse(response, opts.maxBytes); + } + /** * Fetches an image named by note content. * diff --git a/apps/server/src/services/safe_fetch.ts b/apps/server/src/services/safe_fetch.ts index cc72dd607bd..736a160cfbf 100644 --- a/apps/server/src/services/safe_fetch.ts +++ b/apps/server/src/services/safe_fetch.ts @@ -2,6 +2,7 @@ import dns from "node:dns"; import net from "node:net"; import { ValidationError } from "@triliumnext/core"; +import { validateFetchableUrl } from "@triliumnext/core/src/services/request.js"; import ipaddr from "ipaddr.js"; import { Agent, fetch as undiciFetch, type RequestInit as UndiciRequestInit, type Response as UndiciResponse } from "undici"; @@ -61,27 +62,11 @@ async function validateHostResolution(hostname: string): Promise { await expect(promise).rejects.toThrow("404 GET"); }); }); + +describe("BridgedRequestProvider.fetchResource", () => { + it("asks the native side for the bytes and reports what they came under", async () => { + const provider = new BridgedRequestProvider(); + const promise = provider.fetchResource("https://example.com/p", { + maxBytes: 1000, + headers: { "user-agent": "TriliumNotes" } + }); + + expect(lastPosted().request).toMatchObject({ + method: "GET", + url: "https://example.com/p", + headers: { "user-agent": "TriliumNotes" }, + responseType: "arraybuffer" + }); + + respond({ status: 200, headers: { "content-type": "text/html; charset=utf-8" }, body: btoa("") }); + + await expect(promise).resolves.toEqual({ + status: 200, + ok: true, + contentType: "text/html", + bytes: new TextEncoder().encode("") + }); + }); + + it("answers a non-2xx rather than throwing, the page having said something", async () => { + const provider = new BridgedRequestProvider(); + const promise = provider.fetchResource("https://example.com/gone", { maxBytes: 1000 }); + + respond({ status: 404, headers: {}, body: btoa("nope") }); + + await expect(promise).resolves.toMatchObject({ status: 404, ok: false, contentType: "" }); + }); + + it("refuses an address before anything is posted for it", async () => { + const provider = new BridgedRequestProvider(); + const posted = postSpy.mock.calls.length; + + await expect(provider.fetchResource("file:///etc/passwd", { maxBytes: 10 })).rejects.toThrow(/http and https/); + expect(postSpy.mock.calls.length).toBe(posted); + }); + + /** + * The ceiling is checked on what arrived rather than on what is arriving — a native transport + * hands over a whole response, so there is no stream to abandon. It still has to hold. + */ + it("refuses a body over the ceiling once it is back", async () => { + const provider = new BridgedRequestProvider(); + const promise = provider.fetchResource("https://example.com/big", { maxBytes: 10 }); + + respond({ status: 200, headers: {}, body: btoa("x".repeat(50)) }); + + await expect(promise).rejects.toThrow(/exceeds the 10 byte limit/); + }); + + /** + * And refuses it *before* decoding, which is the part that matters. Decoding is where an + * oversized body multiplies — atob builds a binary string and Uint8Array.from copies that again + * — so a check running afterwards has already paid for what it is about to refuse. + */ + it("refuses an oversized body without decoding it", async () => { + const decode = vi.spyOn(globalThis, "atob"); + const provider = new BridgedRequestProvider(); + const promise = provider.fetchResource("https://example.com/big", { maxBytes: 10 }); + + respond({ status: 200, headers: {}, body: btoa("x".repeat(5000)) }); + + await expect(promise).rejects.toThrow(/exceeds the 10 byte limit/); + expect(decode).not.toHaveBeenCalled(); + }); + + it("measures the decoded size rather than the encoded one, at every padding", async () => { + // Base64 runs 4/3 the size of what it carries, so measuring the encoded string would refuse + // a 30-byte body against a 30-byte ceiling. The three lengths cover the three paddings. + for (const length of [ 30, 29, 28 ]) { + const provider = new BridgedRequestProvider(); + const promise = provider.fetchResource("https://example.com/exact", { maxBytes: 30 }); + + respond({ status: 200, headers: {}, body: btoa("x".repeat(length)) }); + + await expect(promise).resolves.toMatchObject({ + bytes: new Uint8Array(length).fill("x".charCodeAt(0)) + }); + } + }); +}); diff --git a/apps/standalone/src/lightweight/bridged_request_provider.ts b/apps/standalone/src/lightweight/bridged_request_provider.ts index aee83d66d40..167aac54c68 100644 --- a/apps/standalone/src/lightweight/bridged_request_provider.ts +++ b/apps/standalone/src/lightweight/bridged_request_provider.ts @@ -1,4 +1,5 @@ -import type { ExecOpts, RequestProvider } from "@triliumnext/core"; +import type { ExecOpts, FetchedResource, FetchResourceOpts, RequestProvider } from "@triliumnext/core"; +import { validateFetchableUrl } from "@triliumnext/core/src/services/request.js"; /** * A RequestProvider that delegates HTTP requests to the main thread via postMessage. @@ -151,4 +152,88 @@ export default class BridgedRequestProvider implements RequestProvider { const bytes = Uint8Array.from(binary, (c) => c.charCodeAt(0)); return bytes.buffer; } + + /** + * Fetches a third-party resource over the native transport, which is the whole reason this + * provider exists: the cross-origin hop happens outside the WebView, so a page that sends no + * CORS headers — which is nearly every page worth previewing — can still be read. + * + * Two things the server's implementation has are missing here, and neither can be had in full: + * + * - The body arrives whole, so there is no stream to abandon partway. `maxBytes` is checked + * against the encoded length the moment the reply lands and before anything is decoded from + * it, which is the earliest point this side of the bridge can see a size at all — see + * {@link decodedLengthOf}. What that spares is the decoding, which is where the cost + * multiplies: `atob` yields a binary string and `Uint8Array.from` yields a copy of that, so a + * body checked afterwards has already been held three times over. + * It does not spare what the native side and the bridge already spent to deliver it. Bounding + * *that* means giving the ceiling to the transport, which the plugin cannot honour — it + * answers only once the whole response is in hand. The Android streaming proxy could, and + * binding these two together is the fix worth making; it is not this change. + * - Nothing resolves the hostname, so the private-address check cannot be made and DNS + * rebinding has no meaning here anyway. What is left is {@link validateFetchableUrl}, and + * the reason that is enough: this transport runs on the user's own device, reaching the + * network that user is already on, at the address that user just pasted. The server's rule — + * that note content is not entitled to the network its host can see — is about a host reached + * by people who are not its owner, which is not this. + */ + async fetchResource(resourceUrl: string, opts: FetchResourceOpts): Promise { + const validated = validateFetchableUrl(resourceUrl).toString(); + const id = String(this.nextId++); + + const msg = await new Promise((resolve, reject) => { + this.pending.set(id, { resolve, reject }); + + (self as unknown as Worker).postMessage({ + type: "HTTP_REQUEST", + id, + request: { + method: "GET", + url: validated, + headers: opts.headers ?? {}, + responseType: "arraybuffer" + } + }); + }); + + const encoded = msg.body ?? ""; + + if (decodedLengthOf(encoded) > opts.maxBytes) { + throw new Error(`Response exceeds the ${opts.maxBytes} byte limit`); + } + + const binary = atob(encoded); + + return { + status: msg.status, + ok: msg.status >= 200 && msg.status < 300, + contentType: (msg.headers?.["content-type"] ?? "").split(";")[0].trim().toLowerCase(), + bytes: Uint8Array.from(binary, (c) => c.charCodeAt(0)) + }; + } +} + +/** + * How many bytes a base64 string will decode to, counted without decoding it. + * + * Four characters carry three bytes, less whatever the trailing `=` padding stands in for. Exact + * for well-formed input, which is what the bridge produces; a malformed body only ever makes this + * an over-estimate, and over-estimating is the safe direction for a ceiling. + */ +function decodedLengthOf(base64: string): number { + if (!base64) { + return 0; + } + + const padding = base64.endsWith("==") ? 2 : base64.endsWith("=") ? 1 : 0; + + return Math.floor(base64.length / 4) * 3 - padding; +} + +/** What the main thread answers an HTTP_REQUEST with, for the binary shape of the exchange. */ +interface BridgedResponse { + status: number; + headers?: Record; + /** Base64 for a binary response; the raw text otherwise. */ + body?: string; } diff --git a/apps/standalone/src/lightweight/request_provider.spec.ts b/apps/standalone/src/lightweight/request_provider.spec.ts index 3cfa07badc1..f9a2fa60622 100644 --- a/apps/standalone/src/lightweight/request_provider.spec.ts +++ b/apps/standalone/src/lightweight/request_provider.spec.ts @@ -150,3 +150,51 @@ describe("FetchRequestProvider.getImage", () => { await expect(provider.getImage(`${location.origin}/missing.png`)).rejects.toThrow("404 GET"); }); }); + +describe("FetchRequestProvider.fetchResource", () => { + it("reads a capped body and the type it came under", async () => { + const spy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response("", { headers: { "content-type": "text/html; charset=utf-8" } }) + ); + + const read = await provider.fetchResource("https://example.com/p", { + maxBytes: 1000, + headers: { "user-agent": "TriliumNotes" } + }); + + expect(read).toEqual({ + status: 200, + ok: true, + contentType: "text/html", + bytes: new TextEncoder().encode("") + }); + expect(spy.mock.calls[0][1]).toMatchObject({ headers: { "user-agent": "TriliumNotes" } }); + }); + + it("refuses an address before a request is made of it", async () => { + const spy = vi.spyOn(globalThis, "fetch"); + + await expect(provider.fetchResource("file:///etc/passwd", { maxBytes: 10 })).rejects.toThrow(/http and https/); + await expect(provider.fetchResource("https://u:p@example.com/", { maxBytes: 10 })).rejects.toThrow(/credentials/); + expect(spy).not.toHaveBeenCalled(); + }); + + it("refuses a body over the ceiling", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("x".repeat(50))); + + await expect(provider.fetchResource("https://example.com/big", { maxBytes: 10 })) + .rejects.toThrow(/exceeds the 10 byte limit/); + }); + + /** + * The case with no server behind it: a site that sends no CORS headers fails the fetch itself, + * however healthy the response was. Nothing here can fix that — what matters is that it surfaces + * as a rejection the caller can degrade on, rather than as an empty preview. + */ + it("propagates the failure a cross-origin refusal shows up as", async () => { + vi.spyOn(globalThis, "fetch").mockRejectedValue(new TypeError("Failed to fetch")); + + await expect(provider.fetchResource("https://example.com/p", { maxBytes: 10 })) + .rejects.toThrow(/Failed to fetch/); + }); +}); diff --git a/apps/standalone/src/lightweight/request_provider.ts b/apps/standalone/src/lightweight/request_provider.ts index cdaa3a34e98..d392d9a7ddc 100644 --- a/apps/standalone/src/lightweight/request_provider.ts +++ b/apps/standalone/src/lightweight/request_provider.ts @@ -1,4 +1,5 @@ -import type { ExecOpts, RequestProvider } from "@triliumnext/core"; +import type { ExecOpts, FetchedResource, FetchResourceOpts, RequestProvider } from "@triliumnext/core"; +import { readCappedResponse, validateFetchableUrl } from "@triliumnext/core/src/services/request.js"; /** * Fetch-based implementation of RequestProvider for browser environments. @@ -90,4 +91,41 @@ export default class FetchRequestProvider implements RequestProvider { return await response.arrayBuffer(); } + + /** + * Fetches a third-party resource with the page's own `fetch`, which means the same-origin + * policy decides whether the answer can be read at all. + * + * A great many sites do not send `Access-Control-Allow-Origin`, and for those this throws + * however healthy the response was — the request goes out, the browser refuses to hand back + * what came of it. That is not a fault to work around here: it is what running with no server + * costs, and the caller is expected to degrade rather than to retry. + * + * Notably it is not universal. Static and documentation hosting frequently sends `*`, and the + * oEmbed endpoints of the large video and audio providers all do, so a good deal is readable + * this way. Where it is not, the native transport in the sibling provider is the way through. + * + * `credentials` stays at its default of `same-origin`: nothing here should carry the user's + * cookies to a third party just because a link to it was pasted into a note. + */ + async fetchResource(resourceUrl: string, opts: FetchResourceOpts): Promise { + const validated = validateFetchableUrl(resourceUrl).toString(); + + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), FETCH_RESOURCE_TIMEOUT_MS); + + try { + const response = await fetch(validated, { + headers: opts.headers, + signal: controller.signal + }); + + return await readCappedResponse(response, opts.maxBytes); + } finally { + clearTimeout(timeoutId); + } + } } + +/** Matches the server's own fetch timeout, a preview not being worth waiting on for longer. */ +const FETCH_RESOURCE_TIMEOUT_MS = 5000; diff --git a/apps/standalone/src/services/image_provider.ts b/apps/standalone/src/services/image_provider.ts index 3e6e920ddff..9703ad02295 100644 --- a/apps/standalone/src/services/image_provider.ts +++ b/apps/standalone/src/services/image_provider.ts @@ -4,7 +4,7 @@ * Images are saved as-is without resizing. */ -import type { ImageCompressionOutcome, ImageCompressionPlan, ImageFormat, ImageProvider, ProcessedImage } from "@triliumnext/core"; +import type { ImageCompressionOutcome, ImageCompressionPlan, ImageFormat, ImageProvider, PreviewResizeOutcome, ProcessedImage } from "@triliumnext/core"; import { inspectImage, UNKNOWN_FORMAT } from "@triliumnext/core/src/services/image_inspect.js"; /** @@ -47,5 +47,18 @@ export const standaloneImageProvider: ImageProvider = { // Settled before an image is so much as weighed: nothing here is going to compress it // whatever its header says, so a run over a tree reads none of them. return { skip: "unsupported-platform", decodeCost: null }; + }, + + /** + * Declined for the same reason as everything else on this runtime: there is no decoder to scale + * a picture with. + * + * The preview is not lost to it. The caller keeps a cover image that is already small enough to + * store as it came, and shows a preview without a picture where it is not — which is exactly + * what the server does for a WebP its own decoder cannot read, so this is a path already + * travelled rather than a new kind of degradation. + */ + async resizeForPreview(): Promise { + return { resized: false, reason: "unsupported-platform" }; } }; diff --git a/packages/trilium-core/src/index.ts b/packages/trilium-core/src/index.ts index 16d562257ea..df001ffe00f 100644 --- a/packages/trilium-core/src/index.ts +++ b/packages/trilium-core/src/index.ts @@ -126,7 +126,7 @@ export { inspectImage, type InspectedImage, UNKNOWN_FORMAT } from "./services/im export { type CoreConfig, initConfig, getConfig } from "./services/config"; export { default as imageService } from "./services/image"; export { t } from "i18next"; -export type { RequestProvider, ExecOpts, CookieJar } from "./services/request"; +export type { RequestProvider, ExecOpts, CookieJar, FetchResourceOpts, FetchedResource } from "./services/request"; export type * from "./meta"; export * as routeHelpers from "./routes/helpers"; diff --git a/apps/server/src/routes/api/link_embed.spec.ts b/packages/trilium-core/src/routes/api/link_embed.spec.ts similarity index 86% rename from apps/server/src/routes/api/link_embed.spec.ts rename to packages/trilium-core/src/routes/api/link_embed.spec.ts index 80dd8b29e19..07a513211ff 100644 --- a/apps/server/src/routes/api/link_embed.spec.ts +++ b/packages/trilium-core/src/routes/api/link_embed.spec.ts @@ -1,75 +1,121 @@ import { extractYouTubeVideoId } from "@triliumnext/commons"; -import { ValidationError } from "@triliumnext/core"; import type { Request } from "express"; import { Jimp } from "jimp"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const safeFetch = vi.hoisted(() => vi.fn()); - -vi.mock("../../services/safe_fetch.js", () => ({ - // Bypass SSRF/DNS checks in tests — just parse the URL. - validateUrl: (u: string) => new URL(u), - safeFetch: (...args: unknown[]) => safeFetch(...args) -})); +import { ValidationError } from "../../errors.js"; +import imageService from "../../services/image.js"; +import { getImageProvider, type ImageProvider, initImageProvider } from "../../services/image_provider.js"; +import { initRequest, readCappedResponse } from "../../services/request.js"; +import { fakeRequestProvider } from "../../test/request_provider.js"; +const safeFetch = vi.hoisted(() => vi.fn()); const saveImageToAttachment = vi.hoisted(() => vi.fn()); const awaitImageWrite = vi.hoisted(() => vi.fn(async () => {})); +import linkEmbedRoute from "./link_embed.js"; + +let attachmentCounter = 0; + /** * The route stores each picture it downloads as an attachment of the note the preview is going * into. What matters here is which picture was handed over, under which role and name — not that a * database took it — so the store is stood in for. + * + * Spied on the singleton rather than mocked as a module: the storing itself lives in core now, and + * core reaches its image service by a relative import that a mock of the package barrel would never + * intercept. */ -vi.mock("@triliumnext/core", async (importOriginal) => ({ - ...(await importOriginal>()), - imageService: { saveImageToAttachment, awaitImageWrite } -})); - -import linkEmbedRoute from "./link_embed.js"; - -let attachmentCounter = 0; - beforeEach(() => { attachmentCounter = 0; saveImageToAttachment.mockReset(); - saveImageToAttachment.mockImplementation((_noteId: string, _buffer: Buffer, fileName: string) => ({ + saveImageToAttachment.mockImplementation((_noteId: string, _bytes: Uint8Array, fileName: string) => ({ attachmentId: `att${++attachmentCounter}`, title: fileName })); + + vi.spyOn(imageService, "saveImageToAttachment").mockImplementation(saveImageToAttachment); + vi.spyOn(imageService, "awaitImageWrite").mockImplementation(awaitImageWrite); + + installResizer(resizeWithJimp); + + // The route reaches the network through the request provider, so that is where the network is + // stood in for. Only the getting of the response is faked: what the tests hand back goes through + // the real readCappedResponse, so the ceilings and the content-type handling under test here are + // the ones that will run in production rather than a second implementation of them. + initRequest(fakeRequestProvider({ + fetchResource: async (url, opts) => readCappedResponse(await safeFetch(url, opts), opts.maxBytes) + })); }); /** What was handed to the attachment store under a given role, if anything was. */ function stored(role: "favicon" | "coverImage") { const call = saveImageToAttachment.mock.calls.find((args: unknown[]) => args[5] === role); - return call ? { buffer: call[1] as Buffer, fileName: call[2] as string } : undefined; + return call ? { buffer: Buffer.from(call[1] as Uint8Array), fileName: call[2] as string } : undefined; } -function oneShotReader(bytes: Buffer) { - let sent = false; - return { - async read() { - if (sent) return { done: true, value: undefined }; - sent = true; - return { done: false, value: new Uint8Array(bytes) }; - }, - async cancel() {} - }; +/** + * Puts a known resizer behind the route. + * + * This spec runs under both test projects, and they install different image providers — the server's + * decodes, standalone's has no decoder at all. Left to whichever one the runtime bootstrapped, half + * these tests would assert a resize under one and its absence under the other. So the provider is + * chosen here instead, and what the route does with each kind of answer is tested deliberately + * rather than by whichever project happened to run it. + */ +function installResizer(resizeForPreview: ImageProvider["resizeForPreview"]) { + initImageProvider({ ...getImageProvider(), resizeForPreview }); } +/** + * A resizer that really resizes, standing in for the platform implementation. + * + * Deliberately the same shape as the server's, since that is what the route is written against. It + * is not the same code: the real one lives in the server package with Jimp, along with its own + * tests. What is under test here is the route's handling of an answer, not the making of one. + */ +const resizeWithJimp: ImageProvider["resizeForPreview"] = async (bytes, { maxEdge, jpegQuality }) => { + try { + const image = await Jimp.fromBuffer(Buffer.from(bytes)); + + if (image.bitmap.width > maxEdge || image.bitmap.height > maxEdge) { + image.scaleToFit({ w: maxEdge, h: maxEdge }); + } + + return { + resized: true, + bytes: new Uint8Array(image.hasAlpha() + ? await image.getBuffer("image/png") + : await image.getBuffer("image/jpeg", { quality: jpegQuality })) + }; + } catch { + return { resized: false, reason: "undecodable" }; + } +}; + +/** What a runtime with no decoder answers — standalone, and the server for a format Jimp refuses. */ +const noResizer: ImageProvider["resizeForPreview"] = async () => ({ + resized: false, + reason: "unsupported-platform" +}); + +/** + * What a site answers with, as a real `Response` — so the reading of it is the real reading of it. + * + * `json` is a convenience for an oEmbed answer: the payload *is* the JSON, since that is what the + * route now parses rather than asking a response object to do it. + */ function fakeResponse(payload: string | Buffer, opts: { ok?: boolean; contentType?: string; json?: unknown } = {}) { - const buf = Buffer.isBuffer(payload) ? payload : Buffer.from(payload); - const headers: Record = { - "content-type": opts.contentType ?? "text/html", - "content-length": String(buf.byteLength) - }; - return { - ok: opts.ok ?? true, + const body = opts.json !== undefined ? Buffer.from(JSON.stringify(opts.json)) : Buffer.from(payload); + + return new Response(body, { status: opts.ok === false ? 500 : 200, - headers: { get: (h: string) => headers[h.toLowerCase()] ?? null }, - body: { getReader: () => oneShotReader(buf) }, - json: async () => opts.json - }; + headers: { + "content-type": opts.contentType ?? "text/html", + "content-length": String(body.byteLength) + } + }); } /** A real, decodable PNG so the image pipeline runs for true rather than against a mock. */ @@ -78,6 +124,24 @@ async function makePng(width: number, height: number, color: number) { return Buffer.from(await image.getBuffer("image/png")); } +/** + * A PNG that is genuinely large, for the ceilings that are about bytes rather than pixels. + * + * Noise, because a flat colour of any dimensions compresses to a few KB — a 2000x2000 one lands + * under even the 100KB verbatim cap, so a test written with one would pass while proving nothing. + */ +async function makeNoisyPng(edge: number) { + const image = new Jimp({ width: edge, height: edge, color: 0x000000ff }); + + for (let i = 0; i < image.bitmap.data.length; i += 4) { + image.bitmap.data[i] = (i * 7919) % 256; + image.bitmap.data[i + 1] = (i * 104729) % 256; + image.bitmap.data[i + 2] = (i * 15485863) % 256; + } + + return Buffer.from(await image.getBuffer("image/png")); +} + /** * A real icon directory of one entry, since a favicon is now named by what its bytes say it is * rather than by the content type the site served it under. @@ -297,6 +361,41 @@ describe("link-embed getMetadata", () => { serveImage({ payload: "404", contentType: "text/html" }); expect(await imageOf()).toBeUndefined(); }); + + /** + * The runtime with no decoder — standalone, where there is no image library at all. The + * preview must survive it: a cover small enough to keep is kept exactly as it arrived, which + * is still stored rather than hotlinked, and one too large is dropped so a note does not + * take a megabyte of someone else's picture to show a card. + * + * This is the same path the server already takes for a WebP its own decoder refuses, which + * is why there is no third behaviour to write — only a second runtime reaching the second. + */ + describe("where nothing can scale the picture", () => { + beforeEach(() => installResizer(noResizer)); + + it("keeps a cover image of a storable size exactly as it arrived", async () => { + const png = await makePng(400, 400, 0x336699ff); + serveImage({ payload: png, contentType: "image/png" }); + + const image = await imageOf(); + expect(image?.fileName).toMatch(/\.png$/); + // Unscaled and byte-for-byte what the site served. + expect(image?.buffer.equals(png)).toBe(true); + }); + + it("drops a cover image too large to keep unscaled, leaving the rest of the preview", async () => { + // Noise rather than a flat colour: a solid 2000x2000 PNG compresses to a few KB and + // would sail under the verbatim cap this is about. + serveImage({ payload: await makeNoisyPng(600), contentType: "image/png" }); + + expect(await imageOf()).toBeUndefined(); + + const result = await linkEmbedRoute.getMetadata(req("https://example.com/page")); + expect(result.unresolved).toBeFalsy(); + expect(result.title).toBeTruthy(); + }); + }); }); describe("falling back to the site icon when there is no og:image", () => { @@ -677,14 +776,13 @@ describe("link-embed getMetadata", () => { it("treats a bodyless or content-type-less page response as unresolved", async () => { // No body: there is no HTML to read, so the page names itself nowhere. - safeFetch.mockResolvedValue({ ...fakeResponse("T"), body: undefined }); + safeFetch.mockResolvedValue(new Response(null, { headers: { "content-type": "text/html" } })); let result = await linkEmbedRoute.getMetadata(req("https://example.com/page")); expect(result.unresolved).toBe(true); - // No content-type header at all: not provably HTML, same as a wrong content type. - const response = fakeResponse("T"); - response.headers = { get: () => null }; - safeFetch.mockResolvedValue(response); + // No content-type header at all: not provably HTML, same as a wrong content type. A byte + // body is what makes that reachable — a string one would have fetch name it text/plain. + safeFetch.mockResolvedValue(new Response(Buffer.from("T"))); result = await linkEmbedRoute.getMetadata(req("https://example.com/page")); expect(result.unresolved).toBe(true); }); diff --git a/apps/server/src/routes/api/link_embed.ts b/packages/trilium-core/src/routes/api/link_embed.ts similarity index 77% rename from apps/server/src/routes/api/link_embed.ts rename to packages/trilium-core/src/routes/api/link_embed.ts index fac3f1a80a1..0ccc90a998a 100644 --- a/apps/server/src/routes/api/link_embed.ts +++ b/packages/trilium-core/src/routes/api/link_embed.ts @@ -1,23 +1,27 @@ import { extractYouTubeVideoId, type ImageAttachmentRole, - imageExtensionForMime, type LinkEmbedMetadata, linkPreviewImageName, safeHostname } from "@triliumnext/commons"; -import { getLog, imageService, ValidationError } from "@triliumnext/core"; import type { Request } from "express"; -import isSvg from "is-svg"; -import { Jimp } from "jimp"; import { parse } from "node-html-parser"; +import { ValidationError } from "../../errors.js"; import { trimIcoToSmallestEntry } from "../../services/ico.js"; -import { getImageTypeFromBuffer } from "../../services/image_codec.js"; +import imageService from "../../services/image.js"; +import { storePictureBytes } from "../../services/image_download.js"; +import { type InspectedImage, inspectImage, UNKNOWN_FORMAT } from "../../services/image_inspect.js"; +import { getImageProvider } from "../../services/image_provider.js"; +import { getLog } from "../../services/log.js"; import { findPageDescription } from "../../services/page_description.js"; -import { safeFetch, validateUrl } from "../../services/safe_fetch.js"; +import request, { type FetchedResource, validateFetchableUrl } from "../../services/request.js"; +import { decodeUtf8 } from "../../services/utils/binary.js"; const MAX_RESPONSE_SIZE = 512 * 1024; // 512KB +/** An oEmbed answer is a handful of fields; anything approaching this is not one. */ +const MAX_OEMBED_SIZE = 64 * 1024; // 64KB /** * How much of a favicon to fetch. @@ -70,66 +74,35 @@ const MAX_ICON_CANDIDATES = 3; const MAX_FAVICON_CANDIDATES = 3; /** - * Reads the response body as text, stopping after maxBytes to avoid - * buffering arbitrarily large responses into memory. + * A fetched resource read as text. + * + * Decoded as UTF-8 without exception, which is what the previous streaming read did as well. A page + * declaring some other encoding comes out mangled in its non-ASCII characters — but the tags this + * reads are ASCII, so it still finds them, and the alternative is carrying a charset decoder for the + * sake of a title that is occasionally accented. */ -async function readResponseText(response: Response, maxBytes: number): Promise { - const reader = response.body?.getReader(); - if (!reader) return ""; - - const decoder = new TextDecoder(); - let result = ""; - let bytesRead = 0; - - while (bytesRead < maxBytes) { - const { done, value } = await reader.read(); - if (done) break; - - bytesRead += value.byteLength; - result += decoder.decode(value, { stream: true }); - } - - void reader.cancel(); - return result.slice(0, maxBytes); +function asText({ bytes }: FetchedResource): string { + return decodeUtf8(bytes); } /** - * Downloads a binary resource, refusing anything over `maxBytes` — both up front, when the server - * advertises the size, and while streaming, when it does not. - * Returns undefined if the download fails or the resource is too large. + * Downloads a binary resource, or nothing where it could not be had. + * + * The ceiling, the streaming and the vetting of the address all belong to the request provider now, + * which is what lets this run on a runtime with no Node in it. What is left here is the one decision + * that is this route's rather than the transport's: a preview would rather go without a picture than + * fail, so every way of not getting one collapses to the same absence. + * + * `defaultContentType` stands in where a server named none — an `.ico` is routinely served as + * `application/octet-stream`, or as nothing at all. */ -async function downloadBinary(url: string, maxBytes: number, defaultContentType: string): Promise<{ buffer: Buffer; contentType: string } | undefined> { +async function downloadBinary(url: string, maxBytes: number, defaultContentType: string): Promise<{ bytes: Uint8Array; contentType: string } | undefined> { try { - const response = await safeFetch(url); + const response = await request.fetchResource(url, { maxBytes }); if (!response.ok) return undefined; - // Bail early if the server advertises a size over the limit - const contentLength = response.headers.get("content-length"); - if (contentLength && parseInt(contentLength, 10) > maxBytes) return undefined; - - const contentType = (response.headers.get("content-type") || defaultContentType).split(";")[0]; - - // Stream the body and enforce the size limit during download - const reader = response.body?.getReader(); - if (!reader) return undefined; - - const chunks: Uint8Array[] = []; - let bytesRead = 0; - - while (true) { - const { done, value } = await reader.read(); - if (done) break; - - bytesRead += value.byteLength; - if (bytesRead > maxBytes) { - void reader.cancel(); - return undefined; - } - chunks.push(value); - } - - return { buffer: Buffer.concat(chunks), contentType }; + return { bytes: response.bytes, contentType: response.contentType || defaultContentType }; } catch { return undefined; } @@ -137,8 +110,7 @@ async function downloadBinary(url: string, maxBytes: number, defaultContentType: /** A picture that has been fetched and identified, but not yet stored anywhere. */ interface DownloadedPicture { - buffer: Buffer; - mime: string; + bytes: Uint8Array; } /** @@ -174,18 +146,17 @@ async function storePicture( return undefined; } - const fileName = `${baseName}.${imageExtensionForMime(picture.mime)}`; - const attachment = imageService.saveImageToAttachment(noteId, picture.buffer, fileName, false, false, role); + const stored = storePictureBytes(noteId, picture.bytes, { role, title: baseName, shrink: false }); - if (!attachment.attachmentId) { + if (!stored) { return undefined; } // The client renders the preview the moment this answers, so the URL it is handed has to be // fetchable by then; otherwise the picture draws broken and stays that way until a reload. - await imageService.awaitImageWrite(attachment.attachmentId); + await imageService.awaitImageWrite(stored.attachmentId); - return `api/attachments/${attachment.attachmentId}/image/${encodeURIComponent(attachment.title)}`; + return stored.url; } /** @@ -203,25 +174,19 @@ async function storePictures(noteId: string, url: string, fetched: FetchedPrevie } /** - * The media type to name a downloaded picture by, read from its own bytes, or undefined when the - * bytes are not a picture at all. + * Bytes that really are a picture, or nothing. * * The response header cannot answer this. `favicon.ico` is routinely served as - * `application/octet-stream` or `text/plain`, and the media type is not decoration: it decides the - * extension the attachment is titled with and the type the picture is later served under. + * `application/octet-stream` or `text/plain`, and the answer is not decoration: what a picture is + * decides the extension its attachment is titled with and the type it is later served under, which + * is why {@link storePictureBytes} reads it off the bytes again at the moment it stores them. * - * Undefined also covers the case the header hides in the other direction — an HTML error page - * served in place of an image, which is not something to keep whatever it claims to be. + * Asking here as well is what lets a candidate be rejected while there are still others to try: an + * HTML error page served where an icon should be has to fail now, so the next `` + * gets its turn, rather than at store time when the search is already over. */ -async function detectImageMime(buffer: Buffer): Promise { - return (await getImageTypeFromBuffer(buffer))?.mime; -} - -/** Downloads a picture and names it by its own bytes, or nothing if those are not a picture. */ -async function asPicture(buffer: Buffer): Promise { - const mime = await detectImageMime(buffer); - - return mime ? { buffer, mime } : undefined; +function asPicture(bytes: Uint8Array): DownloadedPicture | undefined { + return inspectImage(bytes).format === UNKNOWN_FORMAT ? undefined : { bytes }; } /** @@ -236,12 +201,11 @@ async function downloadFavicon(faviconUrl: string): Promise IMAGE_MAX_DIMENSION || image.bitmap.height > IMAGE_MAX_DIMENSION) { - image.scaleToFit({ w: IMAGE_MAX_DIMENSION, h: IMAGE_MAX_DIMENSION }); - } + const resized = await getImageProvider().resizeForPreview(bytes, { + maxEdge: IMAGE_MAX_DIMENSION, + jpegQuality: IMAGE_JPEG_QUALITY + }); - // hasAlpha() inspects the pixels, not just the channel, so an opaque PNG still takes the - // JPEG path. An animated GIF/WebP collapses to its first frame, which is fine for a thumbnail. - return image.hasAlpha() - ? { buffer: Buffer.from(await image.getBuffer("image/png")), mime: "image/png" } - : { buffer: Buffer.from(await image.getBuffer("image/jpeg", { quality: IMAGE_JPEG_QUALITY })), mime: "image/jpeg" }; - } catch (e: unknown) { - // Jimp bundles decoders for PNG/JPEG/GIF/BMP/TIFF only, so a WebP or AVIF lands here. Keep - // the original bytes when they are small enough — unresized, but still not hotlinked. The - // bytes are asked what they are, the same way a favicon's are, so that an error page served - // in place of the image (an undecodable response that is not an image at all) is dropped - // rather than embedded. A caller that requires a minimum size gets nothing: undecodable - // means unverifiable. - // The URL is deliberately left out of the log: it is the user's private browsing, and a - // pasted link can carry a one-time token in its path or query. The timestamp is enough to - // match a log line against the paste that caused it. - getLog().info(`Could not decode a link preview image: ${e}`); - - return isSmallEnoughToKeepVerbatim && !minSourceDimension ? await asPicture(buffer) : undefined; + if (resized.resized) { + return { bytes: resized.bytes }; } + + // Nothing scaled it — an undecodable format (WebP, AVIF) where there is a decoder, anything at + // all where there is not. Keep the original bytes if they are small enough: unresized, but + // still stored rather than hotlinked, which is the property that matters. They are asked what + // they are on the way past, so an error page served in place of a picture is dropped rather + // than embedded. + return isSmallEnoughToKeepVerbatim ? asPicture(bytes) : undefined; +} + +/** + * Whether a picture is at least `minEdge` across, read from its header. + * + * Header rather than decode, so the answer costs nothing and is available on a runtime that has no + * decoder at all — which is the point: the alternative was to decode purely to measure, and to + * treat every picture that would not decode as too small. A header that does not say is treated as + * too small, since unverified is not the same as large enough. + */ +function isAtLeast({ width, height }: InspectedImage, minEdge: number): boolean { + return width !== null && height !== null && Math.max(width, height) >= minEdge; } /** @@ -368,7 +333,7 @@ function collectIconCandidates(document: ReturnType, pageUrl: stri candidates.sort((a, b) => b.size - a.size); const urls = candidates.map((candidate) => candidate.url); - // `pageUrl` already passed validateUrl, so resolving the conventional path against it cannot fail. + // `pageUrl` already passed validateFetchableUrl, so resolving the conventional path against it cannot fail. const conventional = new URL("/apple-touch-icon.png", pageUrl).toString(); if (!urls.includes(conventional)) { urls.push(conventional); @@ -450,7 +415,7 @@ function collectFaviconCandidates(document: ReturnType, pageUrl: s const urls = [ ...new Set(candidates.map((candidate) => candidate.url)) ].slice(0, MAX_FAVICON_CANDIDATES); // Every site is entitled to serve this whether it declares it or not, so it is the last resort - // rather than one of the ranked candidates. `pageUrl` already passed validateUrl, so it forms a + // rather than one of the ranked candidates. `pageUrl` already passed validateFetchableUrl, so it forms a // URL. Appended after the cap, never dropped by it. const conventional = new URL("/favicon.ico", pageUrl).toString(); if (!urls.includes(conventional)) { @@ -526,11 +491,11 @@ async function fetchYouTubeMetadata(url: string, videoId: string): Promise { const ogEl = document.querySelector(`meta[property="${property}"]`); @@ -614,7 +579,7 @@ async function getMetadata(req: Request) { throw new ValidationError("'noteId' is required"); } - const validatedUrl = validateUrl(urlParam); + const validatedUrl = validateFetchableUrl(urlParam); const url = validatedUrl.toString(); const videoId = extractYouTubeVideoId(url); diff --git a/packages/trilium-core/src/routes/index.ts b/packages/trilium-core/src/routes/index.ts index 9f2374fa6db..f7a636707b2 100644 --- a/packages/trilium-core/src/routes/index.ts +++ b/packages/trilium-core/src/routes/index.ts @@ -34,6 +34,7 @@ import backendLogRoute from "./api/backend_log"; import backupRoute from "./api/backup"; import passwordApiRoute from "./api/password"; import loginApiRoute from "./api/login"; +import linkEmbedRoute from "./api/link_embed"; // TODO: Deduplicate with routes.ts const GET = "get", @@ -142,6 +143,10 @@ export function buildSharedApiRoutes({ route, asyncRoute, apiRoute, asyncApiRout asyncApiRoute(PST, "/api/attachments/:attachmentId/compress-image", imageRoute.compressAttachmentImage); asyncRoute(PST, "/api/notes/:noteId/attachments/upload", [checkApiAuthOrElectron, uploadMiddlewareWithErrorHandling, csrfMiddleware], attachmentsApiRoute.uploadAttachment, apiResultHandler); + // POSTed rather than taking the URL in a query string: a link can carry a one-time token or a + // signature, and a query string ends up in every access log along the way. + asyncApiRoute(PST, "/api/link-embed/metadata", linkEmbedRoute.getMetadata); + // group of the services below are meant to be executed from the outside // Not transactional: a status read needs no transaction, and one is unopenable during the moment // a restore has the database detached — which is exactly when the wizard is polling hardest. diff --git a/apps/server/src/services/ico.spec.ts b/packages/trilium-core/src/services/ico.spec.ts similarity index 100% rename from apps/server/src/services/ico.spec.ts rename to packages/trilium-core/src/services/ico.spec.ts diff --git a/apps/server/src/services/ico.ts b/packages/trilium-core/src/services/ico.ts similarity index 100% rename from apps/server/src/services/ico.ts rename to packages/trilium-core/src/services/ico.ts diff --git a/packages/trilium-core/src/services/image_download.spec.ts b/packages/trilium-core/src/services/image_download.spec.ts index e6d8c1e0147..c6d29ddf9cf 100644 --- a/packages/trilium-core/src/services/image_download.spec.ts +++ b/packages/trilium-core/src/services/image_download.spec.ts @@ -5,6 +5,7 @@ import { getContext } from "./context.js"; import { downloadImages, downloadPictureToAttachment, storeLinkPreviewPictures } from "./image_download.js"; import noteService from "./notes.js"; import optionService from "./options.js"; +import { fakeRequestProvider } from "../test/request_provider.js"; import { initRequest } from "./request.js"; /** @@ -49,8 +50,7 @@ let asked: string[] = []; let answerWith: (url: string) => Buffer | undefined = () => PIXEL_PNG; beforeAll(() => { - initRequest({ - exec: async () => { throw new Error("Not used by these tests."); }, + initRequest(fakeRequestProvider({ getImage: async (url: string) => { asked.push(url); const bytes = answerWith(url); @@ -61,14 +61,11 @@ beforeAll(() => { return bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer; } - }); + })); }); afterAll(() => { - initRequest({ - exec: async () => { throw new Error("Request provider not initialized. Call initRequest() first."); }, - getImage: async () => { throw new Error("Request provider not initialized. Call initRequest() first."); } - }); + initRequest(fakeRequestProvider()); }); beforeEach(() => { diff --git a/packages/trilium-core/src/services/image_download.ts b/packages/trilium-core/src/services/image_download.ts index 0c510859e95..6d30de9729a 100644 --- a/packages/trilium-core/src/services/image_download.ts +++ b/packages/trilium-core/src/services/image_download.ts @@ -35,6 +35,18 @@ import { decodeBase64 } from "./utils/binary.js"; import { quoteRegex, unescapeHtml } from "./utils/index.js"; import { basename } from "./utils/path.js"; +/** A picture that has been stored, and the two ways the rest of the code refers to it. */ +export interface StoredPicture { + /** The `api/attachments/...` address the note references the picture by. */ + url: string; + /** + * The attachment now holding it, for a caller that has to know when the bytes are readable — + * `saveImageToAttachment` returns before the write lands. Anything that hands the URL straight + * to a client should pass this to {@link imageService.awaitImageWrite} first. + */ + attachmentId: string; +} + /** * Keeps a picture's bytes as an attachment of `noteId`, answering with the URL that references it — * or nothing when the bytes are not a picture at all. @@ -42,11 +54,11 @@ import { basename } from "./utils/path.js"; * The bytes are asked what they are rather than the address, the server or the `data:` prefix being * taken for it, so a 404 page served where a picture should be is refused rather than stored. */ -function storePictureBytes( +export function storePictureBytes( noteId: string, bytes: Uint8Array, { role, title, shrink }: { role: ImageAttachmentRole; title: string; shrink: boolean } -): string | undefined { +): StoredPicture | undefined { const { format, mime } = inspectImage(bytes); if (format === UNKNOWN_FORMAT) { @@ -64,7 +76,10 @@ function storePictureBytes( ); return attachment.attachmentId - ? `api/attachments/${attachment.attachmentId}/image/${encodeURIComponent(attachment.title)}` + ? { + url: `api/attachments/${attachment.attachmentId}/image/${encodeURIComponent(attachment.title)}`, + attachmentId: attachment.attachmentId + } : undefined; } @@ -93,7 +108,7 @@ export async function downloadPictureToAttachment( try { const bytes = new Uint8Array(await request.getImage(pictureUrl)); - return storePictureBytes(noteId, bytes, { role, title, shrink }); + return storePictureBytes(noteId, bytes, { role, title, shrink })?.url; } catch (e: unknown) { // The address is deliberately left out of the line: it is a page the user was reading, and // the note it came from is enough to find this again. @@ -392,7 +407,7 @@ function storeInlinePicture( // The picture was sized by whoever made the preview — ours are stored at an icon's and a // thumbnail's size already — so this only moves bytes, and recompressing them would be a // second lossy pass over a picture that has had one. - return storePictureBytes(noteId, decodeBase64(inline[1]), { role, title, shrink: false }); + return storePictureBytes(noteId, decodeBase64(inline[1]), { role, title, shrink: false })?.url; } catch (e: unknown) { getLog().info(`Could not store an inline preview picture of note '${noteId}': ${e}`); return undefined; diff --git a/packages/trilium-core/src/services/image_provider.spec.ts b/packages/trilium-core/src/services/image_provider.spec.ts index 54a216f58eb..e6e3cad67ed 100644 --- a/packages/trilium-core/src/services/image_provider.spec.ts +++ b/packages/trilium-core/src/services/image_provider.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; -import type { ImageCompressionOutcome, ImageProvider, ProcessedImage } from "./image_provider.js"; +import type { ImageCompressionOutcome, ImageProvider, PreviewResizeOutcome, ProcessedImage } from "./image_provider.js"; describe("image provider (core)", () => { // The core bootstrap installs a real image provider, so to exercise the @@ -22,7 +22,11 @@ describe("image provider (core)", () => { reason: "no-gain" })), planCompression: vi.fn(async () => ({ decodeCost: null })), - compressionConcurrency: vi.fn(() => 1) + compressionConcurrency: vi.fn(() => 1), + resizeForPreview: vi.fn(async (): Promise => ({ + resized: false, + reason: "unsupported-platform" + })) }; mod.initImageProvider(fake); expect(mod.getImageProvider()).toBe(fake); diff --git a/packages/trilium-core/src/services/image_provider.ts b/packages/trilium-core/src/services/image_provider.ts index 618791e33fa..9fc66f3a64b 100644 --- a/packages/trilium-core/src/services/image_provider.ts +++ b/packages/trilium-core/src/services/image_provider.ts @@ -93,8 +93,38 @@ export interface ImageProvider { * @param request - Fully resolved compression parameters */ planCompression(header: Uint8Array, request: ImageCompressionRequest): Promise; + + /** + * Scales a link preview's cover image down to `maxEdge` and re-encodes it, or declines. + * + * Separate from {@link compressImage} because the two want opposite things. Compression is + * offered an image the user chose to keep and tries to make it cheaper without changing what it + * is; this is handed someone else's `og:image` — up to 5MB of it — and wants a thumbnail nobody + * will see above a couple of hundred pixels. Reusing the compression path would mean carrying + * the full picture through a pipeline sized for the user's own photographs, to produce something + * that is thrown away at the size this asks for anyway. + * + * Declining is a first-class answer, not a failure: a runtime with no decoder cannot do this at + * all, and the caller has a perfectly good fallback — keep the original bytes when they are + * small enough, and otherwise show the preview without a picture, which is what it does for an + * image the decoder cannot read either way. + */ + resizeForPreview(bytes: Uint8Array, request: PreviewResizeRequest): Promise; +} + +/** How a preview's cover image is to be reduced. */ +export interface PreviewResizeRequest { + /** Longest edge to scale down to. Never up: a small picture is left at the size it came. */ + maxEdge: number; + /** Quality for the JPEG an opaque image becomes; one with real transparency becomes a PNG. */ + jpegQuality: number; } +/** Bytes to store in place of the original, or the reason there are none. */ +export type PreviewResizeOutcome = + | { resized: true; bytes: Uint8Array } + | { resized: false; reason: "undecodable" | "unsupported-platform" }; + /** What a provider makes of an image from its header alone. */ export interface ImageCompressionPlan { /** Set when the image is to be left alone, and why; absent when it has to be read in full. */ diff --git a/packages/trilium-core/src/services/import/notion/importer.integration.spec.ts b/packages/trilium-core/src/services/import/notion/importer.integration.spec.ts index 2478eb5ed02..b4c6414b4d4 100644 --- a/packages/trilium-core/src/services/import/notion/importer.integration.spec.ts +++ b/packages/trilium-core/src/services/import/notion/importer.integration.spec.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "vitest"; import becca from "../../../becca/becca.js"; import type BNote from "../../../becca/entities/bnote.js"; import { getContext } from "../../context.js"; +import { fakeRequestProvider } from "../../../test/request_provider.js"; import { initRequest } from "../../request.js"; import TaskContext from "../../task_context.js"; import notionImporter from "./importer.js"; @@ -1087,13 +1088,12 @@ describe("Notion importer — integration", () => { + ``; const asked: string[] = []; - initRequest({ - exec: async () => { throw new Error("Not used by this test."); }, + initRequest(fakeRequestProvider({ getImage: async (address: string) => { asked.push(address); return PIXEL_PNG.buffer.slice(PIXEL_PNG.byteOffset, PIXEL_PNG.byteOffset + PIXEL_PNG.byteLength) as ArrayBuffer; } - }); + })); try { const importRoot = await importNotion({ @@ -1113,10 +1113,7 @@ describe("Notion importer — integration", () => { expect(content).not.toContain("https://cdn.example.com/cover.png"); expect(content).toContain("api/attachments/"); } finally { - initRequest({ - exec: async () => { throw new Error("Request provider not initialized. Call initRequest() first."); }, - getImage: async () => { throw new Error("Request provider not initialized. Call initRequest() first."); } - }); + initRequest(fakeRequestProvider()); } }); diff --git a/packages/trilium-core/src/services/notes.spec.ts b/packages/trilium-core/src/services/notes.spec.ts index 8a76546b019..3267f470bec 100644 --- a/packages/trilium-core/src/services/notes.spec.ts +++ b/packages/trilium-core/src/services/notes.spec.ts @@ -7,6 +7,7 @@ import { disableEntityEvents, getContext } from "./context.js"; import { getLog } from "./log.js"; import noteService, { prepareTitle, saveLinks } from "./notes.js"; import optionService from "./options.js"; +import { fakeRequestProvider } from "../test/request_provider.js"; import { initRequest } from "./request.js"; import { getSql } from "./sql/index.js"; @@ -419,8 +420,7 @@ describe("notes service (real DB)", () => { // The pass itself is covered in image_download.spec.ts. What matters here is the wiring: // saving content is the only thing that starts it. const asked: string[] = []; - initRequest({ - exec: async () => { throw new Error("Not used by this test."); }, + initRequest(fakeRequestProvider({ getImage: async (address: string) => { asked.push(address); const png = Buffer.from( @@ -429,7 +429,7 @@ describe("notes service (real DB)", () => { ); return png.buffer.slice(png.byteOffset, png.byteOffset + png.byteLength) as ArrayBuffer; } - }); + })); const { note } = createNote("root", { title: "spec-preview-wiring", diff --git a/apps/server/src/services/page_description.spec.ts b/packages/trilium-core/src/services/page_description.spec.ts similarity index 100% rename from apps/server/src/services/page_description.spec.ts rename to packages/trilium-core/src/services/page_description.spec.ts diff --git a/apps/server/src/services/page_description.ts b/packages/trilium-core/src/services/page_description.ts similarity index 77% rename from apps/server/src/services/page_description.ts rename to packages/trilium-core/src/services/page_description.ts index 12c45899b95..81fae1ee55e 100644 --- a/apps/server/src/services/page_description.ts +++ b/packages/trilium-core/src/services/page_description.ts @@ -1,5 +1,3 @@ -import type { HTMLElement } from "node-html-parser"; - /** * A description read out of a page's own text, for a page that publishes none of itself. * @@ -11,6 +9,23 @@ import type { HTMLElement } from "node-html-parser"; * always preferred to anything found here. */ +/** + * The part of a parsed document this reads. + * + * Described structurally rather than as node-html-parser's own `HTMLElement`, so that a caller is + * not obliged to hold the very copy of the parser core resolves. A monorepo routinely ends up with + * two, and two copies of the same parser are two nominally different types — which has nothing to + * do with whether the tree can be read, and is not a constraint core has any business imposing on + * the app that parsed the page. + */ +export interface PageElement { + textContent: string; + rawTagName?: string | null; + parentNode: PageElement | null; + querySelector(selector: string): PageElement | null; + querySelectorAll(selector: string): PageElement[]; +} + /** Below this a paragraph is a caption, a byline or a stray line of chrome rather than a summary. */ const MIN_LENGTH = 80; /** What is kept. This ends up in the note's HTML, so it is bounded rather than left to the page. */ @@ -45,7 +60,7 @@ const IGNORED_ANCESTORS = new Set([ "nav", "aside", "header", "footer", "form", "figure", "figcaption", "table", "script", "style", "noscript", "template" ]); -export function findPageDescription(document: HTMLElement): string | undefined { +export function findPageDescription(document: PageElement): string | undefined { for (const scope of [ ...CONTENT_SCOPES.map((selector) => document.querySelector(selector)), document ]) { const summary = firstSummaryParagraph(scope); @@ -57,7 +72,7 @@ export function findPageDescription(document: HTMLElement): string | undefined { return undefined; } -function firstSummaryParagraph(scope: HTMLElement | null): string | undefined { +function firstSummaryParagraph(scope: PageElement | null): string | undefined { for (const paragraph of scope?.querySelectorAll("p") ?? []) { const text = collapseWhitespace(paragraph.textContent); @@ -75,7 +90,7 @@ function collapseWhitespace(text: string): string { return text.replace(/\s+/g, " ").trim(); } -function hasIgnoredAncestor(element: HTMLElement): boolean { +function hasIgnoredAncestor(element: PageElement): boolean { for (let node = element.parentNode; node; node = node.parentNode) { if (IGNORED_ANCESTORS.has(node.rawTagName?.toLowerCase() ?? "")) { return true; @@ -89,7 +104,7 @@ function hasIgnoredAncestor(element: HTMLElement): boolean { * A cookie notice, a row of breadcrumbs or a footer of policy links can all be long enough to pass * for a summary. What they are not is prose: nearly every word in them is a link. */ -function isMostlyLinks(paragraph: HTMLElement, text: string): boolean { +function isMostlyLinks(paragraph: PageElement, text: string): boolean { const linked = paragraph.querySelectorAll("a") .reduce((total, anchor) => total + collapseWhitespace(anchor.textContent).length, 0); diff --git a/packages/trilium-core/src/services/request.spec.ts b/packages/trilium-core/src/services/request.spec.ts index f95b346647d..994c0b72bac 100644 --- a/packages/trilium-core/src/services/request.spec.ts +++ b/packages/trilium-core/src/services/request.spec.ts @@ -1,11 +1,15 @@ import { describe, expect, it, vi } from "vitest"; +import { fakeRequestProvider } from "../test/request_provider.js"; import requestService, { type ExecOpts, + type FetchedResource, getRequestProvider, initRequest, isRequestInitialized, - type RequestProvider + readCappedResponse, + type RequestProvider, + validateFetchableUrl } from "./request.js"; describe("request provider (core)", () => { @@ -16,13 +20,15 @@ describe("request provider (core)", () => { expect(() => getRequestProvider()).toThrow(/not initialized/); }); - it("delegates exec/getImage to the installed provider", async () => { + it("delegates exec/getImage/fetchResource to the installed provider", async () => { const image = new ArrayBuffer(8); + const resource: FetchedResource = { status: 200, ok: true, contentType: "text/html", bytes: new Uint8Array([ 1 ]) }; const execMock = vi.fn(async (_opts: ExecOpts) => ({ ok: true })); - const fake: RequestProvider = { + const fake: RequestProvider = fakeRequestProvider({ exec: execMock as unknown as RequestProvider["exec"], - getImage: vi.fn(async () => image) - }; + getImage: vi.fn(async () => image), + fetchResource: vi.fn(async () => resource) + }); initRequest(fake); expect(isRequestInitialized()).toBe(true); @@ -34,5 +40,82 @@ describe("request provider (core)", () => { await expect(requestService.getImage("http://localhost/img.png")).resolves.toBe(image); expect(fake.getImage).toHaveBeenCalledWith("http://localhost/img.png"); + + await expect(requestService.fetchResource("http://localhost/p", { maxBytes: 10 })).resolves.toBe(resource); + expect(fake.fetchResource).toHaveBeenCalledWith("http://localhost/p", { maxBytes: 10 }); + }); + + it("refuses an address before anything is sent to it", () => { + expect(validateFetchableUrl("https://example.com/p").toString()).toBe("https://example.com/p"); + + expect(() => validateFetchableUrl("not a url")).toThrow(/Invalid URL/); + expect(() => validateFetchableUrl("file:///etc/passwd")).toThrow(/http and https/); + expect(() => validateFetchableUrl("javascript:alert(1)")).toThrow(/http and https/); + // Credentials would otherwise reach the wire, the stored URL and the log alike. + expect(() => validateFetchableUrl("https://user:pw@example.com/")).toThrow(/credentials/); + expect(() => validateFetchableUrl("https://user@example.com/")).toThrow(/credentials/); + }); +}); + +describe("readCappedResponse", () => { + it("reads a body whole and normalises what it came under", async () => { + const read = await readCappedResponse( + new Response("hello", { headers: { "content-type": "Text/HTML; charset=UTF-8" } }), + 1000 + ); + + expect(read).toEqual({ + status: 200, + ok: true, + contentType: "text/html", + bytes: new TextEncoder().encode("hello") + }); + }); + + it("answers a non-2xx rather than throwing, so the caller can read what it was", async () => { + const read = await readCappedResponse(new Response("nope", { status: 404 }), 1000); + + expect(read.status).toBe(404); + expect(read.ok).toBe(false); + }); + + it("names no content type where the server named none", async () => { + const read = await readCappedResponse(new Response(null, { headers: {} }), 1000); + + expect(read.contentType).toBe(""); + expect(read.bytes).toEqual(new Uint8Array()); + }); + + it("refuses a size the server advertises over the ceiling, without reading it", async () => { + const body = "x".repeat(100); + const response = new Response(body, { headers: { "content-length": "100" } }); + + await expect(readCappedResponse(response, 10)).rejects.toThrow(/100 bytes exceeds the 10 byte limit/); + }); + + /** + * The case the advertised size cannot cover: a chunked response states no length at all, so the + * only place to catch it is mid-stream. Written as a real stream of several chunks so that the + * refusal has to happen partway rather than after the whole body has arrived. + */ + it("abandons a stream that goes over the ceiling partway through", async () => { + let pulled = 0; + const stream = new ReadableStream({ + pull(controller) { + pulled++; + controller.enqueue(new Uint8Array(40)); + } + }); + + await expect(readCappedResponse(new Response(stream), 100)).rejects.toThrow(/exceeds the 100 byte limit/); + + // Three chunks of 40 is the first total over 100 — a fourth would mean it read on regardless. + expect(pulled).toBe(3); + }); + + it("keeps a body that fits exactly", async () => { + const read = await readCappedResponse(new Response(new Uint8Array(100)), 100); + + expect(read.bytes.byteLength).toBe(100); }); }); diff --git a/packages/trilium-core/src/services/request.ts b/packages/trilium-core/src/services/request.ts index ac0a5ec9447..b801becbf7f 100644 --- a/packages/trilium-core/src/services/request.ts +++ b/packages/trilium-core/src/services/request.ts @@ -1,3 +1,5 @@ +import { ValidationError } from "../errors.js"; + export interface CookieJar { header?: string; } @@ -19,9 +21,49 @@ export interface ExecOpts { body?: string | {}; } +/** What a resource fetch is allowed to cost, and what it asks for. */ +export interface FetchResourceOpts { + /** + * Refuse a body larger than this — both up front, where the server advertises a size, and while + * it streams, where it does not. + * + * Required rather than defaulted, because the ceiling is the whole point: what arrives is a + * third party's answer, held whole in memory, and on the runtime where that memory is a browser + * tab there is no second line of defence behind it. + */ + maxBytes: number; + headers?: Record; +} + +/** A resource that arrived whole, within its ceiling. */ +export interface FetchedResource { + status: number; + ok: boolean; + /** The media type alone, parameters stripped; empty where the server named none. */ + contentType: string; + bytes: Uint8Array; +} + export interface RequestProvider { exec(opts: ExecOpts): Promise; getImage(imageUrl: string): Promise; + /** + * Fetches a third-party resource named by note content or by the user, capped at + * {@link FetchResourceOpts.maxBytes}. + * + * Distinct from {@link getImage} in the two things a caller reading a page needs and a caller + * downloading a picture never did: a ceiling on what it will hold, and the media type it came + * under. Distinct from {@link exec} in that it is neither JSON nor Trilium's own protocol — the + * body is handed back as bytes, and what they mean is the caller's business. + * + * Throws rather than answering when the resource cannot be had at all: the URL is refused, the + * transport fails, or the body exceeds its ceiling. A non-2xx *answer* is not that — it comes + * back with its status, since a caller may well want to read an error page's content type. + * + * How hard the address is vetted is the implementation's to decide, and differs by runtime by + * necessity — see the note on each. + */ + fetchResource(url: string, opts: FetchResourceOpts): Promise; } let requestProvider: RequestProvider | null = null; @@ -47,5 +89,93 @@ export default { }, getImage(imageUrl: string): Promise { return getRequestProvider().getImage(imageUrl); + }, + fetchResource(url: string, opts: FetchResourceOpts): Promise { + return getRequestProvider().fetchResource(url, opts); } }; + +/** + * The checks on an outbound address that hold on every runtime, being about the address itself + * rather than about what it resolves to. + * + * Resolving the name and refusing the private addresses behind it is the other half of this, and it + * cannot live here: it needs a resolver, which is a Node module on one runtime and does not exist + * at all on another. So an implementation that has one does that too — see the server's safeFetch — + * and one that does not still gets these, which are the checks that need no network to make. + */ +export function validateFetchableUrl(urlString: string): URL { + let parsed: URL; + try { + parsed = new URL(urlString); + } catch { + throw new ValidationError("Invalid URL"); + } + + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + throw new ValidationError("Only http and https URLs are supported"); + } + + // `https://user:pass@host/` would put the credentials on the wire, and — for a link preview — + // into the note's stored URL and the log along with them. Nothing here needs to authenticate, + // so refuse the URL rather than carry a secret around. + if (parsed.username || parsed.password) { + throw new ValidationError("URLs containing credentials are not supported"); + } + + return parsed; +} + +/** + * Reads a `Response` into a {@link FetchedResource}, refusing a body over `maxBytes`. + * + * Shared by every implementation that has a real `Response` to read — the server's, which gets one + * from its SSRF-hardened fetch, and the browser's, which gets one from `fetch` itself. Only the + * getting of it differs between those two; the counting does not, and counting is the part with the + * edge cases: a size advertised and a size not, a body that lies about the first, a stream that has + * to be abandoned mid-flight rather than read to the end just to discover it was too long. + */ +export async function readCappedResponse(response: Response, maxBytes: number): Promise { + const contentType = (response.headers.get("content-type") ?? "").split(";")[0].trim().toLowerCase(); + const base = { status: response.status, ok: response.ok, contentType }; + + // Bail before reading a byte where the server says how many there will be. + const advertised = response.headers.get("content-length"); + if (advertised && Number(advertised) > maxBytes) { + void response.body?.cancel(); + throw new Error(`Response of ${advertised} bytes exceeds the ${maxBytes} byte limit`); + } + + const reader = response.body?.getReader(); + if (!reader) { + return { ...base, bytes: new Uint8Array() }; + } + + const chunks: Uint8Array[] = []; + let total = 0; + + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + + total += value.byteLength; + + // Checked as it streams, because content-length is a claim and a chunked response makes no + // claim at all. Abandoned at the moment it goes over rather than read to the end. + if (total > maxBytes) { + void reader.cancel(); + throw new Error(`Response exceeds the ${maxBytes} byte limit`); + } + + chunks.push(value); + } + + const bytes = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.byteLength; + } + + return { ...base, bytes }; +} diff --git a/packages/trilium-core/src/services/setup.spec.ts b/packages/trilium-core/src/services/setup.spec.ts index 220df563316..39b901b0f44 100644 --- a/packages/trilium-core/src/services/setup.spec.ts +++ b/packages/trilium-core/src/services/setup.spec.ts @@ -3,16 +3,17 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import appInfo from "./app_info.js"; import * as cls from "./context.js"; import options from "./options.js"; +import { fakeRequestProvider } from "../test/request_provider.js"; import { type ExecOpts, initRequest, type RequestProvider } from "./request.js"; import setupService from "./setup.js"; import sqlInit from "./sql_init.js"; import syncService from "./sync.js"; let execImpl: (opts: ExecOpts) => Promise = async () => ({}); -const fakeRequest: RequestProvider = { - exec: (opts: ExecOpts) => execImpl(opts) as Promise, +const fakeRequest: RequestProvider = fakeRequestProvider({ + exec: (opts: ExecOpts) => execImpl(opts) as Promise, getImage: async () => new ArrayBuffer(0) -}; +}); initRequest(fakeRequest); const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); diff --git a/packages/trilium-core/src/services/sync.spec.ts b/packages/trilium-core/src/services/sync.spec.ts index abdcf23f9d3..61b87dac9c4 100644 --- a/packages/trilium-core/src/services/sync.spec.ts +++ b/packages/trilium-core/src/services/sync.spec.ts @@ -8,6 +8,7 @@ import * as cls from "./context.js"; import entityChangesService from "./entity_changes.js"; import getInstanceId from "./instance_id.js"; import options from "./options.js"; +import { fakeRequestProvider } from "../test/request_provider.js"; import { type ExecOpts, initRequest, type RequestProvider } from "./request.js"; import { getSql } from "./sql/index.js"; import setupService from "./setup.js"; @@ -41,8 +42,8 @@ let changedIdx = 0; let checkIdx = 0; const requestLog: Array<{ method: string; url: string }> = []; -const fakeRequest: RequestProvider = { - exec: ((opts: ExecOpts): Promise => { +const fakeRequest: RequestProvider = fakeRequestProvider({ + exec: ((opts: ExecOpts): Promise => { requestLog.push({ method: opts.method, url: opts.url }); const url = opts.url; const reply = (value: unknown) => Promise.resolve(value as T); @@ -68,7 +69,7 @@ const fakeRequest: RequestProvider = { return reply({}); }) as RequestProvider["exec"], getImage: async () => new ArrayBuffer(0) -}; +}); initRequest(fakeRequest); const runSync = () => cls.init(() => syncService.sync()); diff --git a/packages/trilium-core/src/test/request_provider.ts b/packages/trilium-core/src/test/request_provider.ts new file mode 100644 index 00000000000..9b9d2813b74 --- /dev/null +++ b/packages/trilium-core/src/test/request_provider.ts @@ -0,0 +1,23 @@ +/** + * A {@link RequestProvider} standing in for the real one, with only the parts a given test needs. + * + * Every test that installs a provider wants one or two of its methods and has no opinion about the + * rest, and writing the rest out by hand meant that adding a method to the interface broke six + * unrelated specs at once. Here the unstubbed parts fail loudly instead, which is what a test that + * reaches one of them should do — it has wandered somewhere it did not mean to go. + */ + +import type { FetchedResource, FetchResourceOpts, ExecOpts, RequestProvider } from "../services/request.js"; + +export function fakeRequestProvider(overrides: Partial = {}): RequestProvider { + return { + exec: (_opts: ExecOpts): Promise => unstubbed("exec"), + getImage: (_imageUrl: string): Promise => unstubbed("getImage"), + fetchResource: (_url: string, _opts: FetchResourceOpts): Promise => unstubbed("fetchResource"), + ...overrides + }; +} + +function unstubbed(method: string): never { + throw new Error(`This test installed a request provider without ${method}(), and something called it.`); +}