From f9557a4ff6b73a4af37e713bb6b2294253c7b230 Mon Sep 17 00:00:00 2001 From: dubiousjim Date: Mon, 16 Mar 2020 15:02:41 -0400 Subject: [PATCH] Add mode option to open/create (#4289) --- cli/js/files.ts | 22 +++++++++++++--------- cli/js/lib.deno.ns.d.ts | 18 +++++++++++------- cli/js/ops/fs/open.ts | 14 +++++++++++--- cli/js/tests/files_test.ts | 38 ++++++++++++++++++++++++++++++++++++++ cli/ops/fs.rs | 30 ++++++++++++++++++++++-------- 5 files changed, 95 insertions(+), 27 deletions(-) diff --git a/cli/js/files.ts b/cli/js/files.ts index b9da1363f0..99ae198797 100644 --- a/cli/js/files.ts +++ b/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; -export async function open(path: string, mode?: OpenMode): Promise; +export async function open(path: string, openMode?: OpenMode): Promise; + +/**@internal*/ export async function open( path: string, modeOrOptions: OpenOptions | OpenMode = "r" ): Promise { - 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); } diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index e700c286e0..6453b4ab82 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/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; @@ -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; + export function open(path: string, openMode?: OpenMode): Promise; /** 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 | * |------|--------------------------------------------------------------------------------------------------| diff --git a/cli/js/ops/fs/open.ts b/cli/js/ops/fs/open.ts index 4c9281909e..2ac5c715cd 100644 --- a/cli/js/ops/fs/open.ts +++ b/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 { + const mode: number | undefined = options?.mode; return sendAsync("op_open", { path, options, + openMode, mode }); } diff --git a/cli/js/tests/files_test.ts b/cli/js/tests/files_test.ts index 29e2812d48..b087d7398d 100644 --- a/cli/js/tests/files_test.ts +++ b/cli/js/tests/files_test.ts @@ -74,6 +74,44 @@ unitTest(async function readerToAsyncIterator(): Promise { 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 { + 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 { diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs index 6c978d51cf..d163c43060 100644 --- a/cli/ops/fs.rs +++ b/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, path: String, options: Option, - mode: Option, + open_mode: Option, + mode: Option, } #[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(), )); };