Add mode option to open/create (#4289)

cli/js/files.ts

@@ -22,43 +22,47 @@ import {
 } from "./ops/fs/open.ts";
 export { OpenOptions, OpenMode } from "./ops/fs/open.ts";
 
-export function openSync(path: string, mode?: OpenOptions): File;
-export function openSync(path: string, mode?: OpenMode): File;
+export function openSync(path: string, options?: OpenOptions): File;
+export function openSync(path: string, openMode?: OpenMode): File;
+
+/**@internal*/
 export function openSync(
   path: string,
   modeOrOptions: OpenOptions | OpenMode = "r"
 ): File {
-  let mode = undefined;
+  let openMode = undefined;
   let options = undefined;
 
   if (typeof modeOrOptions === "string") {
-    mode = modeOrOptions;
+    openMode = modeOrOptions;
   } else {
     checkOpenOptions(modeOrOptions);
     options = modeOrOptions as OpenOptions;
   }
 
-  const rid = opOpenSync(path, mode as OpenMode, options);
+  const rid = opOpenSync(path, openMode as OpenMode, options);
   return new File(rid);
 }
 
 export async function open(path: string, options?: OpenOptions): Promise<File>;
-export async function open(path: string, mode?: OpenMode): Promise<File>;
+export async function open(path: string, openMode?: OpenMode): Promise<File>;
+
+/**@internal*/
 export async function open(
   path: string,
   modeOrOptions: OpenOptions | OpenMode = "r"
 ): Promise<File> {
-  let mode = undefined;
+  let openMode = undefined;
   let options = undefined;
 
   if (typeof modeOrOptions === "string") {
-    mode = modeOrOptions;
+    openMode = modeOrOptions;
   } else {
     checkOpenOptions(modeOrOptions);
     options = modeOrOptions as OpenOptions;
   }
 
-  const rid = await opOpen(path, mode as OpenMode, options);
+  const rid = await opOpen(path, openMode as OpenMode, options);
   return new File(rid);
 }
 

cli/js/lib.deno.ns.d.ts

@@ -515,7 +515,7 @@ declare namespace Deno {
    *
    *       const file = Deno.openSync("/foo/bar.txt", { read: true, write: true });
    *
-   * Requires `allow-read` and `allow-write` permissions depending on mode.
+   * Requires `allow-read` and `allow-write` permissions depending on openMode.
    */
   export function openSync(path: string, options?: OpenOptions): File;
 
@@ -523,15 +523,15 @@ declare namespace Deno {
    *
    *       const file = Deno.openSync("/foo/bar.txt", "r");
    *
-   * Requires `allow-read` and `allow-write` permissions depending on mode.
+   * Requires `allow-read` and `allow-write` permissions depending on openMode.
    */
-  export function openSync(path: string, mode?: OpenMode): File;
+  export function openSync(path: string, openMode?: OpenMode): File;
 
   /** Open a file and resolve to an instance of the `File` object.
    *
    *     const file = await Deno.open("/foo/bar.txt", { read: true, write: true });
    *
-   * Requires `allow-read` and `allow-write` permissions depending on mode.
+   * Requires `allow-read` and `allow-write` permissions depending on openMode.
    */
   export function open(path: string, options?: OpenOptions): Promise<File>;
 
@@ -539,9 +539,9 @@ declare namespace Deno {
    *
    *     const file = await Deno.open("/foo/bar.txt, "w+");
    *
-   * Requires `allow-read` and `allow-write` permissions depending on mode.
+   * Requires `allow-read` and `allow-write` permissions depending on openMode.
    */
-  export function open(path: string, mode?: OpenMode): Promise<File>;
+  export function open(path: string, openMode?: OpenMode): Promise<File>;
 
   /** Creates a file if none exists or truncates an existing file and returns
    *  an instance of `Deno.File`.
@@ -686,9 +686,13 @@ declare namespace Deno {
      * access to be used. When createNew is set to `true`, create and truncate
      * are ignored. */
     createNew?: boolean;
+    /** Permissions to use if creating the file (defaults to `0o666`, before
+     * the process's umask).
+     * Ignored on Windows. */
+    mode?: number;
   }
 
-  /** A set of string literals which specify the open mode of a file.
+  /** A set of string literals which specify how to open a file.
    *
    * |Value |Description                                                                                       |
    * |------|--------------------------------------------------------------------------------------------------|

cli/js/ops/fs/open.ts

@@ -8,26 +8,34 @@ export interface OpenOptions {
   truncate?: boolean;
   create?: boolean;
   createNew?: boolean;
+  /** Permissions to use if creating the file (defaults to `0o666`, before
+   * the process's umask).
+   * It's an error to specify mode without also setting create or createNew to `true`.
+   * Ignored on Windows. */
+  mode?: number;
 }
 
 export type OpenMode = "r" | "r+" | "w" | "w+" | "a" | "a+" | "x" | "x+";
 
 export function openSync(
   path: string,
-  mode: OpenMode | undefined,
+  openMode: OpenMode | undefined,
   options: OpenOptions | undefined
 ): number {
-  return sendSync("op_open", { path, options, mode });
+  const mode: number | undefined = options?.mode;
+  return sendSync("op_open", { path, options, openMode, mode });
 }
 
 export async function open(
   path: string,
-  mode: OpenMode | undefined,
+  openMode: OpenMode | undefined,
   options: OpenOptions | undefined
 ): Promise<number> {
+  const mode: number | undefined = options?.mode;
   return sendAsync("op_open", {
     path,
     options,
+    openMode,
     mode
   });
 }

cli/js/tests/files_test.ts

@@ -74,6 +74,44 @@ unitTest(async function readerToAsyncIterator(): Promise<void> {
   assertEquals(totalSize, 12);
 });
 
+unitTest(
+  {
+    perms: { read: true, write: true }
+  },
+  function openSyncMode(): void {
+    const path = Deno.makeTempDirSync() + "/test_openSync.txt";
+    const file = Deno.openSync(path, {
+      write: true,
+      createNew: true,
+      mode: 0o626
+    });
+    file.close();
+    const pathInfo = Deno.statSync(path);
+    if (Deno.build.os !== "win") {
+      assertEquals(pathInfo.mode! & 0o777, 0o626 & ~Deno.umask());
+    }
+  }
+);
+
+unitTest(
+  {
+    perms: { read: true, write: true }
+  },
+  async function openMode(): Promise<void> {
+    const path = (await Deno.makeTempDir()) + "/test_open.txt";
+    const file = await Deno.open(path, {
+      write: true,
+      createNew: true,
+      mode: 0o626
+    });
+    file.close();
+    const pathInfo = Deno.statSync(path);
+    if (Deno.build.os !== "win") {
+      assertEquals(pathInfo.mode! & 0o777, 0o626 & ~Deno.umask());
+    }
+  }
+);
+
 unitTest(
   { perms: { write: false } },
   async function writePermFailure(): Promise<void> {

cli/ops/fs.rs

@@ -18,7 +18,7 @@ use std::time::UNIX_EPOCH;
 use tokio;
 
 #[cfg(unix)]
-use std::os::unix::fs::{MetadataExt, PermissionsExt};
+use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt};
 
 pub fn init(i: &mut Isolate, s: &State) {
   i.register_op("op_open", s.stateful_json_op(op_open));
@@ -50,7 +50,8 @@ struct OpenArgs {
   promise_id: Option<u64>,
   path: String,
   options: Option<OpenOptions>,
-  mode: Option<String>,
+  open_mode: Option<String>,
+  mode: Option<u32>,
 }
 
 #[derive(Deserialize, Default, Debug)]
@@ -73,7 +74,20 @@ fn op_open(
   let args: OpenArgs = serde_json::from_value(args)?;
   let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
   let state_ = state.clone();
-  let mut open_options = tokio::fs::OpenOptions::new();
+
+  let mut open_options = if let Some(mode) = args.mode {
+    #[allow(unused_mut)]
+    let mut std_options = fs::OpenOptions::new();
+    // mode only used if creating the file on Unix
+    // if not specified, defaults to 0o666
+    #[cfg(unix)]
+    std_options.mode(mode & 0o777);
+    #[cfg(not(unix))]
+    let _ = mode; // avoid unused warning
+    tokio::fs::OpenOptions::from(std_options)
+  } else {
+    tokio::fs::OpenOptions::new()
+  };
 
   if let Some(options) = args.options {
     if options.read {
@@ -91,9 +105,9 @@ fn op_open(
       .truncate(options.truncate)
       .append(options.append)
       .create_new(options.create_new);
-  } else if let Some(mode) = args.mode {
-    let mode = mode.as_ref();
-    match mode {
+  } else if let Some(open_mode) = args.open_mode {
+    let open_mode = open_mode.as_ref();
+    match open_mode {
       "r" => {
         state.check_read(&path)?;
       }
@@ -106,7 +120,7 @@ fn op_open(
       }
     };
 
-    match mode {
+    match open_mode {
       "r" => {
         open_options.read(true);
       }
@@ -142,7 +156,7 @@ fn op_open(
     }
   } else {
     return Err(OpError::other(
-      "Open requires either mode or options.".to_string(),
+      "Open requires either openMode or options.".to_string(),
     ));
   };