fix(runtime): better error message with Deno.env.get/set (#15966)

denoland · Sep 21, 2022 · cc32a29 · cc32a29
1 parent 35fe9ee
commit cc32a29
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 6 deletions.
diff --git a/cli/tests/unit/os_test.ts b/cli/tests/unit/os_test.ts
@@ -116,6 +116,40 @@ Deno.test(
   },
 );
 
+Deno.test({ permissions: { env: true } }, function envInvalidChars() {
+  assertThrows(() => Deno.env.get(""), TypeError, "Key is an empty string");
+  assertThrows(
+    () => Deno.env.get("\0"),
+    TypeError,
+    'Key contains invalid characters: "\\0"',
+  );
+  assertThrows(
+    () => Deno.env.get("="),
+    TypeError,
+    'Key contains invalid characters: "="',
+  );
+  assertThrows(
+    () => Deno.env.set("", "foo"),
+    TypeError,
+    "Key is an empty string",
+  );
+  assertThrows(
+    () => Deno.env.set("\0", "foo"),
+    TypeError,
+    'Key contains invalid characters: "\\0"',
+  );
+  assertThrows(
+    () => Deno.env.set("=", "foo"),
+    TypeError,
+    'Key contains invalid characters: "="',
+  );
+  assertThrows(
+    () => Deno.env.set("foo", "\0"),
+    TypeError,
+    'Value contains invalid characters: "\\0"',
+  );
+});
+
 Deno.test(function osPid() {
   assert(Deno.pid > 0);
 });

diff --git a/runtime/ops/os.rs b/runtime/ops/os.rs
@@ -80,10 +80,20 @@ fn op_set_env(
   value: String,
 ) -> Result<(), AnyError> {
   state.borrow_mut::<Permissions>().env.check(&key)?;
-  let invalid_key = key.is_empty() || key.contains(&['=', '\0'] as &[char]);
-  let invalid_value = value.contains('\0');
-  if invalid_key || invalid_value {
-    return Err(type_error("Key or value contains invalid characters."));
+  if key.is_empty() {
+    return Err(type_error("Key is an empty string."));
+  }
+  if key.contains(&['=', '\0'] as &[char]) {
+    return Err(type_error(format!(
+      "Key contains invalid characters: {:?}",
+      key
+    )));
+  }
+  if value.contains('\0') {
+    return Err(type_error(format!(
+      "Value contains invalid characters: {:?}",
+      value
+    )));
   }
   env::set_var(key, value);
   Ok(())
@@ -108,9 +118,17 @@ fn op_get_env(
     state.borrow_mut::<Permissions>().env.check(&key)?;
   }
 
-  if key.is_empty() || key.contains(&['=', '\0'] as &[char]) {
-    return Err(type_error("Key contains invalid characters."));
+  if key.is_empty() {
+    return Err(type_error("Key is an empty string."));
   }
+
+  if key.contains(&['=', '\0'] as &[char]) {
+    return Err(type_error(format!(
+      "Key contains invalid characters: {:?}",
+      key
+    )));
+  }
+
   let r = match env::var(key) {
     Err(env::VarError::NotPresent) => None,
     v => Some(v?),