diff --git a/.changeset/tender-points-sleep.md b/.changeset/tender-points-sleep.md new file mode 100644 index 00000000000..3206a87c20e --- /dev/null +++ b/.changeset/tender-points-sleep.md @@ -0,0 +1,5 @@ +--- +"effect": patch +--- + +Generate unique persisted paths for multipart files with duplicate filenames. diff --git a/packages/effect/src/unstable/http/Multipart.ts b/packages/effect/src/unstable/http/Multipart.ts index b5e7e42a589..08a32651428 100644 --- a/packages/effect/src/unstable/http/Multipart.ts +++ b/packages/effect/src/unstable/http/Multipart.ts @@ -678,6 +678,8 @@ export const toPersisted = ( const path_ = yield* Path.Path const dir = yield* fs.makeTempDirectoryScoped() const persisted: Record | Array | string> = Object.create(null) + const usedPaths = new Set() + let fileIndex = 0 yield* Stream.runForEach(stream, (part) => { if (part._tag === "Field") { if (!(part.key in persisted)) { @@ -692,7 +694,12 @@ export const toPersisted = ( return Effect.void } const file = part - const path = path_.join(dir, path_.basename(file.name).slice(-128)) + const fileName = path_.basename(file.name).slice(-128) + let path = path_.join(dir, fileName) + while (usedPaths.has(path)) { + path = path_.join(dir, `${fileIndex++}-${fileName}`) + } + usedPaths.add(path) const filePart = new PersistedFileImpl( file.key, file.name, diff --git a/packages/effect/test/unstable/http/Multipart.test.ts b/packages/effect/test/unstable/http/Multipart.test.ts index 1dafd5a5631..701914eea12 100644 --- a/packages/effect/test/unstable/http/Multipart.test.ts +++ b/packages/effect/test/unstable/http/Multipart.test.ts @@ -1,8 +1,8 @@ import { describe, it } from "@effect/vitest" -import { Effect, ErrorReporter, identity, Schema, Stream, Unify } from "effect" -import { Multipart } from "effect/unstable/http" +import { Effect, ErrorReporter, FileSystem, identity, Path, Schema, Stream, Unify } from "effect" +import { HttpClientRequest, HttpServerRequest, Multipart } from "effect/unstable/http" import * as HttpServerRespondable from "effect/unstable/http/HttpServerRespondable" -import { deepStrictEqual, strictEqual } from "node:assert" +import { deepStrictEqual, notStrictEqual, strictEqual } from "node:assert" describe("Multipart", () => { it.effect("parses fields and streams file content", () => @@ -60,6 +60,34 @@ describe("Multipart", () => { strictEqual(error.reason._tag, "TooManyParts") })) + it.effect("returns distinct persisted file paths for files with the same client filename", () => + Effect.scoped(Effect.gen(function*() { + const formData = new FormData() + formData.append("first", new File(["one"], "same.txt")) + formData.append("second", new File(["two"], "same.txt")) + const request = HttpServerRequest.fromClientRequest( + HttpClientRequest.bodyFormData(HttpClientRequest.post("https://example.com"), formData) + ) + const writes: Array = [] + const persisted = yield* Multipart.toPersisted( + request.multipartStream, + (path) => Effect.sync(() => writes.push(path)) + ).pipe( + Effect.provideService( + FileSystem.FileSystem, + FileSystem.makeNoop({ + makeTempDirectoryScoped: () => Effect.succeed("/tmp/audit") + }) + ), + Effect.provide(Path.layer) + ) + const first = (persisted.first as Array)[0] + const second = (persisted.second as Array)[0] + strictEqual(first.path, "/tmp/audit/same.txt") + notStrictEqual(first.path, second.path) + deepStrictEqual(writes, [first.path, second.path]) + }))) + it.effect("responds based on the reason and is ignored by the ErrorReporter", () => Effect.gen(function*() { const cases = [