getenv: Make os.getenv() show a better error

.. when an associated value is not a quoted string.
This includes some cases where it would previously return an
integer, a CPython incompatibility.

However, it's an incompatible behavior change with circuitpython
since previously a number would be returned.

Closes: #9113

Signed-off-by: Jeff Epler <[email protected]>

Author: jepler

locale/circuitpython.pot

@@ -960,6 +960,10 @@ msgstr ""
 msgid "Expected a kind of %q"
 msgstr ""
 
+#: shared-module/os/getenv.c
+msgid "Expected quoted string"
+msgstr ""
+
 #: ports/espressif/common-hal/_bleio/Adapter.c
 #: ports/nordic/common-hal/_bleio/Adapter.c
 msgid "Extended advertisements with scan response not supported."

shared-bindings/os/__init__.h

@@ -15,7 +15,6 @@
 void common_hal_os_chdir(const char *path);
 mp_obj_t common_hal_os_getcwd(void);
 mp_obj_t common_hal_os_getenv(const char *key, mp_obj_t default_);
-mp_obj_t common_hal_os_getenv_path(const char *path, const char *key, mp_obj_t default_);
 
 mp_obj_t common_hal_os_listdir(const char *path);
 void common_hal_os_mkdir(const char *path);

shared-module/os/getenv.c

@@ -365,7 +365,7 @@ os_getenv_err_t common_hal_os_getenv_str(const char *key, char *value, size_t va
     return result;
 }
 
-mp_obj_t common_hal_os_getenv_path(const char *path, const char *key, mp_obj_t default_) {
+static mp_obj_t common_hal_os_getenv_path(const char *path, const char *key, mp_obj_t default_) {
     vstr_t buf;
     bool quoted;
 
@@ -379,7 +379,7 @@ mp_obj_t common_hal_os_getenv_path(const char *path, const char *key, mp_obj_t d
     if (quoted) {
         return mp_obj_new_str_from_vstr(&buf);
     } else {
-        return mp_parse_num_integer(buf.buf, buf.len, 0, NULL);
+        mp_raise_ValueError(MP_ERROR_TEXT("Expected quoted string"));
     }
 }
 

tests/circuitpython/getenv.py

@@ -38,17 +38,17 @@ def ioctl(self, op, arg):
 content_good = b"""
 # comment
 key0 = "hello world"
-key1 = 7
 key2= "\\n"
 key3 ="\\u00c1x"
 key4 = "\\U000000c1x"
 key5 = "\\f\\"\\\\"
 key6 = "\\t\\r\\b"
 key7 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 key8 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
- key9 = "hello comment" # comment
-  key10 = 0x7f # comment
-key11 = 0
+ key1 = "hello comment" # comment
+ikey0 = 7
+  ikey1 = 0x7f # comment
+ikey2 = 0
 [section]
 subvalue = "hi"
 """
@@ -58,35 +58,55 @@ def ioctl(self, op, arg):
     b'key = """\n',
     b"key =\n",
     b'key="',
+    b'key="\0',
     b"key = strings must be quoted\n",
+    b"key=7",
+    # These all return None even though some of them look a bit like they might define key...
+    b"",
+    b"key",
+    b"key ",
+    b"key\0",
+    b'key\0="x"',
+    b"\n",
 ]
 
 
-def run_test(key, content):
+def run_test(key, content, getter=os.getenv):
     with open("/settings.toml", "wb") as f:
         f.write(content)
 
     try:
-        v = os.getenv(key)
+        v = getter(key)
         print(key, repr(v))
     except Exception as e:
         print(key, str(e))
 
 
+print("*** Tests that should succeed ***")
 run_test("key", b"")
 
-for i in range(13):
+for i in range(10):
     run_test(f"key{i}", content_good)
+for i in range(4):
+    run_test(f"ikey{i}", content_good, getenv_int)
 
+# Run the test again but on CRLF content
 content_good = content_good.replace(b"\n", b"\r\n")
-for i in range(13):
+for i in range(10):
     run_test(f"key{i}", content_good)
+for i in range(4):
+    run_test(f"ikey{i}", content_good, getenv_int)
 
-run_test(f"K", b"K = 7\r\n")
-print(getenv_int("K"))
+print()
+print("*** Integer tests that should succeed***")
+run_test(f"K", b"K = 7\r\n", getenv_int)
 
 # Test value without trailing newline
-run_test(f"noeol", b"noeol=3")
+run_test(f"noeol", b"noeol=3", getenv_int)
 
+print("Tests that should fail")
 for content in content_bad:
     run_test("key", content)
+
+# (note that getenv_int displays additional info on output)
+run_test(f"K", b'K = "7"\r\n', getenv_int)

tests/circuitpython/getenv.py.exp

@@ -1,35 +1,51 @@
+*** Tests that should succeed ***
 key None
 key0 'hello world'
-key1 7
+key1 'hello comment'
 key2 '\n'
 key3 'Áx'
 key4 'Áx'
 key5 '\x0c"\\'
 key6 '\t\r\x08'
 key7 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
 key8 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
-key9 'hello comment'
-key10 127
-key11 0
-key12 None
+key9 None
+ikey0 7
+ikey1 127
+ikey2 0
+ikey3 None
 key0 'hello world'
-key1 7
+key1 'hello comment'
 key2 '\n'
 key3 'Áx'
 key4 'Áx'
 key5 '\x0c"\\'
 key6 '\t\r\x08'
 key7 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
 key8 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
-key9 'hello comment'
-key10 127
-key11 0
-key12 None
+key9 None
+ikey0 7
+ikey1 127
+ikey2 0
+ikey3 None
+
+*** Integer tests that should succeed***
 K 7
-7
 noeol 3
+Tests that should fail
 key Invalid byte '\n'
 key Invalid byte '"'
-key invalid syntax for integer with base 10: ''
+key Expected quoted string
 key Invalid byte 'EOF'
-key invalid syntax for integer with base 10: 'strings must be quoted'
+key Invalid byte 'EOF'
+key Expected quoted string
+key Expected quoted string
+key None
+key None
+key None
+key None
+key None
+key None
+An error occurred while retrieving 'K':
+Invalid byte '"'
+K None