Bug 1654295: Reject early when relative paths are used in IOUtils methods r=barret,Gijs

This patch introduces a new `REJECT_IF_RELATIVE_PATH` macro for use in
`IOUtils`. Its usage ensures that every method rejects in the same way when
unsupported relative paths are passed as parameters to `IOUtils` public
methods.

Differential Revision: https://phabricator.services.mozilla.com/D84731

dom/system/IOUtils.cpp

@@ -50,6 +50,18 @@
     }                                                             \
   } while (false)
 
+#define REJECT_IF_RELATIVE_PATH(aPath, aJSPromise)                    \
+  do {                                                                \
+    if (!IsAbsolutePath(aPath)) {                                     \
+      (aJSPromise)                                                    \
+          ->MaybeRejectWithOperationError(nsPrintfCString(            \
+              "Refusing to work with path(%s) because only absolute " \
+              "file paths are permitted",                             \
+              NS_ConvertUTF16toUTF8(aPath).get()));                   \
+      return (aJSPromise).forget();                                   \
+    }                                                                 \
+  } while (false)
+
 namespace mozilla {
 namespace dom {
 
@@ -154,18 +166,14 @@ already_AddRefed<Promise> IOUtils::Read(GlobalObject& aGlobal,
                                         const Optional<uint32_t>& aMaxBytes) {
   RefPtr<Promise> promise = CreateJSPromise(aGlobal);
   NS_ENSURE_TRUE(!!promise, nullptr);
+
   REJECT_IF_SHUTTING_DOWN(promise);
+  REJECT_IF_RELATIVE_PATH(aPath, promise);
 
   RefPtr<nsISerialEventTarget> bg = GetBackgroundEventTarget();
   REJECT_IF_NULL_EVENT_TARGET(bg, promise);
 
   // Process arguments.
-  if (!IsAbsolutePath(aPath)) {
-    promise->MaybeRejectWithOperationError(
-        "Only absolute file paths are permitted");
-    return promise.forget();
-  }
-
   uint32_t toRead = 0;
   if (aMaxBytes.WasPassed()) {
     toRead = aMaxBytes.Value();
@@ -255,17 +263,14 @@ already_AddRefed<Promise> IOUtils::WriteAtomic(
     const WriteAtomicOptions& aOptions) {
   RefPtr<Promise> promise = CreateJSPromise(aGlobal);
   NS_ENSURE_TRUE(!!promise, nullptr);
+
   REJECT_IF_SHUTTING_DOWN(promise);
+  REJECT_IF_RELATIVE_PATH(aPath, promise);
 
   RefPtr<nsISerialEventTarget> bg = GetBackgroundEventTarget();
   REJECT_IF_NULL_EVENT_TARGET(bg, promise);
 
   // Process arguments.
-  if (!IsAbsolutePath(aPath)) {
-    promise->MaybeRejectWithOperationError(
-        "Only absolute file paths are permitted");
-    return promise.forget();
-  }
   aData.ComputeState();
   FallibleTArray<uint8_t> toWrite;
   if (!toWrite.InsertElementsAt(0, aData.Data(), aData.Length(), fallible)) {
@@ -391,7 +396,10 @@ already_AddRefed<Promise> IOUtils::Move(GlobalObject& aGlobal,
                                         const MoveOptions& aOptions) {
   RefPtr<Promise> promise = CreateJSPromise(aGlobal);
   NS_ENSURE_TRUE(!!promise, nullptr);
+
   REJECT_IF_SHUTTING_DOWN(promise);
+  REJECT_IF_RELATIVE_PATH(aSourcePath, promise);
+  REJECT_IF_RELATIVE_PATH(aDestPath, promise);
 
   RefPtr<nsISerialEventTarget> bg = GetBackgroundEventTarget();
   REJECT_IF_NULL_EVENT_TARGET(bg, promise);
@@ -463,19 +471,14 @@ already_AddRefed<Promise> IOUtils::Remove(GlobalObject& aGlobal,
                                           const nsAString& aPath,
                                           const RemoveOptions& aOptions) {
   RefPtr<Promise> promise = CreateJSPromise(aGlobal);
-  REJECT_IF_SHUTTING_DOWN(promise);
+  NS_ENSURE_TRUE(!!promise, nullptr);
+
+  REJECT_IF_SHUTTING_DOWN(promise);
+  REJECT_IF_RELATIVE_PATH(aPath, promise);
 
-  // Do the IO on a background thread and return the result to this thread.
   RefPtr<nsISerialEventTarget> bg = GetBackgroundEventTarget();
   REJECT_IF_NULL_EVENT_TARGET(bg, promise);
 
-  // Process arguments.
-  if (!IsAbsolutePath(aPath)) {
-    promise->MaybeRejectWithOperationError(
-        "Only absolute file paths are permitted");
-    return promise.forget();
-  }
-
   InvokeAsync(bg, __func__,
               [path = nsAutoString(aPath), aOptions]() {
                 nsresult rv = RemoveSync(path, aOptions.mIgnoreAbsent,
@@ -521,19 +524,14 @@ already_AddRefed<Promise> IOUtils::MakeDirectory(
     GlobalObject& aGlobal, const nsAString& aPath,
     const MakeDirectoryOptions& aOptions) {
   RefPtr<Promise> promise = CreateJSPromise(aGlobal);
-  REJECT_IF_SHUTTING_DOWN(promise);
+  NS_ENSURE_TRUE(!!promise, nullptr);
+
+  REJECT_IF_SHUTTING_DOWN(promise);
+  REJECT_IF_RELATIVE_PATH(aPath, promise);
 
-  // Do the IO on a background thread and return the result to this thread.
   RefPtr<nsISerialEventTarget> bg = GetBackgroundEventTarget();
   REJECT_IF_NULL_EVENT_TARGET(bg, promise);
 
-  // Process arguments.
-  if (!IsAbsolutePath(aPath)) {
-    promise->MaybeRejectWithOperationError(
-        "Only absolute file paths are permitted");
-    return promise.forget();
-  }
-
   InvokeAsync(bg, __func__,
               [path = nsAutoString(aPath), aOptions]() {
                 nsresult rv = CreateDirectorySync(
@@ -579,19 +577,14 @@ already_AddRefed<Promise> IOUtils::MakeDirectory(
 already_AddRefed<Promise> IOUtils::Stat(GlobalObject& aGlobal,
                                         const nsAString& aPath) {
   RefPtr<Promise> promise = CreateJSPromise(aGlobal);
-  REJECT_IF_SHUTTING_DOWN(promise);
+  NS_ENSURE_TRUE(!!promise, nullptr);
+
+  REJECT_IF_SHUTTING_DOWN(promise);
+  REJECT_IF_RELATIVE_PATH(aPath, promise);
 
-  // Do the IO on a background thread and return the result to this thread.
   RefPtr<nsISerialEventTarget> bg = GetBackgroundEventTarget();
   REJECT_IF_NULL_EVENT_TARGET(bg, promise);
 
-  // Process arguments.
-  if (!IsAbsolutePath(aPath)) {
-    promise->MaybeRejectWithOperationError(
-        "Only absolute file paths are permitted");
-    return promise.forget();
-  }
-
   InvokeAsync(
       bg, __func__,
       [path = nsAutoString(aPath)]() {
@@ -1004,3 +997,4 @@ NS_IMETHODIMP IOUtilsShutdownBlocker::GetState(nsIPropertyBag** aState) {
 
 #undef REJECT_IF_NULL_EVENT_TARGET
 #undef REJECT_IF_SHUTTING_DOWN
+#undef REJECT_IF_RELATIVE_PATH

dom/system/tests/test_ioutils.html

@@ -388,7 +388,7 @@
       info("Test writing a file at a relative destination");
       await Assert.rejects(
         window.IOUtils.writeAtomic(tmpFileName, bytes),
-        /Only absolute file paths are permitted/,
+        /Refusing to work with path\(.*\) because only absolute file paths are permitted/,
         "IOUtils::writeAtomic only works with absolute paths"
       );
     });
@@ -399,7 +399,7 @@
       info("Test reading a file at a relative destination");
       await Assert.rejects(
         window.IOUtils.read(tmpFileName),
-        /Only absolute file paths are permitted/,
+        /Refusing to work with path\(.*\) because only absolute file paths are permitted/,
         "IOUtils::writeAtomic only works with absolute paths"
       );
     });
@@ -412,7 +412,7 @@
       info("Test moving a file to a relative destination");
       await Assert.rejects(
         window.IOUtils.move(tmpFileName, dest),
-        /Only absolute file paths are permitted/,
+        /Refusing to work with path\(.*\) because only absolute file paths are permitted/,
         "IOUtils::move only works with absolute paths"
       );
       ok(