Skip to content

Commit 64ecace

Browse files
authored
refactor(link-preview): move link previews into core so standalone can serve them (#10913)
2 parents 308df6c + bd1de70 commit 64ecace

32 files changed

Lines changed: 1009 additions & 257 deletions

apps/build-docs/src/build-docs.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ const stubImageProvider: ImageProvider = {
4444
},
4545
compressImage: async () => ({ compressed: false, reason: "unsupported-platform" }),
4646
planCompression: async () => ({ skip: "unsupported-platform" as const, decodeCost: null }),
47-
compressionConcurrency: () => 1
47+
compressionConcurrency: () => 1,
48+
resizeForPreview: async () => ({ resized: false, reason: "unsupported-platform" as const })
4849
};
4950
import { ZipArchive } from "archiver";
5051
import { execSync } from "child_process";

apps/server/src/routes/routes.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import etapiTokensApiRoutes from "./api/etapi_tokens.js";
2424
import filesRoute from "./api/files.js";
2525
import fontsRoute from "./api/fonts.js";
2626
// API routes
27-
import linkEmbedRoute from "./api/link_embed.js";
2827
import llmChatRoute from "./api/llm_chat.js";
2928
import llmSpecialNotesRoute from "./api/llm_special_notes.js";
3029
import loginApiRoute from "./api/login.js";
@@ -201,7 +200,6 @@ function register(app: express.Application) {
201200
// POST rather than GET: the URL would otherwise sit in the query string of every access-log
202201
// line (Trilium's own, and any reverse proxy in front of it), and a pasted URL can carry a
203202
// one-time token or a signed signature. The body is not logged.
204-
asyncApiRoute(PST, "/api/link-embed/metadata", linkEmbedRoute.getMetadata);
205203

206204
asyncApiRoute(PST, "/api/onenote-import/device-login", onenoteImportRoute.deviceLogin);
207205
asyncApiRoute(PST, "/api/onenote-import/device-poll", onenoteImportRoute.devicePoll);

apps/server/src/services/image_codec.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
import { IMAGE_COMPRESSIBLE_FORMATS, type ImageCompressionSkipReason } from "@triliumnext/commons";
1414
import { type InspectedImage, inspectImage } from "@triliumnext/core/src/services/image_inspect.js";
15-
import type { ImageCompressionOutcome, ImageCompressionRequest, ImageFormat } from "@triliumnext/core/src/services/image_provider.js";
15+
import type { ImageCompressionOutcome, ImageCompressionRequest, ImageFormat, PreviewResizeOutcome, PreviewResizeRequest } from "@triliumnext/core/src/services/image_provider.js";
1616
import { estimateJpegQuality } from "@triliumnext/core/src/services/jpeg_quality.js";
1717
import imageType from "image-type";
1818
import isAnimated from "is-animated";
@@ -442,3 +442,49 @@ export async function compressImageBytes(
442442
return { compressed: true, buffer: result, format: toJpeg ? JPEG_FORMAT : PNG_FORMAT };
443443
}
444444

445+
/**
446+
* Scales a link preview's cover image down to `maxEdge` and re-encodes it.
447+
*
448+
* The bytes make a round trip — downloaded and stored in the same breath — so the reduction happens
449+
* here rather than being left to the generic compression pass: a 5MB `og:image` has no business
450+
* being carried through a pipeline sized for the user's own photographs to become a thumbnail
451+
* nobody will see above a couple of hundred pixels.
452+
*
453+
* Transparency survives by re-encoding to PNG only where the picture actually has non-opaque pixels;
454+
* an opaque one becomes a JPEG, which is several times smaller.
455+
*
456+
* Answers `undecodable` rather than throwing when Jimp cannot read the bytes — it bundles decoders
457+
* for PNG/JPEG/GIF/BMP/TIFF only, so a WebP or an AVIF lands there, as does an error page served
458+
* where a picture should have been. Saying so is enough; what a preview does without its picture is
459+
* not this function's business.
460+
*/
461+
export async function resizePreviewImage(
462+
bytes: Uint8Array,
463+
{ maxEdge, jpegQuality }: PreviewResizeRequest,
464+
log: CodecLog = () => {}
465+
): Promise<PreviewResizeOutcome> {
466+
try {
467+
const image = await decodeImage(bytes);
468+
469+
// Only ever down: scaleToFit() would happily enlarge a smaller picture.
470+
if (image.bitmap.width > maxEdge || image.bitmap.height > maxEdge) {
471+
image.scaleToFit({ w: maxEdge, h: maxEdge });
472+
}
473+
474+
// hasAlpha() inspects the pixels rather than just the channel, so an opaque PNG still takes
475+
// the JPEG path. An animated GIF or WebP collapses to its first frame, which is all a
476+
// thumbnail wanted of it.
477+
const encoded = image.hasAlpha()
478+
? await image.getBuffer("image/png")
479+
: await image.getBuffer("image/jpeg", { quality: jpegQuality });
480+
481+
return { resized: true, bytes: new Uint8Array(encoded) };
482+
} catch (e: unknown) {
483+
// The address is deliberately left out of the line: it is the user's private browsing, and a
484+
// pasted link can carry a one-time token in its path or query.
485+
log(`Could not decode a link preview image: ${e}`, true);
486+
487+
return { resized: false, reason: "undecodable" };
488+
}
489+
}
490+
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { Jimp } from "jimp";
2+
import { describe, expect, it } from "vitest";
3+
4+
import { resizePreviewImage } from "./image_codec.js";
5+
6+
const REQUEST = { maxEdge: 256, jpegQuality: 75 };
7+
8+
async function png(width: number, height: number, color = 0xff0000ff) {
9+
return new Uint8Array(await new Jimp({ width, height, color }).getBuffer("image/png"));
10+
}
11+
12+
async function decoded(bytes: Uint8Array) {
13+
return await Jimp.fromBuffer(Buffer.from(bytes));
14+
}
15+
16+
describe("resizePreviewImage", () => {
17+
it("scales a large picture down to the longest edge asked for, keeping its proportions", async () => {
18+
const result = await resizePreviewImage(await png(1000, 500), REQUEST);
19+
20+
expect(result.resized).toBe(true);
21+
if (!result.resized) return;
22+
23+
const image = await decoded(result.bytes);
24+
expect(image.bitmap.width).toBe(256);
25+
expect(image.bitmap.height).toBe(128);
26+
});
27+
28+
it("leaves a picture already smaller than the ceiling at the size it came", async () => {
29+
const result = await resizePreviewImage(await png(64, 48), REQUEST);
30+
31+
expect(result.resized).toBe(true);
32+
if (!result.resized) return;
33+
34+
const image = await decoded(result.bytes);
35+
expect(image.bitmap.width).toBe(64);
36+
expect(image.bitmap.height).toBe(48);
37+
});
38+
39+
/**
40+
* The choice of encoding is by what the pixels actually are, not by what the format allows: a
41+
* PNG with nothing transparent in it is several times smaller as a JPEG, and a card thumbnail
42+
* is not where that saving should be passed up.
43+
*/
44+
it("re-encodes an opaque picture to JPEG and a transparent one to PNG", async () => {
45+
const opaque = await resizePreviewImage(await png(300, 300, 0x336699ff), REQUEST);
46+
const transparent = await resizePreviewImage(await png(300, 300, 0x33669900), REQUEST);
47+
48+
expect(opaque.resized && (await decoded(opaque.bytes)).mime).toBe("image/jpeg");
49+
expect(transparent.resized && (await decoded(transparent.bytes)).mime).toBe("image/png");
50+
});
51+
52+
it("says so rather than throwing when the bytes are not a picture it can read", async () => {
53+
// An error page served where a picture should have been — the case this has to survive.
54+
const html = new TextEncoder().encode("<!doctype html><title>404</title>");
55+
56+
await expect(resizePreviewImage(html, REQUEST)).resolves.toEqual({
57+
resized: false,
58+
reason: "undecodable"
59+
});
60+
});
61+
62+
it("passes the reason to the log it was given, and manages without one", async () => {
63+
const lines: string[] = [];
64+
await resizePreviewImage(new Uint8Array([ 1, 2, 3 ]), REQUEST, (message) => lines.push(message));
65+
66+
expect(lines).toHaveLength(1);
67+
expect(lines[0]).toMatch(/Could not decode a link preview image/);
68+
// The address never appears: a pasted link can carry a one-time token in its query.
69+
expect(lines[0]).not.toMatch(/http/);
70+
71+
await expect(resizePreviewImage(new Uint8Array([ 1, 2, 3 ]), REQUEST)).resolves.toMatchObject({
72+
resized: false
73+
});
74+
});
75+
});

apps/server/src/services/image_compression.spec.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -533,10 +533,7 @@ describe("compression parameters", () => {
533533
// The header pass is an optimization of the read, never a precondition for it — so when it
534534
// cannot answer, the run must go the long way round rather than report a failure.
535535
initImageProvider({
536-
getImageType: (buffer) => real.getImageType(buffer),
537-
processImage: (buffer, name, shrink) => real.processImage(buffer, name, shrink),
538-
compressImage: (buffer, compressionRequest) => real.compressImage(buffer, compressionRequest),
539-
compressionConcurrency: () => real.compressionConcurrency(),
536+
...real,
540537
planCompression: () => Promise.reject(new Error("planner unavailable"))
541538
});
542539

@@ -567,9 +564,7 @@ describe("compression parameters", () => {
567564
let compressedSoFar = 0;
568565

569566
initImageProvider({
570-
getImageType: (buffer) => real.getImageType(buffer),
571-
processImage: (buffer, name, shrink) => real.processImage(buffer, name, shrink),
572-
planCompression: (header, req) => real.planCompression(header, req),
567+
...real,
573568
compressionConcurrency: () => 1,
574569
compressImage: async (buffer, req) => {
575570
const outcome = await real.compressImage(buffer, req);
@@ -898,10 +893,7 @@ async function whileCompressing<T>(interfere: () => void, request: () => Promise
898893
const real = getImageProvider();
899894

900895
initImageProvider({
901-
getImageType: (buffer) => real.getImageType(buffer),
902-
processImage: (buffer, originalName, shrink) => real.processImage(buffer, originalName, shrink),
903-
planCompression: (header, compressionRequest) => real.planCompression(header, compressionRequest),
904-
compressionConcurrency: () => real.compressionConcurrency(),
896+
...real,
905897
compressImage: (buffer, compressionRequest) => {
906898
interfere();
907899
return real.compressImage(buffer, compressionRequest);

apps/server/src/services/image_provider.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { getLog, imageCompressionService, options as optionService } from "@triliumnext/core";
10-
import type { ImageCompressionOutcome, ImageCompressionPlan, ImageCompressionRequest, ImageFormat, ImageProvider, ProcessedImage } from "@triliumnext/core/src/services/image_provider.js";
10+
import type { ImageCompressionOutcome, ImageCompressionPlan, ImageCompressionRequest, ImageFormat, ImageProvider, PreviewResizeOutcome, PreviewResizeRequest, ProcessedImage } from "@triliumnext/core/src/services/image_provider.js";
1111

1212
import { createConcurrencyGate } from "./concurrency_gate.js";
1313
import {
@@ -16,7 +16,8 @@ import {
1616
decodeCostOf,
1717
detectSvg,
1818
getImageTypeFromBuffer,
19-
planFromBytes
19+
planFromBytes,
20+
resizePreviewImage
2021
} from "./image_codec.js";
2122
import { compressInWorker, compressionConcurrency } from "./image_worker_pool.js";
2223

@@ -124,5 +125,9 @@ export const serverImageProvider: ImageProvider = {
124125
return offThread ?? compressImageBytes(buffer, request, toBackendLog);
125126
},
126127

127-
compressionConcurrency
128+
compressionConcurrency,
129+
130+
async resizeForPreview(bytes: Uint8Array, request: PreviewResizeRequest): Promise<PreviewResizeOutcome> {
131+
return await resizePreviewImage(bytes, request, toBackendLog);
132+
}
128133
};

apps/server/src/services/request.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
"use strict";
22

3-
import { type CookieJar, type ExecOpts, getLog, type RequestProvider, sync_options as syncOptions } from "@triliumnext/core";
3+
import { type CookieJar, type ExecOpts, type FetchedResource, type FetchResourceOpts, getLog, type RequestProvider, sync_options as syncOptions } from "@triliumnext/core";
4+
import { readCappedResponse } from "@triliumnext/core/src/services/request.js";
45
import url from "url";
56

6-
import { createPinnedLookup, validateHostResolution, validateUrl } from "./safe_fetch.js";
7+
import { createPinnedLookup, safeFetch, validateHostResolution, validateUrl } from "./safe_fetch.js";
78

89
// this service provides abstraction over node's HTTP/HTTPS modules.
910
// Subclasses (e.g. apps/desktop's ElectronRequestProvider) can override
@@ -193,6 +194,20 @@ export default class NodeRequestProvider implements RequestProvider {
193194
});
194195
}
195196

197+
/**
198+
* Fetches a third-party resource, hardened the same way {@link getImage} is: the address is
199+
* vetted, the name resolved, the private ranges refused, and the connection pinned to the
200+
* addresses that were actually checked, so a second lookup cannot answer differently.
201+
*
202+
* This is the runtime where all of that is possible, and where it is most needed — the network
203+
* a server can see is not one the author of a note is entitled to reach through it.
204+
*/
205+
async fetchResource(resourceUrl: string, opts: FetchResourceOpts): Promise<FetchedResource> {
206+
const response = await safeFetch(resourceUrl, { headers: opts.headers });
207+
208+
return await readCappedResponse(response, opts.maxBytes);
209+
}
210+
196211
/**
197212
* Fetches an image named by note content.
198213
*

apps/server/src/services/safe_fetch.ts

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import dns from "node:dns";
22
import net from "node:net";
33

44
import { ValidationError } from "@triliumnext/core";
5+
import { validateFetchableUrl } from "@triliumnext/core/src/services/request.js";
56
import ipaddr from "ipaddr.js";
67
import { Agent, fetch as undiciFetch, type RequestInit as UndiciRequestInit, type Response as UndiciResponse } from "undici";
78

@@ -61,27 +62,11 @@ async function validateHostResolution(hostname: string): Promise<dns.LookupAddre
6162
return addresses;
6263
}
6364

64-
function validateUrl(urlString: string): URL {
65-
let parsed: URL;
66-
try {
67-
parsed = new URL(urlString);
68-
} catch {
69-
throw new ValidationError("Invalid URL");
70-
}
71-
72-
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") {
73-
throw new ValidationError("Only http and https URLs are supported");
74-
}
75-
76-
// `https://user:pass@host/` would put the credentials on the wire, and — for a link preview —
77-
// into the note's stored URL and the server log along with them. Nothing here needs to
78-
// authenticate, so refuse the URL rather than carry a secret around.
79-
if (parsed.username || parsed.password) {
80-
throw new ValidationError("URLs containing credentials are not supported");
81-
}
82-
83-
return parsed;
84-
}
65+
/**
66+
* The address checks, which are core's — they are about the URL rather than about the network, so
67+
* every runtime makes them and only this one can follow them with a resolution.
68+
*/
69+
const validateUrl = validateFetchableUrl;
8570

8671
/**
8772
* Creates a custom DNS lookup function that only returns pre-validated IP addresses,

apps/standalone/src/lightweight/bridged_request_provider.spec.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,3 +209,90 @@ describe("BridgedRequestProvider.getImage", () => {
209209
await expect(promise).rejects.toThrow("404 GET");
210210
});
211211
});
212+
213+
describe("BridgedRequestProvider.fetchResource", () => {
214+
it("asks the native side for the bytes and reports what they came under", async () => {
215+
const provider = new BridgedRequestProvider();
216+
const promise = provider.fetchResource("https://example.com/p", {
217+
maxBytes: 1000,
218+
headers: { "user-agent": "TriliumNotes" }
219+
});
220+
221+
expect(lastPosted().request).toMatchObject({
222+
method: "GET",
223+
url: "https://example.com/p",
224+
headers: { "user-agent": "TriliumNotes" },
225+
responseType: "arraybuffer"
226+
});
227+
228+
respond({ status: 200, headers: { "content-type": "text/html; charset=utf-8" }, body: btoa("<html/>") });
229+
230+
await expect(promise).resolves.toEqual({
231+
status: 200,
232+
ok: true,
233+
contentType: "text/html",
234+
bytes: new TextEncoder().encode("<html/>")
235+
});
236+
});
237+
238+
it("answers a non-2xx rather than throwing, the page having said something", async () => {
239+
const provider = new BridgedRequestProvider();
240+
const promise = provider.fetchResource("https://example.com/gone", { maxBytes: 1000 });
241+
242+
respond({ status: 404, headers: {}, body: btoa("nope") });
243+
244+
await expect(promise).resolves.toMatchObject({ status: 404, ok: false, contentType: "" });
245+
});
246+
247+
it("refuses an address before anything is posted for it", async () => {
248+
const provider = new BridgedRequestProvider();
249+
const posted = postSpy.mock.calls.length;
250+
251+
await expect(provider.fetchResource("file:///etc/passwd", { maxBytes: 10 })).rejects.toThrow(/http and https/);
252+
expect(postSpy.mock.calls.length).toBe(posted);
253+
});
254+
255+
/**
256+
* The ceiling is checked on what arrived rather than on what is arriving — a native transport
257+
* hands over a whole response, so there is no stream to abandon. It still has to hold.
258+
*/
259+
it("refuses a body over the ceiling once it is back", async () => {
260+
const provider = new BridgedRequestProvider();
261+
const promise = provider.fetchResource("https://example.com/big", { maxBytes: 10 });
262+
263+
respond({ status: 200, headers: {}, body: btoa("x".repeat(50)) });
264+
265+
await expect(promise).rejects.toThrow(/exceeds the 10 byte limit/);
266+
});
267+
268+
/**
269+
* And refuses it *before* decoding, which is the part that matters. Decoding is where an
270+
* oversized body multiplies — atob builds a binary string and Uint8Array.from copies that again
271+
* — so a check running afterwards has already paid for what it is about to refuse.
272+
*/
273+
it("refuses an oversized body without decoding it", async () => {
274+
const decode = vi.spyOn(globalThis, "atob");
275+
const provider = new BridgedRequestProvider();
276+
const promise = provider.fetchResource("https://example.com/big", { maxBytes: 10 });
277+
278+
respond({ status: 200, headers: {}, body: btoa("x".repeat(5000)) });
279+
280+
await expect(promise).rejects.toThrow(/exceeds the 10 byte limit/);
281+
expect(decode).not.toHaveBeenCalled();
282+
});
283+
284+
it("measures the decoded size rather than the encoded one, at every padding", async () => {
285+
// Base64 runs 4/3 the size of what it carries, so measuring the encoded string would refuse
286+
// a 30-byte body against a 30-byte ceiling. The three lengths cover the three paddings.
287+
for (const length of [ 30, 29, 28 ]) {
288+
const provider = new BridgedRequestProvider();
289+
const promise = provider.fetchResource("https://example.com/exact", { maxBytes: 30 });
290+
291+
respond({ status: 200, headers: {}, body: btoa("x".repeat(length)) });
292+
293+
await expect(promise).resolves.toMatchObject({
294+
bytes: new Uint8Array(length).fill("x".charCodeAt(0))
295+
});
296+
}
297+
});
298+
});

0 commit comments

Comments
 (0)