From cc32a297da2d92983573a1cea1ca669d6139d77d Mon Sep 17 00:00:00 2001
From: Yoshiya Hinosawa <stibium121@gmail.com>
Date: Wed, 21 Sep 2022 15:18:58 +0900
Subject: [PATCH] fix(runtime): better error message with Deno.env.get/set
 (#15966)

---
 cli/tests/unit/os_test.ts | 34 ++++++++++++++++++++++++++++++++++
 runtime/ops/os.rs         | 30 ++++++++++++++++++++++++------
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/cli/tests/unit/os_test.ts b/cli/tests/unit/os_test.ts
index 3564ffa471..bdbf7f0ca5 100644
--- 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
index 21a94b0fbd..70d4298397 100644
--- 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?),