Skip to content

Commit a914909

Browse files
trivikraduh95
authored andcommitted
vfs: support renaming implicit ZIP directories
Treat archive entry prefixes as directories when renaming with ZipProvider. Move all descendant entries to the new prefix for both asynchronous and synchronous operations. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: #65752 Fixes: #65751 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent d141ddb commit a914909

2 files changed

Lines changed: 114 additions & 30 deletions

File tree

lib/internal/vfs/providers/ziparchive.js

Lines changed: 79 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const {
2929
const {
3030
createEBADF,
3131
createEEXIST,
32+
createEINVAL,
3233
createEISDIR,
3334
createENOENT,
3435
createENOTDIR,
@@ -78,17 +79,41 @@ function isWritableFlag(flags) {
7879
}
7980

8081
/**
81-
* The `options.method` value that reproduces `method` (a `zipEntry.method`
82-
* raw compression method number) on `add()`/`addSync()`, so `rename()`
83-
* doesn't silently recompress an entry with a different method than the one
84-
* it already had (e.g. turning a zstd-compressed entry into a stored one).
85-
* @param {number} method
86-
* @returns {'store' | 'zstd' | 'deflate'}
82+
* Builds the options needed to preserve an entry's metadata and compression
83+
* method when adding it under a new name. Converts the raw ZIP compression
84+
* method number to the corresponding `add()`/`addSync()` option.
85+
* @param {ZipEntry} entry
86+
* @returns {{ mode: number | undefined, modified: Date,
87+
* method: 'store' | 'zstd' | 'deflate' }}
8788
*/
88-
function methodOption(method) {
89-
if (method === 0) return 'store';
90-
if (method === 93) return 'zstd';
91-
return 'deflate';
89+
function renameOptions(entry) {
90+
return {
91+
mode: entry.mode || undefined,
92+
modified: entry.modified,
93+
method: entry.method === 0 ? 'store' : entry.method === 93 ? 'zstd' : 'deflate',
94+
};
95+
}
96+
97+
/**
98+
* Finds the entries belonging to an implicit directory and maps their names
99+
* from the old directory prefix to the new one.
100+
* @param {ZipBuffer | ZipFile} source
101+
* @param {string} oldName
102+
* @param {string} newName
103+
* @returns {Array<{ oldName: string, newName: string }>}
104+
*/
105+
function getDirectoryRenames(source, oldName, newName) {
106+
const entries = [];
107+
const prefix = `${oldName}/`;
108+
for (const name of source.keys()) {
109+
if (StringPrototypeStartsWith(name, prefix)) {
110+
ArrayPrototypePush(entries, {
111+
oldName: name,
112+
newName: newName + StringPrototypeSlice(name, oldName.length),
113+
});
114+
}
115+
}
116+
return entries;
92117
}
93118

94119
/**
@@ -537,32 +562,56 @@ class ZipProvider extends VirtualProvider {
537562
const oldName = normalize(oldPath);
538563
const newName = normalize(newPath);
539564
const entry = await this.#getEntry(oldName);
540-
if (entry === null) throw createENOENT('rename', oldPath);
541-
// A file cannot take a directory's name; the archive would otherwise
542-
// hold both under it.
543-
if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath);
544-
const content = await entry.content();
545-
await this.#source.add(newName, content, {
546-
mode: entry.mode || undefined,
547-
modified: entry.modified,
548-
method: methodOption(entry.method),
549-
});
550-
await this.#source.delete(oldName);
565+
let entries;
566+
if (entry === null) {
567+
entries = getDirectoryRenames(this.#source, oldName, newName);
568+
if (entries.length === 0) throw createENOENT('rename', oldPath);
569+
if (StringPrototypeStartsWith(newName, `${oldName}/`)) {
570+
throw createEINVAL('rename', oldPath);
571+
}
572+
} else {
573+
if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath);
574+
entries = [{ oldName, newName, entry }];
575+
}
576+
if (oldName === newName) return;
577+
578+
for (let i = 0; i < entries.length; i++) {
579+
const item = entries[i];
580+
item.entry ??= await this.#getEntry(item.oldName);
581+
await this.#source.add(
582+
item.newName, await item.entry.content(), renameOptions(item.entry));
583+
}
584+
for (let i = 0; i < entries.length; i++) {
585+
await this.#source.delete(entries[i].oldName);
586+
}
551587
}
552588
renameSync(oldPath, newPath) {
553589
if (this.readonly) throw createEROFS('rename', oldPath);
554590
const oldName = normalize(oldPath);
555591
const newName = normalize(newPath);
556592
const entry = this.#getEntrySync(oldName);
557-
if (entry === null) throw createENOENT('rename', oldPath);
558-
if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath);
559-
const content = entry.contentSync();
560-
this.#source.addSync(newName, content, {
561-
mode: entry.mode || undefined,
562-
modified: entry.modified,
563-
method: methodOption(entry.method),
564-
});
565-
this.#deleteEntrySync(oldName);
593+
let entries;
594+
if (entry === null) {
595+
entries = getDirectoryRenames(this.#source, oldName, newName);
596+
if (entries.length === 0) throw createENOENT('rename', oldPath);
597+
if (StringPrototypeStartsWith(newName, `${oldName}/`)) {
598+
throw createEINVAL('rename', oldPath);
599+
}
600+
} else {
601+
if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath);
602+
entries = [{ oldName, newName, entry }];
603+
}
604+
if (oldName === newName) return;
605+
606+
for (let i = 0; i < entries.length; i++) {
607+
const item = entries[i];
608+
item.entry ??= this.#getEntrySync(item.oldName);
609+
this.#source.addSync(
610+
item.newName, item.entry.contentSync(), renameOptions(item.entry));
611+
}
612+
for (let i = 0; i < entries.length; i++) {
613+
this.#deleteEntrySync(entries[i].oldName);
614+
}
566615
}
567616

568617
/**

test/parallel/test-vfs-zip-provider.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,37 @@ async function buildArchive(entries, comment) {
119119
await assert.rejects(archiveVfs.promises.open('/does-not-exist.txt', 'r'), { code: 'ENOENT' });
120120
await assert.rejects(archiveVfs.promises.open('/a.txt', 'wx'), { code: 'EEXIST' });
121121
await assert.rejects(archiveVfs.promises.open('/dir', 'r'), { code: 'EISDIR' });
122+
123+
await archiveVfs.promises.rename('/dir', '/renamed-dir');
124+
await assert.rejects(archiveVfs.promises.stat('/dir'), { code: 'ENOENT' });
125+
assert.strictEqual(await archiveVfs.promises.readFile('/renamed-dir/b.txt', 'utf8'), 'nested');
126+
}
127+
128+
// Renaming an explicit directory also renames its children.
129+
{
130+
const archive = await buildArchive([
131+
await zlib.ZipEntry.create('async-dir/', Buffer.alloc(0)),
132+
await zlib.ZipEntry.create('async-dir/child.txt', Buffer.from('async')),
133+
await zlib.ZipEntry.create('sync-dir/', Buffer.alloc(0)),
134+
await zlib.ZipEntry.create('sync-dir/child.txt', Buffer.from('sync')),
135+
]);
136+
const zip = new zlib.ZipBuffer(archive);
137+
const archiveVfs = vfs.create(new vfs.ZipProvider(zip));
138+
139+
await archiveVfs.promises.rename('/async-dir', '/renamed-async-dir');
140+
assert.strictEqual(zip.has('async-dir/'), false);
141+
assert.strictEqual(zip.has('async-dir/child.txt'), false);
142+
assert.strictEqual(zip.has('renamed-async-dir/'), true);
143+
assert.strictEqual(
144+
await archiveVfs.promises.readFile('/renamed-async-dir/child.txt', 'utf8'),
145+
'async',
146+
);
147+
148+
archiveVfs.renameSync('/sync-dir', '/renamed-sync-dir');
149+
assert.strictEqual(zip.has('sync-dir/'), false);
150+
assert.strictEqual(zip.has('sync-dir/child.txt'), false);
151+
assert.strictEqual(zip.has('renamed-sync-dir/'), true);
152+
assert.strictEqual(archiveVfs.readFileSync('/renamed-sync-dir/child.txt', 'utf8'), 'sync');
122153
}
123154

124155
// --- ZipFile-backed, read-only: writes rejected with EROFS ----------------
@@ -220,6 +251,10 @@ async function buildArchive(entries, comment) {
220251
assert.throws(() => archiveVfs.openSync('/does-not-exist.txt', 'r'), { code: 'ENOENT' });
221252
assert.throws(() => archiveVfs.openSync('/a.txt', 'wx'), { code: 'EEXIST' });
222253
assert.throws(() => archiveVfs.openSync('/dir', 'r'), { code: 'EISDIR' });
254+
255+
archiveVfs.renameSync('/dir', '/renamed-dir');
256+
assert.throws(() => archiveVfs.statSync('/dir'), { code: 'ENOENT' });
257+
assert.strictEqual(archiveVfs.readFileSync('/renamed-dir/b.txt', 'utf8'), 'nested');
223258
}
224259

225260
// --- ZipFile-backed via openSync: sync-only round trip on disk -----------

0 commit comments

Comments
 (0)