feat: Deno.chown() make uid, gid args optional (#4612)

4 files changed, 174 insertions, 180 deletions

diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts
index cd82b4adf..82b3fc829 100644
--- a/cli/js/lib.deno.ns.d.ts
+++ b/cli/js/lib.deno.ns.d.ts
@@ -1080,10 +1080,14 @@ declare namespace Deno {
    * Throws Error (not implemented) if executed on Windows
    *
    * @param path path to the file
-   * @param uid user id (UID) of the new owner
-   * @param gid group id (GID) of the new owner
+   * @param uid user id (UID) of the new owner, or `null` for no change
+   * @param gid group id (GID) of the new owner, or `null` for no change
    */
-  export function chownSync(path: string | URL, uid: number, gid: number): void;
+  export function chownSync(
+    path: string | URL,
+    uid: number | null,
+    gid: number | null
+  ): void;
 
   /** Change owner of a regular file or directory. This functionality
    * is not available on Windows.
@@ -1097,13 +1101,13 @@ declare namespace Deno {
    * Throws Error (not implemented) if executed on Windows
    *
    * @param path path to the file
-   * @param uid user id (UID) of the new owner
-   * @param gid group id (GID) of the new owner
+   * @param uid user id (UID) of the new owner, or `null` for no change
+   * @param gid group id (GID) of the new owner, or `null` for no change
    */
   export function chown(
     path: string | URL,
-    uid: number,
-    gid: number
+    uid: number | null,
+    gid: number | null
   ): Promise<void>;
 
   export interface RemoveOptions {
diff --git a/cli/js/ops/fs/chown.ts b/cli/js/ops/fs/chown.ts
index f24ab5e55..3afe07f16 100644
--- a/cli/js/ops/fs/chown.ts
+++ b/cli/js/ops/fs/chown.ts
@@ -2,15 +2,19 @@
 import { sendSync, sendAsync } from "../dispatch_json.ts";
 import { pathFromURL } from "../../util.ts";
 
-export function chownSync(path: string | URL, uid: number, gid: number): void {
+export function chownSync(
+  path: string | URL,
+  uid: number | null,
+  gid: number | null
+): void {
   path = pathFromURL(path);
   sendSync("op_chown", { path, uid, gid });
 }
 
 export async function chown(
   path: string | URL,
-  uid: number,
-  gid: number
+  uid: number | null,
+  gid: number | null
 ): Promise<void> {
   path = pathFromURL(path);
   await sendAsync("op_chown", { path, uid, gid });
diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs
index 0d5d0c9bf..66487c41b 100644
--- a/cli/ops/fs.rs
+++ b/cli/ops/fs.rs
@@ -475,8 +475,8 @@ fn op_chmod(
 struct ChownArgs {
   promise_id: Option<u64>,
   path: String,
-  uid: u32,
-  gid: u32,
+  uid: Option<u32>,
+  gid: Option<u32>,
 }
 
 fn op_chown(
@@ -491,20 +491,18 @@ fn op_chown(
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_chown {} {} {}", path.display(), args.uid, args.gid);
+    debug!("op_chown {} {:?} {:?}", path.display(), args.uid, args.gid,);
     #[cfg(unix)]
     {
       use nix::unistd::{chown, Gid, Uid};
-      let nix_uid = Uid::from_raw(args.uid);
-      let nix_gid = Gid::from_raw(args.gid);
-      chown(&path, Option::Some(nix_uid), Option::Some(nix_gid))?;
+      let nix_uid = args.uid.map(Uid::from_raw);
+      let nix_gid = args.gid.map(Gid::from_raw);
+      chown(&path, nix_uid, nix_gid)?;
       Ok(json!({}))
     }
     // TODO Implement chown for Windows
     #[cfg(not(unix))]
     {
-      // Still check file/dir exists on Windows
-      let _metadata = std::fs::metadata(&path)?;
       Err(OpError::not_implemented())
     }
   })
diff --git a/cli/tests/unit/chown_test.ts b/cli/tests/unit/chown_test.ts
index bcd5ab9fe..163bfa48b 100644
--- a/cli/tests/unit/chown_test.ts
+++ b/cli/tests/unit/chown_test.ts
@@ -2,186 +2,174 @@
 import { unitTest, assertEquals, assert } from "./test_util.ts";
 
 // chown on Windows is noop for now, so ignore its testing on Windows
-if (Deno.build.os !== "windows") {
-  async function getUidAndGid(): Promise<{ uid: number; gid: number }> {
-    // get the user ID and group ID of the current process
-    const uidProc = Deno.run({
-      stdout: "piped",
-      cmd: ["python", "-c", "import os; print(os.getuid())"],
-    });
-    const gidProc = Deno.run({
-      stdout: "piped",
-      cmd: ["python", "-c", "import os; print(os.getgid())"],
-    });
-
-    assertEquals((await uidProc.status()).code, 0);
-    assertEquals((await gidProc.status()).code, 0);
-    const uid = parseInt(
-      new TextDecoder("utf-8").decode(await uidProc.output())
-    );
-    uidProc.close();
-    const gid = parseInt(
-      new TextDecoder("utf-8").decode(await gidProc.output())
-    );
-    gidProc.close();
-
-    return { uid, gid };
-  }
 
-  unitTest(async function chownNoWritePermission(): Promise<void> {
+async function getUidAndGid(): Promise<{ uid: number; gid: number }> {
+  // get the user ID and group ID of the current process
+  const uidProc = Deno.run({
+    stdout: "piped",
+    cmd: ["python", "-c", "import os; print(os.getuid())"],
+  });
+  const gidProc = Deno.run({
+    stdout: "piped",
+    cmd: ["python", "-c", "import os; print(os.getgid())"],
+  });
+
+  assertEquals((await uidProc.status()).code, 0);
+  assertEquals((await gidProc.status()).code, 0);
+  const uid = parseInt(new TextDecoder("utf-8").decode(await uidProc.output()));
+  uidProc.close();
+  const gid = parseInt(new TextDecoder("utf-8").decode(await gidProc.output()));
+  gidProc.close();
+
+  return { uid, gid };
+}
+
+unitTest(
+  { ignore: Deno.build.os == "windows" },
+  async function chownNoWritePermission(): Promise<void> {
     const filePath = "chown_test_file.txt";
     try {
       await Deno.chown(filePath, 1000, 1000);
     } catch (e) {
       assert(e instanceof Deno.errors.PermissionDenied);
     }
-  });
+  }
+);
 
-  unitTest(
-    { perms: { run: true, write: true } },
-    async function chownSyncFileNotExist(): Promise<void> {
-      const { uid, gid } = await getUidAndGid();
-      const filePath = Deno.makeTempDirSync() + "/chown_test_file.txt";
-
-      try {
-        Deno.chownSync(filePath, uid, gid);
-      } catch (e) {
-        assert(e instanceof Deno.errors.NotFound);
-      }
-    }
-  );
-
-  unitTest(
-    { perms: { run: true, write: true } },
-    async function chownFileNotExist(): Promise<void> {
-      const { uid, gid } = await getUidAndGid();
-      const filePath = (await Deno.makeTempDir()) + "/chown_test_file.txt";
-
-      try {
-        await Deno.chown(filePath, uid, gid);
-      } catch (e) {
-        assert(e instanceof Deno.errors.NotFound);
-      }
-    }
-  );
-
-  unitTest(
-    { perms: { write: true } },
-    function chownSyncPermissionDenied(): void {
-      const enc = new TextEncoder();
-      const dirPath = Deno.makeTempDirSync();
-      const filePath = dirPath + "/chown_test_file.txt";
-      const fileData = enc.encode("Hello");
-      Deno.writeFileSync(filePath, fileData);
-
-      try {
-        // try changing the file's owner to root
-        Deno.chownSync(filePath, 0, 0);
-      } catch (e) {
-        assert(e instanceof Deno.errors.PermissionDenied);
-      }
-      Deno.removeSync(dirPath, { recursive: true });
-    }
-  );
-
-  unitTest(
-    { perms: { write: true } },
-    async function chownPermissionDenied(): Promise<void> {
-      const enc = new TextEncoder();
-      const dirPath = await Deno.makeTempDir();
-      const filePath = dirPath + "/chown_test_file.txt";
-      const fileData = enc.encode("Hello");
-      await Deno.writeFile(filePath, fileData);
-
-      try {
-        // try changing the file's owner to root
-        await Deno.chown(filePath, 0, 0);
-      } catch (e) {
-        assert(e instanceof Deno.errors.PermissionDenied);
-      }
-      await Deno.remove(dirPath, { recursive: true });
-    }
-  );
-
-  unitTest(
-    { perms: { run: true, write: true } },
-    async function chownSyncSucceed(): Promise<void> {
-      // TODO: when a file's owner is actually being changed,
-      // chown only succeeds if run under priviledged user (root)
-      // The test script has no such privilege, so need to find a better way to test this case
-      const { uid, gid } = await getUidAndGid();
-
-      const enc = new TextEncoder();
-      const dirPath = Deno.makeTempDirSync();
-      const filePath = dirPath + "/chown_test_file.txt";
-      const fileData = enc.encode("Hello");
-      Deno.writeFileSync(filePath, fileData);
-
-      // the test script creates this file with the same uid and gid,
-      // here chown is a noop so it succeeds under non-priviledged user
-      Deno.chownSync(filePath, uid, gid);
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownSyncFileNotExist(): Promise<void> {
+    const { uid, gid } = await getUidAndGid();
+    const filePath = Deno.makeTempDirSync() + "/chown_test_file.txt";
 
-      Deno.removeSync(dirPath, { recursive: true });
+    try {
+      Deno.chownSync(filePath, uid, gid);
+    } catch (e) {
+      assert(e instanceof Deno.errors.NotFound);
     }
-  );
+  }
+);
 
-  unitTest(
-    { perms: { run: true, write: true } },
-    async function chownSyncWithUrl(): Promise<void> {
-      // TODO: same as chownSyncSucceed
-      const { uid, gid } = await getUidAndGid();
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownFileNotExist(): Promise<void> {
+    const { uid, gid } = await getUidAndGid();
+    const filePath = (await Deno.makeTempDir()) + "/chown_test_file.txt";
 
-      const enc = new TextEncoder();
-      const dirPath = Deno.makeTempDirSync();
-      const fileUrl = new URL(`file://${dirPath}/chown_test_file.txt`);
-      const fileData = enc.encode("Hello");
-      Deno.writeFileSync(fileUrl, fileData);
+    try {
+      await Deno.chown(filePath, uid, gid);
+    } catch (e) {
+      assert(e instanceof Deno.errors.NotFound);
+    }
+  }
+);
 
-      // the test script creates this file with the same uid and gid,
-      // here chown is a noop so it succeeds under non-priviledged user
-      Deno.chownSync(fileUrl, uid, gid);
+unitTest(
+  { perms: { write: true }, ignore: Deno.build.os == "windows" },
+  function chownSyncPermissionDenied(): void {
+    const dirPath = Deno.makeTempDirSync();
+    const filePath = dirPath + "/chown_test_file.txt";
+    Deno.writeTextFileSync(filePath, "Hello");
 
-      Deno.removeSync(dirPath, { recursive: true });
+    try {
+      // try changing the file's owner to root
+      Deno.chownSync(filePath, 0, 0);
+    } catch (e) {
+      assert(e instanceof Deno.errors.PermissionDenied);
     }
-  );
-
-  unitTest(
-    { perms: { run: true, write: true } },
-    async function chownSucceed(): Promise<void> {
-      // TODO: same as chownSyncSucceed
-      const { uid, gid } = await getUidAndGid();
-
-      const enc = new TextEncoder();
-      const dirPath = await Deno.makeTempDir();
-      const filePath = dirPath + "/chown_test_file.txt";
-      const fileData = enc.encode("Hello");
-      await Deno.writeFile(filePath, fileData);
-
-      // the test script creates this file with the same uid and gid,
-      // here chown is a noop so it succeeds under non-priviledged user
-      await Deno.chown(filePath, uid, gid);
+    Deno.removeSync(dirPath, { recursive: true });
+  }
+);
 
-      Deno.removeSync(dirPath, { recursive: true });
+unitTest(
+  { perms: { write: true }, ignore: Deno.build.os == "windows" },
+  async function chownPermissionDenied(): Promise<void> {
+    const dirPath = await Deno.makeTempDir();
+    const filePath = dirPath + "/chown_test_file.txt";
+    await Deno.writeTextFile(filePath, "Hello");
+
+    try {
+      // try changing the file's owner to root
+      await Deno.chown(filePath, 0, 0);
+    } catch (e) {
+      assert(e instanceof Deno.errors.PermissionDenied);
     }
-  );
+    await Deno.remove(dirPath, { recursive: true });
+  }
+);
 
-  unitTest(
-    { perms: { run: true, write: true } },
-    async function chownWithUrl(): Promise<void> {
-      // TODO: same as chownSyncSucceed
-      const { uid, gid } = await getUidAndGid();
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownSyncSucceed(): Promise<void> {
+    // TODO: when a file's owner is actually being changed,
+    // chown only succeeds if run under priviledged user (root)
+    // The test script has no such privilege, so need to find a better way to test this case
+    const { uid, gid } = await getUidAndGid();
 
-      const enc = new TextEncoder();
-      const dirPath = await Deno.makeTempDir();
-      const fileUrl = new URL(`file://${dirPath}/chown_test_file.txt`);
-      const fileData = enc.encode("Hello");
-      await Deno.writeFile(fileUrl, fileData);
+    const dirPath = Deno.makeTempDirSync();
+    const filePath = dirPath + "/chown_test_file.txt";
+    Deno.writeTextFileSync(filePath, "Hello");
 
-      // the test script creates this file with the same uid and gid,
-      // here chown is a noop so it succeeds under non-priviledged user
-      await Deno.chown(fileUrl, uid, gid);
+    // the test script creates this file with the same uid and gid,
+    // here chown is a noop so it succeeds under non-priviledged user
+    Deno.chownSync(filePath, uid, gid);
 
-      Deno.removeSync(dirPath, { recursive: true });
-    }
-  );
-}
+    Deno.removeSync(dirPath, { recursive: true });
+  }
+);
+
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownSyncWithUrl(): Promise<void> {
+    const { uid, gid } = await getUidAndGid();
+    const dirPath = Deno.makeTempDirSync();
+    const fileUrl = new URL(`file://${dirPath}/chown_test_file.txt`);
+    Deno.writeTextFileSync(fileUrl, "Hello");
+    Deno.chownSync(fileUrl, uid, gid);
+    Deno.removeSync(dirPath, { recursive: true });
+  }
+);
+
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownSucceed(): Promise<void> {
+    const { uid, gid } = await getUidAndGid();
+    const dirPath = await Deno.makeTempDir();
+    const filePath = dirPath + "/chown_test_file.txt";
+    await Deno.writeTextFile(filePath, "Hello");
+    await Deno.chown(filePath, uid, gid);
+    Deno.removeSync(dirPath, { recursive: true });
+  }
+);
+
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownUidOnly(): Promise<void> {
+    const { uid } = await getUidAndGid();
+    const dirPath = await Deno.makeTempDir();
+    const filePath = dirPath + "/chown_test_file.txt";
+    await Deno.writeTextFile(filePath, "Foo");
+    await Deno.chown(filePath, uid, null);
+    Deno.removeSync(dirPath, { recursive: true });
+  }
+);
+
+unitTest(
+  { perms: { run: true, write: true }, ignore: Deno.build.os == "windows" },
+  async function chownWithUrl(): Promise<void> {
+    // TODO: same as chownSyncSucceed
+    const { uid, gid } = await getUidAndGid();
+
+    const enc = new TextEncoder();
+    const dirPath = await Deno.makeTempDir();
+    const fileUrl = new URL(`file://${dirPath}/chown_test_file.txt`);
+    const fileData = enc.encode("Hello");
+    await Deno.writeFile(fileUrl, fileData);
+
+    // the test script creates this file with the same uid and gid,
+    // here chown is a noop so it succeeds under non-priviledged user
+    await Deno.chown(fileUrl, uid, gid);
+
+    Deno.removeSync(dirPath, { recursive: true });
+  }
+);