Skip to content

Commit ceab78c

Browse files
NicolappsConvex, Inc.
authored andcommitted
Make node-executor tolerate ZIP directory entries (#40542)
When node-executor downloads a source package, it verifies that the `metadata.json` file is correct by reading all entries from the source ZIP file and comparing them to `modulePaths` in `metadata.json`. However, the ZIP archiver used by node-executor integration tests also creates entries for directories, which makes tests fail if there is a module in a subfolder. This PR fixes node-executor to tolerate ZIP entries for directories, and also adds a subfolder module to tests. GitOrigin-RevId: ac24d1eb27742620b73a0ae376069470926b40a5
1 parent f72ba42 commit ceab78c

File tree

5 files changed

+14
-2
lines changed

5 files changed

+14
-2
lines changed

npm-packages/node-executor/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"prebuild": "rm -rf dist",
66
"build": "tsc && esbuild src/aws_lambda.ts --log-level=warning --bundle --sourcemap --platform=node --target=esnext --outfile=dist/aws_lambda.cjs && esbuild src/local.ts --log-level=warning --bundle --sourcemap --platform=node --target=esnext --outfile=dist/local.cjs",
77
"pretest": "rm -rf dist",
8-
"build-test-modules": "esbuild test/modules/*.ts --format=esm --log-level=warning --bundle --sourcemap --platform=node --target=esnext --outdir=dist/tests/modules && cp test/modules/*.js dist/tests/modules && cp test/metadata.json dist/tests/metadata.json && cp -r test/external_modules dist/tests/external_modules",
8+
"build-test-modules": "esbuild test/modules/*.ts --format=esm --log-level=warning --bundle --sourcemap --platform=node --target=esnext --outdir=dist/tests/modules && rsync -a --include '*/' --include '*.js' --exclude '*' test/modules/ dist/tests/modules/ && cp test/metadata.json dist/tests/metadata.json && cp -r test/external_modules dist/tests/external_modules",
99
"test": "vitest; npm run build-test-modules && esbuild test/integration-test.ts --log-level=warning --bundle --sourcemap --platform=node --target=esnext --outfile=dist/tests/integration-test.cjs && node --enable-source-maps dist/tests/integration-test.cjs"
1010
},
1111
"module": "esnext",

npm-packages/node-executor/src/source_package.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,16 @@ async function processSourcePackageStream(
329329
const startWrites = performance.now();
330330
const entries = await unzipFile(zipBuffer, dir, null);
331331
const actualModulePaths = entries
332-
.filter((entry) => entry !== "metadata.json")
332+
.filter(
333+
(entry) =>
334+
entry !== "metadata.json" &&
335+
// Some ZIP implementations store entries for directories themselves
336+
// (https://unix.stackexchange.com/a/743512/485280)
337+
// The Rust implementation we use in production doesn’t do it, but some
338+
// implementations (including the `archiver` npm package used in
339+
// node-executor integration tests) do so, so we are filtering them out.
340+
!entry.endsWith("/"),
341+
)
333342
.map((entry) => entry.substring("modules/".length));
334343
await fs.promises.chmod(`${dir}/metadata.json`, "444");
335344
const metadataJson = parseMetadataFile(

npm-packages/node-executor/test/integration-test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,7 @@ async function test_download() {
487487
const sourceDir = path.join(os.tmpdir(), `source/test_modules_key`);
488488
assert.equal(local.dir, sourceDir);
489489
assert.equal(local.modules.has("a.js"), true);
490+
assert.equal(local.modules.has("someFolder/someFile.js"), true);
490491
// Non-node modules
491492
assert.equal(local.modules.has("third_party.js"), false);
492493
assert.equal(local.modules.has("d.js"), false);

npm-packages/node-executor/test/metadata.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"diamond.js",
1111
"left.js",
1212
"logging.js",
13+
"someFolder/someFile.js",
1314
"right.js",
1415
"shared.js",
1516
"third_party.js",
@@ -29,6 +30,7 @@
2930
["logging.js", "isolate"],
3031
["right.js", "isolate"],
3132
["shared.js", "isolate"],
33+
["someFolder/someFile.js", "node"],
3234
["third_party.js", "isolate"],
3335
["transitive.js", "isolate"]
3436
]

npm-packages/node-executor/test/modules/someFolder/someFile.js

Whitespace-only changes.

0 commit comments

Comments
 (0)