Compare commits

...

2 Commits

Author SHA1 Message Date
Claude Bot
812166805c test: use JS string parsing instead of shell pipe for portability
Replace shell pipeline `| head -1` with JS-based string splitting
and filtering for better cross-platform compatibility.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-06 16:14:36 +00:00
Claude Bot
a823413827 fix(write): respect createPath for S3-to-file writes
When using Bun.write() with an S3 file as the source and a local file
path as the destination, the createPath option (which defaults to true)
was not being respected. Parent directories were not created, causing
the write to fail if the directory didn't exist.

The fix adds mkdirp support to pipeReadableStreamToBlob() by:
1. Adding a mkdirp_if_not_exists parameter to the function
2. On Windows: handling NOENT error with mkdirp and retry loop
3. On POSIX: handling NOENT error with mkdirp and retry

Fixes #26780

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-06 16:09:13 +00:00
2 changed files with 175 additions and 13 deletions

View File

@@ -1078,7 +1078,7 @@ pub fn writeFileWithSourceDestination(ctx: *jsc.JSGlobalObject, source_blob: *Bl
source_blob,
@truncate(s3.options.partSize),
), ctx)) |stream| {
return destination_blob.pipeReadableStreamToBlob(ctx, stream, options.extra_options);
return destination_blob.pipeReadableStreamToBlob(ctx, stream, options.extra_options, options.mkdirp_if_not_exists orelse true);
} else {
return jsc.JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(ctx, ctx.createErrorInstance("Failed to stream bytes from s3 bucket", .{}));
}
@@ -2419,7 +2419,7 @@ comptime {
@export(&jsonRejectRequestStream, .{ .name = "Bun__FileStreamWrapper__onRejectRequestStream" });
}
pub fn pipeReadableStreamToBlob(this: *Blob, globalThis: *jsc.JSGlobalObject, readable_stream: jsc.WebCore.ReadableStream, extra_options: ?JSValue) bun.JSError!jsc.JSValue {
pub fn pipeReadableStreamToBlob(this: *Blob, globalThis: *jsc.JSGlobalObject, readable_stream: jsc.WebCore.ReadableStream, extra_options: ?JSValue, mkdirp_if_not_exists: bool) bun.JSError!jsc.JSValue {
var store = this.store orelse {
return jsc.JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, globalThis.createErrorInstance("Blob is detached", .{}));
};
@@ -2463,17 +2463,37 @@ pub fn pipeReadableStreamToBlob(this: *Blob, globalThis: *jsc.JSGlobalObject, re
const fd: bun.FileDescriptor = if (pathlike == .fd) pathlike.fd else brk: {
var file_path: bun.PathBuffer = undefined;
const path = pathlike.path.sliceZ(&file_path);
switch (bun.sys.open(
path,
bun.O.WRONLY | bun.O.CREAT | bun.O.NONBLOCK,
write_permissions,
)) {
.result => |result| {
break :brk result;
},
.err => |err| {
return jsc.JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, try err.withPath(path).toJS(globalThis));
},
var did_mkdirp = false;
while (true) {
switch (bun.sys.open(
path,
bun.O.WRONLY | bun.O.CREAT | bun.O.NONBLOCK,
write_permissions,
)) {
.result => |result| {
break :brk result;
},
.err => |err| {
// Try to create parent directories if they don't exist
if (err.getErrno() == .NOENT and mkdirp_if_not_exists and !did_mkdirp) {
if (std.fs.path.dirname(path)) |dirname| {
var node_fs: jsc.Node.fs.NodeFS = .{};
switch (node_fs.mkdirRecursive(.{
.path = .{ .string = bun.PathString.init(dirname) },
.recursive = true,
.always_return_none = true,
})) {
.result => {
did_mkdirp = true;
continue;
},
.err => {},
}
}
}
return jsc.JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, try err.withPath(path).toJS(globalThis));
},
}
}
unreachable;
};
@@ -2546,6 +2566,31 @@ pub fn pipeReadableStreamToBlob(this: *Blob, globalThis: *jsc.JSGlobalObject, re
switch (sink.start(stream_start)) {
.err => |err| {
// Try to create parent directories if they don't exist
if (err.getErrno() == .NOENT and mkdirp_if_not_exists and input_path == .path) {
if (std.fs.path.dirname(input_path.path.slice())) |dirname| {
var node_fs: jsc.Node.fs.NodeFS = .{};
switch (node_fs.mkdirRecursive(.{
.path = .{ .string = bun.PathString.init(dirname) },
.recursive = true,
.always_return_none = true,
})) {
.result => {
// Retry opening the file after creating directories
switch (sink.start(stream_start)) {
.err => |retry_err| {
sink.deref();
return jsc.JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, try retry_err.toJS(globalThis));
},
else => {
break :brk_sink sink;
},
}
},
.err => {},
}
}
}
sink.deref();
return jsc.JSPromise.dangerouslyCreateRejectedPromiseValueWithoutNotifyingVM(globalThis, try err.toJS(globalThis));
},

View File

@@ -0,0 +1,117 @@
// Regression test for https://github.com/oven-sh/bun/issues/26780
// S3-to-file writes should respect createPath option (default: true)
import { describe, expect, it } from "bun:test";
import child_process from "child_process";
import { isDockerEnabled, tempDir } from "harness";
import path from "path";
import * as dockerCompose from "../../docker/index.ts";
function getMinioContainerName(): string {
// Get container name without shell pipe for portability
const output = child_process.execSync(
`docker ps --filter "ancestor=minio/minio:latest" --filter "status=running" --format "{{.Names}}"`,
{
encoding: "utf-8",
},
);
const names = output.split("\n").filter(name => name.trim() !== "");
return names[0]?.trim() ?? "";
}
describe.skipIf(!isDockerEnabled())("issue #26780 - S3-to-file createPath", () => {
it("should create parent directories when writing S3 file to local path", async () => {
// Start MinIO using docker-compose
const minioInfo = await dockerCompose.ensure("minio");
// Get container name for docker exec
const containerName = getMinioContainerName();
if (!containerName) {
throw new Error("MinIO container not found");
}
// Create a bucket using mc inside the container
child_process.spawnSync("docker", [`exec`, containerName, `mc`, `mb`, `data/createpath-test`], {
stdio: "ignore",
});
const credentials = {
endpoint: `http://${minioInfo.host}:${minioInfo.ports[9000]}`,
accessKeyId: "minioadmin",
secretAccessKey: "minioadmin",
bucket: "createpath-test",
};
// Create S3 client and upload a test file
const s3Client = new Bun.S3Client(credentials);
const testContent = "Hello from S3 createPath test!";
const s3File = s3Client.file("test-file.txt");
await s3File.write(testContent);
// Verify S3 file was created
expect(await s3File.exists()).toBe(true);
// Create a temp directory and nested path that doesn't exist
using dir = tempDir("s3-createpath-test", {});
const nestedPath = path.join(String(dir), "nested", "deep", "path", "output.txt");
// This should work - createPath defaults to true
await Bun.write(nestedPath, s3File);
// Verify the file was written
expect(await Bun.file(nestedPath).exists()).toBe(true);
expect(await Bun.file(nestedPath).text()).toBe(testContent);
// Clean up S3
await s3File.unlink();
});
it("should fail when createPath is false and parent directory doesn't exist", async () => {
// Start MinIO using docker-compose
const minioInfo = await dockerCompose.ensure("minio");
// Get container name for docker exec
const containerName = getMinioContainerName();
if (!containerName) {
throw new Error("MinIO container not found");
}
// Create a bucket using mc inside the container
child_process.spawnSync("docker", [`exec`, containerName, `mc`, `mb`, `data/createpath-test2`], {
stdio: "ignore",
});
const credentials = {
endpoint: `http://${minioInfo.host}:${minioInfo.ports[9000]}`,
accessKeyId: "minioadmin",
secretAccessKey: "minioadmin",
bucket: "createpath-test2",
};
// Create S3 client and upload a test file
const s3Client = new Bun.S3Client(credentials);
const testContent = "Hello from S3 createPath false test!";
const s3File = s3Client.file("test-file2.txt");
await s3File.write(testContent);
// Verify S3 file was created
expect(await s3File.exists()).toBe(true);
// Create a temp directory and nested path that doesn't exist
using dir = tempDir("s3-createpath-false-test", {});
const nestedPath = path.join(String(dir), "nested2", "deep2", "path2", "output.txt");
// This should fail - createPath is false and directory doesn't exist
try {
await Bun.write(nestedPath, s3File, { createPath: false });
expect.unreachable();
} catch (err: any) {
expect(err.code).toBe("ENOENT");
}
// Clean up S3
await s3File.unlink();
});
});