Fix mkdir dedupeName for existing directories (HeyPuter#3215)

Author: xsourabhsharma

src/backend/controllers/fs/FSController.test.ts

@@ -953,6 +953,82 @@ describe('FSController.mkdirEntry', () => {
         expect(body.isDir).toBe(true);
     });
 
+    it('dedupes an existing directory when dedupe_name is true', async () => {
+        const { actor } = await makeUser();
+        const username = actor.user!.username!;
+        const target = `/${username}/Documents/hello`;
+
+        await withActor(actor, () =>
+            controller.mkdirEntry(
+                makeReq({
+                    body: { path: target },
+                    actor,
+                }),
+                makeRes().res,
+            ),
+        );
+
+        const { res, captured } = makeRes();
+        await withActor(actor, () =>
+            controller.mkdirEntry(
+                makeReq({
+                    body: { path: target, dedupe_name: true },
+                    actor,
+                }),
+                res,
+            ),
+        );
+
+        const body = captured.body as {
+            path: string;
+            name: string;
+            isDir: boolean;
+        };
+        expect(body.path).toBe(`/${username}/Documents/hello (1)`);
+        expect(body.name).toBe('hello (1)');
+        expect(body.isDir).toBe(true);
+        expect(
+            await server.stores.fsEntry.getEntryByPath(
+                `/${username}/Documents/hello (1)`,
+            ),
+        ).toMatchObject({ isDir: true });
+    });
+
+    it('requires parent write when deduping an existing directory', async () => {
+        const { actor: userActor } = await makeUser();
+        const username = userActor.user!.username!;
+        const appUid = `app-mkdir-${uuidv4()}`;
+        const appActor: Actor = { ...userActor, app: { uid: appUid } };
+        const target = `/${username}/AppData/${appUid}`;
+
+        await withActor(userActor, () =>
+            controller.mkdirEntry(
+                makeReq({
+                    body: { path: target },
+                    actor: userActor,
+                }),
+                makeRes().res,
+            ),
+        );
+
+        await expect(
+            withActor(appActor, () =>
+                controller.mkdirEntry(
+                    makeReq({
+                        body: { path: target, dedupe_name: true },
+                        actor: appActor,
+                    }),
+                    makeRes().res,
+                ),
+            ),
+        ).rejects.toMatchObject({ statusCode: 404 });
+        expect(
+            await server.stores.fsEntry.getEntryByPath(
+                `/${username}/AppData/${appUid} (1)`,
+            ),
+        ).toBeNull();
+    });
+
     it('expands ~/ in the path to the user home', async () => {
         const { actor } = await makeUser();
         const username = actor.user!.username!;

src/backend/controllers/fs/FSController.ts

@@ -1039,13 +1039,15 @@ export class FSController extends PuterController {
                 legacyCode: 'bad_request',
             });
 
+        const dedupeName =
+            this.#toBoolean(body.dedupe_name ?? body.dedupeName) ?? false;
         await this.#assertCanCreate(actor, path);
+        if (dedupeName) await this.#assertCanDedupeCreate(actor, path);
 
         const entry = await this.services.fs.mkdir(userId, {
             path,
             overwrite: this.#toBoolean(body.overwrite) ?? false,
-            dedupeName:
-                this.#toBoolean(body.dedupe_name ?? body.dedupeName) ?? false,
+            dedupeName,
             createMissingParents:
                 this.#toBoolean(
                     body.create_missing_parents ??
@@ -1310,6 +1312,17 @@ export class FSController extends PuterController {
         await this.#assertAccess(actor, parentForCheck, 'write');
     }
 
+    async #assertCanDedupeCreate(actor: Actor, targetPath: string) {
+        const existing = await this.stores.fsEntry.getEntryByPath(targetPath);
+        if (!existing) return;
+        const parent = pathPosix.dirname(targetPath);
+        await this.#assertAccess(
+            actor,
+            parent === '/' ? targetPath : parent,
+            'write',
+        );
+    }
+
     async #assertAccess(
         actor: Actor,
         path: string,

src/backend/controllers/fs/LegacyFSController.test.ts

@@ -206,6 +206,78 @@ describe('LegacyFSController.mkdir', () => {
         expect(fetched?.isDir).toBe(true);
     });
 
+    it('dedupes an existing directory when dedupe_name is true', async () => {
+        const { actor } = await makeUser();
+        const username = actor.user!.username!;
+        const parent = `/${username}/Documents`;
+
+        await withActor(actor, () =>
+            controller.mkdir(
+                makeReq({
+                    body: { parent, path: 'hello' },
+                    actor,
+                }),
+                makeRes().res,
+            ),
+        );
+
+        const { res, captured } = makeRes();
+        await withActor(actor, () =>
+            controller.mkdir(
+                makeReq({
+                    body: { parent, path: 'hello', dedupe_name: true },
+                    actor,
+                }),
+                res,
+            ),
+        );
+
+        const body = captured.body as Record<string, unknown>;
+        expect(body).toMatchObject({
+            path: `${parent}/hello (1)`,
+            name: 'hello (1)',
+            is_dir: true,
+        });
+        expect(
+            await server.stores.fsEntry.getEntryByPath(`${parent}/hello (1)`),
+        ).toMatchObject({ isDir: true });
+    });
+
+    it('requires parent write when deduping an existing directory', async () => {
+        const { actor: userActor } = await makeUser();
+        const username = userActor.user!.username!;
+        const appUid = `app-legacy-mkdir-${uuidv4()}`;
+        const appActor: Actor = { ...userActor, app: { uid: appUid } };
+        const parent = `/${username}/AppData`;
+
+        await withActor(userActor, () =>
+            controller.mkdir(
+                makeReq({
+                    body: { parent, path: appUid },
+                    actor: userActor,
+                }),
+                makeRes().res,
+            ),
+        );
+
+        await expect(
+            withActor(appActor, () =>
+                controller.mkdir(
+                    makeReq({
+                        body: { parent, path: appUid, dedupe_name: true },
+                        actor: appActor,
+                    }),
+                    makeRes().res,
+                ),
+            ),
+        ).rejects.toMatchObject({ statusCode: 404 });
+        expect(
+            await server.stores.fsEntry.getEntryByPath(
+                `${parent}/${appUid} (1)`,
+            ),
+        ).toBeNull();
+    });
+
     it("rejects writing into another user's home with a 4xx", async () => {
         const a = await makeUser();
         const b = await makeUser();

src/backend/controllers/fs/LegacyFSController.ts

@@ -427,17 +427,33 @@ export class LegacyFSController extends PuterController {
         const normalizedTarget = targetPath.startsWith('/')
             ? targetPath
             : `/${targetPath}`;
+        const dedupeName =
+            getBoolean(body, 'dedupe_name', 'change_name') ?? false;
         await assertCanCreate(
             this.services.acl,
             this.services.fs,
             actor,
             normalizedTarget,
         );
+        if (dedupeName) {
+            const existing =
+                await this.stores.fsEntry.getEntryByPath(normalizedTarget);
+            if (existing) {
+                const parent = pathPosix.dirname(normalizedTarget);
+                await assertAccess(
+                    this.services.acl,
+                    this.services.fs,
+                    actor,
+                    parent === '/' ? normalizedTarget : parent,
+                    'write',
+                );
+            }
+        }
 
         const entry = await this.services.fs.mkdir(userId, {
             path: targetPath,
             overwrite: getBoolean(body, 'overwrite') ?? false,
-            dedupeName: getBoolean(body, 'dedupe_name', 'change_name') ?? false,
+            dedupeName,
             createMissingParents:
                 getBoolean(
                     body,

src/backend/services/fs/FSService.ts

@@ -2840,10 +2840,13 @@ export class FSService extends PuterService {
         const existing = await this.stores.fsEntry.getEntryByPath(targetPath);
         if (existing) {
             if (existing.isDir) {
-                // A directory already exists at path: idempotent success.
-                return existing;
-            }
-            if (input.overwrite) {
+                if (input.dedupeName) {
+                    name = await this.#findDedupedName(parent, name);
+                } else {
+                    // A directory already exists at path: idempotent success.
+                    return existing;
+                }
+            } else if (input.overwrite) {
                 // Remove the non-directory occupant then create the dir.
                 await this.remove(userId, {
                     entry: existing,