Clarify writeFile options and avoid unexpected perm modification (#1643)

js/chmod_test.ts

@@ -9,7 +9,7 @@ testPerm({ write: true }, function chmodSyncSuccess() {
   const data = enc.encode("Hello");
   const tempDir = deno.makeTempDirSync();
   const filename = tempDir + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
 
   // On windows no effect, but should not crash
   deno.chmodSync(filename, 0o777);
@@ -29,7 +29,7 @@ if (isNotWindows) {
     const tempDir = deno.makeTempDirSync();
 
     const filename = tempDir + "/test.txt";
-    deno.writeFileSync(filename, data, 0o666);
+    deno.writeFileSync(filename, data, { perm: 0o666 });
     const symlinkName = tempDir + "/test_symlink.txt";
     deno.symlinkSync(filename, symlinkName);
 
@@ -74,7 +74,7 @@ testPerm({ write: true }, async function chmodSuccess() {
   const data = enc.encode("Hello");
   const tempDir = deno.makeTempDirSync();
   const filename = tempDir + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
 
   // On windows no effect, but should not crash
   await deno.chmod(filename, 0o777);
@@ -94,7 +94,7 @@ if (isNotWindows) {
     const tempDir = deno.makeTempDirSync();
 
     const filename = tempDir + "/test.txt";
-    deno.writeFileSync(filename, data, 0o666);
+    deno.writeFileSync(filename, data, { perm: 0o666 });
     const symlinkName = tempDir + "/test_symlink.txt";
     deno.symlinkSync(filename, symlinkName);
 

js/copy_file_test.ts

@@ -11,7 +11,7 @@ function readFileString(filename: string): string {
 function writeFileString(filename: string, s: string) {
   const enc = new TextEncoder();
   const data = enc.encode(s);
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
 }
 
 function assertSameContent(filename1: string, filename2: string) {

js/metrics_test.ts

@@ -27,7 +27,7 @@ testPerm({ write: true }, function metricsUpdatedIfNoResponseSync() {
   const filename = deno.makeTempDirSync() + "/test.txt";
 
   const data = new Uint8Array([41, 42, 43]);
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
 
   const metrics = deno.metrics();
   assert(metrics.opsDispatched === metrics.opsCompleted);
@@ -37,7 +37,7 @@ testPerm({ write: true }, async function metricsUpdatedIfNoResponseAsync() {
   const filename = deno.makeTempDirSync() + "/test.txt";
 
   const data = new Uint8Array([41, 42, 43]);
-  await deno.writeFile(filename, data, 0o666);
+  await deno.writeFile(filename, data, { perm: 0o666 });
 
   const metrics = deno.metrics();
   assert(metrics.opsDispatched === metrics.opsCompleted);

js/remove_test.ts

@@ -28,7 +28,7 @@ testPerm({ write: true }, function removeSyncFileSuccess() {
   const enc = new TextEncoder();
   const data = enc.encode("Hello");
   const filename = deno.makeTempDirSync() + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
   const fileInfo = deno.statSync(filename);
   assert(fileInfo.isFile()); // check exist first
   deno.removeSync(filename); // remove
@@ -129,7 +129,7 @@ testPerm({ write: true }, function removeAllSyncFileSuccess() {
   const enc = new TextEncoder();
   const data = enc.encode("Hello");
   const filename = deno.makeTempDirSync() + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
   const fileInfo = deno.statSync(filename);
   assert(fileInfo.isFile()); // check exist first
   deno.removeSync(filename, { recursive: true }); // remove
@@ -195,7 +195,7 @@ testPerm({ write: true }, async function removeFileSuccess() {
   const enc = new TextEncoder();
   const data = enc.encode("Hello");
   const filename = deno.makeTempDirSync() + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
   const fileInfo = deno.statSync(filename);
   assert(fileInfo.isFile()); // check exist first
   await deno.remove(filename); // remove
@@ -295,7 +295,7 @@ testPerm({ write: true }, async function removeAllFileSuccess() {
   const enc = new TextEncoder();
   const data = enc.encode("Hello");
   const filename = deno.makeTempDirSync() + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data, { perm: 0o666 });
   const fileInfo = deno.statSync(filename);
   assert(fileInfo.isFile()); // check exist first
   await deno.remove(filename, { recursive: true }); // remove

js/write_file.ts

@@ -3,6 +3,17 @@ import * as msg from "gen/msg_generated";
 import * as flatbuffers from "./flatbuffers";
 import * as dispatch from "./dispatch";
 
+/** Options for writing to a file.
+ * `perm` would change the file's permission if set.
+ * `create` decides if the file should be created if not exists (default: true)
+ * `append` decides if the file should be appended (default: false)
+ */
+export interface WriteFileOptions {
+  perm?: number;
+  create?: boolean;
+  append?: boolean;
+}
+
 /** Write a new file, with given filename and data synchronously.
  *
  *       import { writeFileSync } from "deno";
@@ -14,9 +25,9 @@ import * as dispatch from "./dispatch";
 export function writeFileSync(
   filename: string,
   data: Uint8Array,
-  perm = 0o666
+  options: WriteFileOptions = {}
 ): void {
-  dispatch.sendSync(...req(filename, data, perm));
+  dispatch.sendSync(...req(filename, data, options));
 }
 
 /** Write a new file, with given filename and data.
@@ -30,21 +41,35 @@ export function writeFileSync(
 export async function writeFile(
   filename: string,
   data: Uint8Array,
-  perm = 0o666
+  options: WriteFileOptions = {}
 ): Promise<void> {
-  await dispatch.sendAsync(...req(filename, data, perm));
+  await dispatch.sendAsync(...req(filename, data, options));
 }
 
 function req(
   filename: string,
   data: Uint8Array,
-  perm: number
+  options: WriteFileOptions
 ): [flatbuffers.Builder, msg.Any, flatbuffers.Offset, Uint8Array] {
   const builder = flatbuffers.createBuilder();
   const filename_ = builder.createString(filename);
   msg.WriteFile.startWriteFile(builder);
   msg.WriteFile.addFilename(builder, filename_);
-  msg.WriteFile.addPerm(builder, perm);
+  // Perm is not updated by default
+  if (options.perm !== undefined && options.perm !== null) {
+    msg.WriteFile.addUpdatePerm(builder, true);
+    msg.WriteFile.addPerm(builder, options.perm!);
+  } else {
+    msg.WriteFile.addUpdatePerm(builder, false);
+    msg.WriteFile.addPerm(builder, 0o666);
+  }
+  // Create is turned on by default
+  if (options.create !== undefined) {
+    msg.WriteFile.addIsCreate(builder, !!options.create);
+  } else {
+    msg.WriteFile.addIsCreate(builder, true);
+  }
+  msg.WriteFile.addIsAppend(builder, !!options.append);
   const inner = msg.WriteFile.endWriteFile(builder);
   return [builder, msg.Any.WriteFile, inner, data];
 }

js/write_file_test.ts

@@ -6,7 +6,7 @@ testPerm({ write: true }, function writeFileSyncSuccess() {
   const enc = new TextEncoder();
   const data = enc.encode("Hello");
   const filename = deno.makeTempDirSync() + "/test.txt";
-  deno.writeFileSync(filename, data, 0o666);
+  deno.writeFileSync(filename, data);
   const dataRead = deno.readFileSync(filename);
   const dec = new TextDecoder("utf-8");
   const actual = dec.decode(dataRead);
@@ -45,11 +45,69 @@ testPerm({ write: false }, function writeFileSyncPerm() {
   assert(caughtError);
 });
 
+testPerm({ write: true }, function writeFileSyncUpdatePerm() {
+  if (deno.platform.os !== "win") {
+    const enc = new TextEncoder();
+    const data = enc.encode("Hello");
+    const filename = deno.makeTempDirSync() + "/test.txt";
+    deno.writeFileSync(filename, data, { perm: 0o755 });
+    assertEqual(deno.statSync(filename).mode & 0o777, 0o755);
+    deno.writeFileSync(filename, data, { perm: 0o666 });
+    assertEqual(deno.statSync(filename).mode & 0o777, 0o666);
+  }
+});
+
+testPerm({ write: true }, function writeFileSyncCreate() {
+  const enc = new TextEncoder();
+  const data = enc.encode("Hello");
+  const filename = deno.makeTempDirSync() + "/test.txt";
+  let caughtError = false;
+  // if create turned off, the file won't be created
+  try {
+    deno.writeFileSync(filename, data, { create: false });
+  } catch (e) {
+    caughtError = true;
+    assertEqual(e.kind, deno.ErrorKind.NotFound);
+    assertEqual(e.name, "NotFound");
+  }
+  assert(caughtError);
+
+  // Turn on create, should have no error
+  deno.writeFileSync(filename, data, { create: true });
+  deno.writeFileSync(filename, data, { create: false });
+  const dataRead = deno.readFileSync(filename);
+  const dec = new TextDecoder("utf-8");
+  const actual = dec.decode(dataRead);
+  assertEqual("Hello", actual);
+});
+
+testPerm({ write: true }, function writeFileSyncAppend() {
+  const enc = new TextEncoder();
+  const data = enc.encode("Hello");
+  const filename = deno.makeTempDirSync() + "/test.txt";
+  deno.writeFileSync(filename, data);
+  deno.writeFileSync(filename, data, { append: true });
+  let dataRead = deno.readFileSync(filename);
+  const dec = new TextDecoder("utf-8");
+  let actual = dec.decode(dataRead);
+  assertEqual("HelloHello", actual);
+  // Now attempt overwrite
+  deno.writeFileSync(filename, data, { append: false });
+  dataRead = deno.readFileSync(filename);
+  actual = dec.decode(dataRead);
+  assertEqual("Hello", actual);
+  // append not set should also overwrite
+  deno.writeFileSync(filename, data);
+  dataRead = deno.readFileSync(filename);
+  actual = dec.decode(dataRead);
+  assertEqual("Hello", actual);
+});
+
 testPerm({ write: true }, async function writeFileSuccess() {
   const enc = new TextEncoder();
   const data = enc.encode("Hello");
   const filename = deno.makeTempDirSync() + "/test.txt";
-  await deno.writeFile(filename, data, 0o666);
+  await deno.writeFile(filename, data);
   const dataRead = deno.readFileSync(filename);
   const dec = new TextDecoder("utf-8");
   const actual = dec.decode(dataRead);
@@ -87,3 +145,61 @@ testPerm({ write: false }, async function writeFilePerm() {
   }
   assert(caughtError);
 });
+
+testPerm({ write: true }, async function writeFileUpdatePerm() {
+  if (deno.platform.os !== "win") {
+    const enc = new TextEncoder();
+    const data = enc.encode("Hello");
+    const filename = deno.makeTempDirSync() + "/test.txt";
+    await deno.writeFile(filename, data, { perm: 0o755 });
+    assertEqual(deno.statSync(filename).mode & 0o777, 0o755);
+    await deno.writeFile(filename, data, { perm: 0o666 });
+    assertEqual(deno.statSync(filename).mode & 0o777, 0o666);
+  }
+});
+
+testPerm({ write: true }, async function writeFileCreate() {
+  const enc = new TextEncoder();
+  const data = enc.encode("Hello");
+  const filename = deno.makeTempDirSync() + "/test.txt";
+  let caughtError = false;
+  // if create turned off, the file won't be created
+  try {
+    await deno.writeFile(filename, data, { create: false });
+  } catch (e) {
+    caughtError = true;
+    assertEqual(e.kind, deno.ErrorKind.NotFound);
+    assertEqual(e.name, "NotFound");
+  }
+  assert(caughtError);
+
+  // Turn on create, should have no error
+  await deno.writeFile(filename, data, { create: true });
+  await deno.writeFile(filename, data, { create: false });
+  const dataRead = deno.readFileSync(filename);
+  const dec = new TextDecoder("utf-8");
+  const actual = dec.decode(dataRead);
+  assertEqual("Hello", actual);
+});
+
+testPerm({ write: true }, async function writeFileAppend() {
+  const enc = new TextEncoder();
+  const data = enc.encode("Hello");
+  const filename = deno.makeTempDirSync() + "/test.txt";
+  await deno.writeFile(filename, data);
+  await deno.writeFile(filename, data, { append: true });
+  let dataRead = deno.readFileSync(filename);
+  const dec = new TextDecoder("utf-8");
+  let actual = dec.decode(dataRead);
+  assertEqual("HelloHello", actual);
+  // Now attempt overwrite
+  await deno.writeFile(filename, data, { append: false });
+  dataRead = deno.readFileSync(filename);
+  actual = dec.decode(dataRead);
+  assertEqual("Hello", actual);
+  // append not set should also overwrite
+  await deno.writeFile(filename, data);
+  dataRead = deno.readFileSync(filename);
+  actual = dec.decode(dataRead);
+  assertEqual("Hello", actual);
+});

src/fs.rs

@@ -18,16 +18,29 @@ pub fn write_file<T: AsRef<[u8]>>(
   data: T,
   perm: u32,
 ) -> std::io::Result<()> {
-  let is_append = perm & (1 << 31) != 0;
+  write_file_2(filename, data, true, perm, true, false)
+}
+
+pub fn write_file_2<T: AsRef<[u8]>>(
+  filename: &Path,
+  data: T,
+  update_perm: bool,
+  perm: u32,
+  is_create: bool,
+  is_append: bool,
+) -> std::io::Result<()> {
   let mut file = OpenOptions::new()
     .read(false)
     .write(true)
     .append(is_append)
     .truncate(!is_append)
-    .create(true)
+    .create(is_create)
     .open(filename)?;
 
-  set_permissions(&mut file, perm)?;
+  if update_perm {
+    set_permissions(&mut file, perm)?;
+  }
+
   file.write_all(data.as_ref())
 }
 

src/msg.fbs

@@ -295,8 +295,11 @@ table ReadDirRes {
 table WriteFile {
   filename: string;
   data: [ubyte];
+  update_perm: bool;
   perm: uint;
   // perm specified by https://godoc.org/os#FileMode
+  is_create: bool;
+  is_append: bool;
 }
 
 table CopyFile {

src/ops.rs

@@ -1058,7 +1058,10 @@ fn op_write_file(
 ) -> Box<Op> {
   let inner = base.inner_as_write_file().unwrap();
   let filename = String::from(inner.filename().unwrap());
+  let update_perm = inner.update_perm();
   let perm = inner.perm();
+  let is_create = inner.is_create();
+  let is_append = inner.is_append();
 
   if let Err(e) = state.check_write(&filename) {
     return odd_future(e);
@@ -1066,7 +1069,14 @@ fn op_write_file(
 
   blocking(base.sync(), move || -> OpResult {
     debug!("op_write_file {} {}", filename, data.len());
-    deno_fs::write_file(Path::new(&filename), data, perm)?;
+    deno_fs::write_file_2(
+      Path::new(&filename),
+      data,
+      update_perm,
+      perm,
+      is_create,
+      is_append,
+    )?;
     Ok(empty_buf())
   })
 }