-
Notifications
You must be signed in to change notification settings - Fork 5k
shell(mkdir, touch): report operands longer than the path buffers instead of aborting #38379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
robobun
wants to merge
7
commits into
main
Choose a base branch
from
farm/2191c16d/shell-mkdir-touch-long-operand
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+164
−8
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
0700c3f
shell(mkdir, touch): report operands longer than the path buffers ins…
robobun ed5c902
test: assert on absolute paths inside the fixtures
robobun 154db46
scratch: trim shell comments
robobun 6f25f02
shell(mkdir, touch): one-line comments
robobun 6e9136b
test(mkdir): pin that an absolute operand is bounded as written
robobun b1e04f4
ci: retrigger
robobun cdf4156
test(mkdir): assert the exact Windows outcome of the absolute dot-sla…
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, isWindows, tempDir } from "harness"; | ||
| import { join } from "node:path"; | ||
|
|
||
| // A relative operand was joined onto the cwd in a fixed 4096-byte buffer, so an | ||
| // operand longer than that crashed the process, and an operand longer than a | ||
| // PathBuffer (however it was spelled) was handed to the fs layer as "" and | ||
| // reported as ENOENT. Runs in a child process so a crash shows up as a failed | ||
| // assertion rather than taking the test runner down with it. | ||
| test("operands longer than the path buffers are reported, not a crash", async () => { | ||
| using dir = tempDir("mkdir-long-operand", {}); | ||
| const fixture = /* ts */ ` | ||
| import { $ } from "bun"; | ||
| import { existsSync } from "node:fs"; | ||
| $.nothrow(); | ||
| const dir = process.argv[1]; | ||
| const long = Buffer.alloc(5000, "a").toString(); | ||
| // Past the path buffer on every platform, Windows included. | ||
| const huge = Buffer.alloc(100_000, "h").toString(); | ||
| // Longer than the buffers as written, but normalizes down to one component. | ||
| const dotSlashes = Buffer.alloc(6000, "./").toString(); | ||
| const run = async (...args: string[]) => { | ||
| const { exitCode, stderr } = await $\`mkdir \${args}\`.quiet(); | ||
| return { exitCode, stderr: stderr.toString() }; | ||
| }; | ||
| console.log(JSON.stringify({ | ||
| cwd: process.cwd(), | ||
| relative: await run(long), | ||
| absolute: await run(dir + "/" + long), | ||
| parents: await run("-p", long), | ||
| huge: await run(huge), | ||
| mixed: { ...(await run(long, "short")), shortCreated: existsSync(dir + "/short") }, | ||
| dotSlashes: { ...(await run(dotSlashes + "normalized")), created: existsSync(dir + "/normalized") }, | ||
| absoluteDotSlashes: { | ||
| ...(await run(dir + "/" + dotSlashes + "as-written")), | ||
| created: existsSync(dir + "/as-written"), | ||
| }, | ||
| })); | ||
| `; | ||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", fixture, String(dir)], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(""); | ||
| const { cwd, ...results } = JSON.parse(stdout); | ||
|
|
||
| const long = Buffer.alloc(5000, "a").toString(); | ||
| const huge = Buffer.alloc(100_000, "h").toString(); | ||
| const dotSlashes = Buffer.alloc(6000, "./").toString(); | ||
| // mkdir reports the path it operated on: a relative operand joined onto the | ||
| // cwd, an absolute one as written. | ||
| const tooLong = (path: string) => `mkdir: ${path}: File name too long\n`; | ||
| // 5000 bytes fits Windows' much larger path buffer, so there the OS picks | ||
| // the error; what matters is that each operand fails on its own. | ||
| const failed = (path: string) => | ||
| isWindows ? { exitCode: 1, stderr: expect.stringMatching(/^mkdir: /) } : { exitCode: 1, stderr: tooLong(path) }; | ||
| expect(results).toEqual({ | ||
| relative: failed(join(cwd, long)), | ||
| absolute: failed(`${dir}/${long}`), | ||
| parents: failed(join(cwd, long)), | ||
| huge: { exitCode: 1, stderr: tooLong(join(cwd, huge)) }, | ||
| mixed: { ...failed(join(cwd, long)), shortCreated: true }, | ||
| dotSlashes: { exitCode: 0, stderr: "", created: true }, | ||
| // An absolute operand is not normalized (`..` through a symlink means | ||
| // something else to the kernel), so like the kernel and coreutils, mkdir | ||
| // bounds it as written. On Windows it fits the much larger buffer and the | ||
| // fs layer normalizes it while converting it to a wide path, so it works. | ||
| absoluteDotSlashes: isWindows | ||
| ? { exitCode: 0, stderr: "", created: true } | ||
| : { ...failed(`${dir}/${dotSlashes}as-written`), created: false }, | ||
| }); | ||
| expect(exitCode).toBe(0); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, isWindows, tempDir } from "harness"; | ||
| import { join } from "node:path"; | ||
|
|
||
| // Every operand, absolute or not, was joined into a fixed-size path buffer, so | ||
| // an operand longer than that crashed the process. Runs in a child process so a | ||
| // crash shows up as a failed assertion rather than taking the test runner down | ||
| // with it. | ||
| test("operands longer than the path buffer are reported, not a crash", async () => { | ||
| using dir = tempDir("touch-long-operand", {}); | ||
| const fixture = /* ts */ ` | ||
| import { $ } from "bun"; | ||
| import { existsSync } from "node:fs"; | ||
| $.nothrow(); | ||
| const dir = process.argv[1]; | ||
| const long = Buffer.alloc(5000, "a").toString(); | ||
| // Past the path buffer on every platform, Windows included. | ||
| const huge = Buffer.alloc(100_000, "h").toString(); | ||
| // Longer than the buffer as written, but normalizes down to one component. | ||
| const dotSlashes = Buffer.alloc(6000, "./").toString() + "normalized"; | ||
| const run = async (...args: string[]) => { | ||
| const { exitCode, stderr } = await $\`touch \${args}\`.quiet(); | ||
| return { exitCode, stderr: stderr.toString() }; | ||
| }; | ||
| console.log(JSON.stringify({ | ||
| cwd: process.cwd(), | ||
| relative: await run(long), | ||
| absolute: await run(dir + "/" + long), | ||
| huge: await run(huge), | ||
| mixed: { ...(await run(long, "short")), shortCreated: existsSync(dir + "/short") }, | ||
| dotSlashes: { ...(await run(dotSlashes)), created: existsSync(dir + "/normalized") }, | ||
| })); | ||
| `; | ||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", fixture, String(dir)], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(""); | ||
| const { cwd, ...results } = JSON.parse(stdout); | ||
|
|
||
| const long = Buffer.alloc(5000, "a").toString(); | ||
| const huge = Buffer.alloc(100_000, "h").toString(); | ||
| // The over-long path is passed to the OS whole, and touch reports the path | ||
| // it operated on: a relative operand joined onto the cwd. Which errno | ||
| // Windows picks for it is up to the OS; what matters is that each operand | ||
| // fails on its own. | ||
| const failed = (path: string) => | ||
| isWindows | ||
| ? { exitCode: 1, stderr: expect.stringMatching(/^touch: /) } | ||
| : { exitCode: 1, stderr: `touch: ${path}: File name too long\n` }; | ||
| expect(results).toEqual({ | ||
| relative: failed(join(cwd, long)), | ||
| absolute: failed(`${dir}/${long}`), | ||
| huge: failed(join(cwd, huge)), | ||
| mixed: { ...failed(join(cwd, long)), shortCreated: true }, | ||
| dotSlashes: { exitCode: 0, stderr: "", created: true }, | ||
| }); | ||
| expect(exitCode).toBe(0); | ||
| }); |
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.
Uh oh!
There was an error while loading. Please reload this page.