fix(core): ignore explicitly undefined baseUrl in enqueueLinks helpers - #3924
Merged
Conversation
The per-crawler enqueueLinks helpers built their call as
`{ urls, baseUrl, ...enqueueLinksOptions }`, so a user options object
carrying an explicit `baseUrl: undefined` key overwrote the internally
resolved base URL. A falsy `baseUrl` leaves `enqueueStrategyPatterns`
empty, which means "no filtering" - every link on the page got enqueued
regardless of the requested strategy, silently going off-domain.
Spreading before `baseUrl` is safe because
`resolveBaseUrlForEnqueueLinksFiltering` already returns the user's
value whenever they did supply one.
B4nan
added
adhoc
github-actions
Bot
added
the
tested
vladfrangu
approved these changes
Jul 29, 2026
…rley-28431d # Conflicts: # test/core/enqueue_links/enqueue_links.test.ts
B4nan
added a commit
that referenced
this pull request
Aug 4, 2026
…defined (#3927) Follow-up to #3924, same bug class one level down. `BasicCrawler`'s context-bound `enqueueLinks` built its call as `{ requestQueue, robotsTxtFile, respectRobotsTxtFile, onSkippedRequest, limit, ...options }`, so a user options object carrying an explicitly present `undefined` key silently discarded the computed value: ```ts await enqueueLinks({ urls, limit: config.limit }); // config.limit is not set ``` Neither `ow.optional.number` nor TypeScript (without `exactOptionalPropertyTypes`) rejects an explicit `undefined`, so the `calculateEnqueuedRequestLimit()` result was dropped — the queue kept growing past the remaining `maxRequestsPerCrawl` budget and the `enqueueLimit` skips were never reported to `onSkippedRequest`. `maxRequestsPerCrawl` is still enforced as a stop condition via `isMaxPagesExceeded()`, so the effect was queue bloat and missing reporting rather than an unbounded crawl. The same applied to `robotsTxtFile` and `respectRobotsTxtFile` (robots.txt filtering silently disabled at enqueue time), and to `requestQueue`, where the explicit `undefined` made `ow` throw instead. Unlike #3924 this isn't a pure reordering, so the semantics for each computed key: - **`limit`** — now `min(user limit, remaining maxRequestsPerCrawl budget)`. This is what `addRequests()` (`maxNewRequests` after the spread) and the per-crawler `*CrawlerEnqueueLinks` helpers have always done; this path was the outlier. A user limit larger than the remaining budget is now capped, which is a user-visible change. - **`onSkippedRequest`** — composed instead of replaced: the crawler's reporting (logging, crawler-level `onSkippedRequest`) runs, then the user's callback. Previously passing a callback to `enqueueLinks` silently disabled the crawler-level one. - **`requestQueue`, `robotsTxtFile`, `respectRobotsTxtFile`** — `??` fallbacks, so a defined user value still wins. The enqueue-limit log message now also distinguishes the two causes, instead of blaming the user's `limit` when the `maxRequestsPerCrawl` clamp is what bit. Not covered here: `JSDOMCrawler` and `LinkeDOMCrawler` never route `context.enqueueLinks` through this wrapper at all (they call the standalone `enqueueLinks()` directly, unlike `CheerioCrawler`/`BrowserCrawler` which forward the bound context function), so `crawlDepth` injection and `maxCrawlDepth` are inert there. Separate issue, separate fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The per-crawler
*CrawlerEnqueueLinkshelpers built their call as{ urls, baseUrl, ...enqueueLinksOptions }, so a user options object carrying an explicitbaseUrl: undefinedkey overwrote the internally resolved base URL:Neither
ow.optional.stringnor TypeScript (withoutexactOptionalPropertyTypes) rejects an explicitly presentundefined, so this slipped through silently. InsideenqueueLinks()a falsybaseUrlleavesenqueueStrategyPatternsempty, and an empty pattern array means "no filtering" in bothcreateRequestsandfilterRequestsByPatterns— so every link on the page got enqueued regardless of the requested strategy, turning asame-domaincrawl into an unbounded off-domain one.Moving
baseUrlafter the spread fixes it, and is safe becauseresolveBaseUrlForEnqueueLinksFilteringalready returnsuserProvidedBaseUrlfirst when the user did supply one, so the resolved value equals the user's whenever it is set.adaptive-playwright-crawler.tsalready used this ordering.Affected:
cheerio-crawler,browser-crawler,jsdom-crawler,linkedom-crawler(both the boundoptions.enqueueLinks()path and the directenqueueLinks()path).