fix: prevent findUp infinite loop when given a relative dir - #17743
Open
JoshConley wants to merge 1 commit into
Open
fix: prevent findUp infinite loop when given a relative dir#17743JoshConley wants to merge 1 commit into
JoshConley wants to merge 1 commit into
Conversation
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
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.
What?
When
diris a relative path,findUpSync()andfindUp()loop forever at 100% CPU instead of returningnull.This is a follow-up to #12457, which fixed the same infinite loop for absolute paths. That fix added a
dir !== rootguard:rootcomes frompath.parse(dir).root, which is''for any relative path.path.dirname()bottoms out at'.', and'.'is never equal to'', so neither that guard nor theif (dir === root) return nullbelow it can ever fire:Why?
findConfig()inpackages/payload/src/config/find.tspassescompilerOptions.rootDirtofindUpSync()without resolving it first:A relative
rootDirin tsconfig.json is very common, for example"rootDir": "./"or"rootDir": "./src". Anypayloadbin command hangs if the nearest tsconfig sets one. In a monorepo this is easy to hit by accident, for example by runningpayload generate:typesfrom the wrong workspace. In this case, the process hangs until noticed.Note that this isn't the same as #15553. There,
generate:typesfinishes and writes its output but fails to exit but herefindConfig()never returns, so nothing is written at all.How?
Resolve
dirto an absolute path at the start of both functions, which makes the existing root checks reachable. After the fix the same call returnsnullright away, and callers report the intended error (e.g. "cannot find Payload config. Please create a configuration file...").Reproduction
No app or install is needed. Against stock
payload@3.87.1:It also reproduces end to end, in a package whose tsconfig.json sets
"rootDir": "./":node ./node_modules/payload/bin.js generate:types # hangs at 100% CPU, never returnsEnvironment Info
Test
Adds
packages/payload/src/utilities/findUp.spec.ts, co-located to matchformatLabels.spec.tsandisURLAllowed.spec.ts. It covers both functions with three cases: the relative dir with no match that used to hang, the absolute dir with no match that #12457 already fixed, and a positive case checking that a relative dir still finds a match in an ancestor.pnpm test:unitpasses with 5 tests.Note for review: The old behaviour is a synchronous infinite loop, which blocks the vitest worker thread, so no test timeout can fire. A regression hangs the run instead of failing an assertion.