Skip to content

Commit d625706

Browse files
committed
Resolve known link-shortener URLs to their destination
When a bookmark is saved as a Google short/share link (search.app, share.google), the crawler already follows the redirect to fetch the real content, but the stored URL kept the opaque short link. Overwrite the stored URL with the resolved destination, scoped to a known-shortener allowlist and guarded by a safe-scheme check, so ordinary redirects (http->https, tracking-param strips) never rewrite the user's URL. Applied across all three crawl outcomes: - HTML pages: via the browser's final URL (Phase-1 bookmarkLinks write) - direct PDF/image links: the probe now returns its redirect target so the asset-bookmark path stores the resolved sourceUrl - downstream video jobs: crawlAndParseUrl returns the effective URL Covered by unit tests (packages/shared/utils/url.test.ts) and e2e tests that alias the nginx fixture as search.app and assert the resolved url and sourceUrl. Closes #2235
1 parent f6fae9c commit d625706

8 files changed

Lines changed: 265 additions & 21 deletions

File tree

apps/workers/workers/crawler/crawlAndParse.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import serverConfig from "@karakeep/shared/config";
3535
import logger from "@karakeep/shared/logger";
3636
import { BookmarkTypes } from "@karakeep/shared/types/bookmarks";
3737
import type { ZReaderViewReason } from "@karakeep/shared/types/bookmarks";
38+
import { resolveShortenedBookmarkUrl } from "@karakeep/shared/utils/url";
3839

3940
import type { ParseSubprocessOutput } from "../utils/parseHtmlSubprocessIpc";
4041
import {
@@ -51,7 +52,11 @@ import {
5152
} from "./assetStorage";
5253
import { crawlPage } from "./crawlPage";
5354
import { runParseSubprocess } from "./parseSubprocess";
54-
import { redactUrlCredentials, shouldRetryCrawlStatusCode } from "./utils";
55+
import {
56+
redactUrlCredentials,
57+
shouldRetryCrawlStatusCode,
58+
truncateUrl,
59+
} from "./utils";
5560

5661
const tracer = getTracer("@karakeep/workers");
5762

@@ -176,7 +181,7 @@ export interface CrawlAndParseUrlArgs {
176181
*/
177182
export async function crawlAndParseUrl(
178183
args: CrawlAndParseUrlArgs,
179-
): Promise<() => Promise<void>> {
184+
): Promise<{ runArchival: () => Promise<void>; effectiveUrl: string }> {
180185
const {
181186
url,
182187
userId,
@@ -329,12 +334,24 @@ export async function crawlAndParseUrl(
329334
}
330335
};
331336

337+
// Replace a known shortener (search.app/share.google) with its resolved
338+
// destination so the bookmark reflects the real URL. See issue #2235.
339+
const resolvedUrl = resolveShortenedBookmarkUrl(url, browserUrl);
340+
if (resolvedUrl) {
341+
logger.info(
342+
`[Crawler][${jobId}] Resolved shortened URL "${truncateUrl(
343+
url,
344+
)}" to "${truncateUrl(resolvedUrl)}". Updating the bookmark URL.`,
345+
);
346+
}
347+
332348
// Phase 1: Write metadata immediately for fast user feedback.
333349
// Content and asset storage happen later and can be slow (banner
334350
// image download, screenshot/pdf upload, etc.).
335351
await db
336352
.update(bookmarkLinks)
337353
.set({
354+
...(resolvedUrl ? { url: resolvedUrl } : {}),
338355
title: meta.title,
339356
description: meta.description,
340357
// Don't store data URIs as they're not valid URLs and are usually quite large
@@ -489,7 +506,7 @@ export async function crawlAndParseUrl(
489506
// Delete the old assets if any
490507
await Promise.all(assetDeletionTasks);
491508

492-
return async () => {
509+
const runArchival = async () => {
493510
if (
494511
!precrawledArchiveAssetId &&
495512
(serverConfig.crawler.fullPageArchive || archiveFullPage)
@@ -531,6 +548,10 @@ export async function crawlAndParseUrl(
531548
}
532549
}
533550
};
551+
552+
// effectiveUrl reflects any shortener resolution so downstream jobs
553+
// (e.g. video) use the real destination, not the opaque short link.
554+
return { runArchival, effectiveUrl: resolvedUrl ?? url };
534555
},
535556
);
536557
}

apps/workers/workers/crawler/probe.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ const PROBE_HTML_CONTENT_TYPES = new Set<string>([
4848

4949
export interface UrlProbeResult {
5050
contentType: string | null;
51+
/**
52+
* The URL after any redirects the probe followed. Callers use this to
53+
* resolve link-shortener URLs to their real destination.
54+
*/
55+
finalUrl: string;
5156
/**
5257
* Resolves with the page's preview metadata (or null). The extraction runs
5358
* in the background so it can overlap with the browser crawl — await it
@@ -120,7 +125,11 @@ export async function getContentTypeAndMetadata(
120125
logger.error(
121126
`[Crawler][${jobId}] Failed to determine the content-type for the url ${truncateUrl(url)}: ${e}`,
122127
);
123-
return { contentType: null, metadata: Promise.resolve(null) };
128+
return {
129+
contentType: null,
130+
finalUrl: url,
131+
metadata: Promise.resolve(null),
132+
};
124133
}
125134
setSpanAttributes({
126135
"crawler.getContentType.statusCode": response.status,
@@ -135,7 +144,11 @@ export async function getContentTypeAndMetadata(
135144
);
136145

137146
if (!contentType || !PROBE_HTML_CONTENT_TYPES.has(contentType)) {
138-
return { contentType, metadata: Promise.resolve(null) };
147+
return {
148+
contentType,
149+
finalUrl: response.url,
150+
metadata: Promise.resolve(null),
151+
};
139152
}
140153

141154
// A previous run already extracted and stored this page's metadata; the
@@ -147,7 +160,11 @@ export async function getContentTypeAndMetadata(
147160
addLogFields<"crawlerWorker.run">({
148161
"crawler.probe.metadata": "reused_stored",
149162
});
150-
return { contentType, metadata: Promise.resolve(null) };
163+
return {
164+
contentType,
165+
finalUrl: response.url,
166+
metadata: Promise.resolve(null),
167+
};
151168
}
152169

153170
// A blocked/retryable status usually means a challenge or error page
@@ -159,7 +176,11 @@ export async function getContentTypeAndMetadata(
159176
addLogFields<"crawlerWorker.run">({
160177
"crawler.probe.metadata": "blocked_status",
161178
});
162-
return { contentType, metadata: Promise.resolve(null) };
179+
return {
180+
contentType,
181+
finalUrl: response.url,
182+
metadata: Promise.resolve(null),
183+
};
163184
}
164185

165186
// The response is an HTML page: parse its metadata from the body we've
@@ -205,7 +226,7 @@ export async function getContentTypeAndMetadata(
205226
return null;
206227
}
207228
})();
208-
return { contentType, metadata };
229+
return { contentType, finalUrl: response.url, metadata };
209230
},
210231
);
211232
}

apps/workers/workers/crawlerWorker.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
} from "@karakeep/shared/queueing";
4040
import { getRateLimitClient } from "@karakeep/shared/ratelimiting";
4141
import { tryCatch } from "@karakeep/shared/tryCatch";
42+
import { resolveShortenedBookmarkUrl } from "@karakeep/shared/utils/url";
4243
import { WebhooksService } from "@karakeep/trpc/models/webhooks.service";
4344

4445
import {
@@ -393,20 +394,32 @@ async function runCrawler(
393394
// it — reload it from the bookmark row instead.
394395
const reuseStoredProbeMetadata =
395396
job.runNumber > 0 && probeMetadataAt !== null;
396-
const { contentType, metadata: probeMetadata }: UrlProbeResult =
397-
precrawledArchiveAssetId
398-
? { contentType: ASSET_TYPES.TEXT_HTML, metadata: Promise.resolve(null) }
399-
: await getContentTypeAndMetadata(url, jobId, job.abortSignal, runProxy, {
400-
skipMetadataExtraction: reuseStoredProbeMetadata,
401-
});
397+
const {
398+
contentType,
399+
finalUrl,
400+
metadata: probeMetadata,
401+
}: UrlProbeResult = precrawledArchiveAssetId
402+
? {
403+
contentType: ASSET_TYPES.TEXT_HTML,
404+
finalUrl: url,
405+
metadata: Promise.resolve(null),
406+
}
407+
: await getContentTypeAndMetadata(url, jobId, job.abortSignal, runProxy, {
408+
skipMetadataExtraction: reuseStoredProbeMetadata,
409+
});
402410
job.abortSignal.throwIfAborted();
403411

412+
// For asset bookmarks the shortener is resolved via the content-type probe's
413+
// redirect target (there's no browser crawl to derive it from). The HTML path
414+
// resolves separately inside crawlAndParseUrl via the browser URL.
415+
const assetUrl = resolveShortenedBookmarkUrl(url, finalUrl) ?? url;
416+
404417
// Link bookmarks get transformed into asset bookmarks if they point to a supported asset instead of a webpage
405418
const isPdf = contentType === ASSET_TYPES.APPLICATION_PDF;
406419

407420
if (isPdf) {
408421
await handleAsAssetBookmark(
409-
url,
422+
assetUrl,
410423
"pdf",
411424
userId,
412425
jobId,
@@ -420,7 +433,7 @@ async function runCrawler(
420433
SUPPORTED_UPLOAD_ASSET_TYPES.has(contentType)
421434
) {
422435
await handleAsAssetBookmark(
423-
url,
436+
assetUrl,
424437
"image",
425438
userId,
426439
jobId,
@@ -454,7 +467,7 @@ async function runCrawler(
454467
}
455468
return metadata;
456469
});
457-
const archivalLogic = await crawlAndParseUrl({
470+
const { runArchival, effectiveUrl } = await crawlAndParseUrl({
458471
url,
459472
userId,
460473
jobId,
@@ -475,10 +488,10 @@ async function runCrawler(
475488
probeMetadataPromise,
476489
});
477490

478-
await enqueuePostCrawlJobs(job, bookmarkId, userId, url);
491+
await enqueuePostCrawlJobs(job, bookmarkId, userId, effectiveUrl);
479492

480493
// Do the archival as a separate last step as it has the potential for failure
481-
await archivalLogic();
494+
await runArchival();
482495
}
483496

484497
// Record the latency from bookmark creation to crawl completion.

packages/e2e_tests/docker-compose.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ services:
1515
MEILI_ADDR: http://meilisearch:7700
1616
BROWSER_WEB_URL: http://chrome:9222
1717
CRAWLER_NUM_WORKERS: 6
18-
CRAWLER_ALLOWED_INTERNAL_HOSTNAMES: nginx
18+
CRAWLER_ALLOWED_INTERNAL_HOSTNAMES: nginx,search.app
1919
CRAWLER_STORE_PDF: "true"
2020
CRAWLER_VIDEO_DOWNLOAD: "true"
2121
CRAWLER_VIDEO_DOWNLOAD_MAX_SIZE: -1
@@ -48,6 +48,11 @@ services:
4848
nginx:
4949
image: nginx:alpine
5050
restart: unless-stopped
51+
networks:
52+
# Alias so the crawler can reach a "known link shortener" host in tests.
53+
default:
54+
aliases:
55+
- search.app
5156
volumes:
5257
- ./setup/html:/usr/share/nginx/html
5358
- ./setup/nginx/default.conf:/etc/nginx/conf.d/default.conf:ro

packages/e2e_tests/setup/nginx/default.conf

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ server {
77
return 302 http://127.0.0.1:80/hello.html;
88
}
99

10+
# Simulate a link-shortener redirect (reached via the search.app alias).
11+
location = /shortlink {
12+
return 302 http://nginx:80/hello.html;
13+
}
14+
15+
location = /shortlink-image {
16+
return 302 http://nginx:80/image.png;
17+
}
18+
1019
location / {
1120
try_files $uri =404;
1221
}

packages/e2e_tests/tests/workers/crawler.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,56 @@ describe("Crawler Tests", () => {
144144
expect(bookmark.content.htmlContent).toBeNull();
145145
});
146146

147+
it("resolves a known link-shortener URL to its destination", async () => {
148+
// search.app is aliased to the nginx container and allow-listed as a
149+
// known shortener, so /shortlink 302s to the real hello.html page.
150+
let { data: bookmark } = await client.POST("/bookmarks", {
151+
body: {
152+
type: "link",
153+
url: "http://search.app/shortlink",
154+
},
155+
});
156+
assert(bookmark);
157+
158+
await waitUntil(async () => {
159+
const data = await getBookmark(bookmark!.id);
160+
assert(data);
161+
assert(data.content.type === "link");
162+
return data.content.crawledAt !== null;
163+
}, "Shortened bookmark is crawled");
164+
165+
bookmark = await getBookmark(bookmark.id);
166+
assert(bookmark && bookmark.content.type === "link");
167+
// The stored URL should now be the resolved destination, not the short link.
168+
expect(bookmark.content.url).not.toContain("search.app");
169+
expect(bookmark.content.url).toContain("hello.html");
170+
expect(bookmark.content.htmlContent).toContain("Hello World");
171+
});
172+
173+
it("resolves a shortener that points directly to an asset", async () => {
174+
let { data: bookmark } = await client.POST("/bookmarks", {
175+
body: {
176+
type: "link",
177+
url: "http://search.app/shortlink-image",
178+
},
179+
});
180+
assert(bookmark);
181+
182+
await waitUntil(async () => {
183+
const data = await getBookmark(bookmark!.id);
184+
assert(data);
185+
return data.content.type === "asset";
186+
}, "Shortened asset bookmark is converted to an image");
187+
188+
bookmark = await getBookmark(bookmark.id);
189+
assert(bookmark && bookmark.content.type === "asset");
190+
expect(bookmark.content.assetType).toBe("image");
191+
// sourceUrl should be the resolved destination, not the short link.
192+
expect(bookmark.content.sourceUrl).not.toContain("search.app");
193+
expect(bookmark.content.sourceUrl).toContain("image.png");
194+
expect(bookmark.content.fileName).toBe("image.png");
195+
});
196+
147197
it("image lings jobs be converted into images", async () => {
148198
let { data: bookmark } = await client.POST("/bookmarks", {
149199
body: {

0 commit comments

Comments
 (0)