diff --git a/lib/pure/includes/osenv.nim b/lib/pure/includes/osenv.nim index 172370f035fa..6aaafbfdaf80 100644 --- a/lib/pure/includes/osenv.nim +++ b/lib/pure/includes/osenv.nim @@ -45,6 +45,7 @@ else: importc: "getenv", header: "".} when defined(windows): proc c_putenv_s(envname: cstring, envval: cstring): cint {.importc: "_putenv_s", header: "".} + from std/private/win_setenv import setEnvImpl else: proc c_setenv(envname: cstring, envval: cstring, overwrite: cint): cint {.importc: "setenv", header: "".} proc c_unsetenv(env: cstring): cint {.importc: "unsetenv", header: "".} @@ -83,8 +84,7 @@ else: return c_getenv(key) != nil - proc putEnv*(key, val: string) {.tags: [ - WriteEnvEffect].} = + proc putEnv*(key, val: string) {.tags: [WriteEnvEffect].} = ## Sets the value of the `environment variable`:idx: named `key` to `val`. ## If an error occurs, `OSError` is raised. ## @@ -93,11 +93,14 @@ else: ## * `existsEnv proc <#existsEnv,string>`_ ## * `delEnv proc <#delEnv,string>`_ ## * `envPairs iterator <#envPairs.i>`_ - template bail = raiseOSError(osLastError(), $(key, val)) when defined(windows): - if c_putenv_s(key, val) != 0'i32: bail + if key.len == 0 or '=' in key: + raise newException(OSError, "invalid key, got: " & $(key, val)) + if setEnvImpl(key, val, 1'i32) != 0'i32: + raiseOSError(osLastError(), $(key, val)) else: - if c_setenv(key, val, 1'i32) != 0'i32: bail + if c_setenv(key, val, 1'i32) != 0'i32: + raiseOSError(osLastError(), $(key, val)) proc delEnv*(key: string) {.tags: [WriteEnvEffect].} = ## Deletes the `environment variable`:idx: named `key`. @@ -115,6 +118,8 @@ else: > You can remove a variable from the environment by specifying an empty string (that is, "") for value_string note that nil is not legal ]# + if key.len == 0 or '=' in key: + raise newException(OSError, "invalid key, got: " & key) if c_putenv_s(key, "") != 0'i32: bail else: if c_unsetenv(key) != 0'i32: bail diff --git a/lib/std/private/win_setenv.nim b/lib/std/private/win_setenv.nim new file mode 100644 index 000000000000..067e656a317f --- /dev/null +++ b/lib/std/private/win_setenv.nim @@ -0,0 +1,92 @@ +#[ +Copyright (c) Facebook, Inc. and its affiliates. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +Adapted `setenv` from https://github.com/facebook/folly/blob/master/folly/portability/Stdlib.cpp +translated from C to nim. +]# + +#[ +Introduced in https://github.com/facebook/folly/commit/5d8ca09a3f96afefb44e35808f03651a096ab9c7 + +TODO: +check errno_t vs cint +]# + +when not defined(windows): discard +else: + proc setEnvironmentVariableA*(lpName, lpValue: cstring): int32 {. + stdcall, dynlib: "kernel32", importc: "SetEnvironmentVariableA", sideEffect.} + # same as winlean.setEnvironmentVariableA + + proc c_getenv(env: cstring): cstring {.importc: "getenv", header: "".} + proc c_putenv_s(envname: cstring, envval: cstring): cint {.importc: "_putenv_s", header: "".} + proc c_wgetenv(varname: WideCString): WideCString {.importc: "_wgetenv", header: "".} + + var errno {.importc, header: "".}: cint + type wchar_t {.importc: "wchar_t".} = int16 + var gWenviron {.importc:"_wenviron".}: ptr ptr wchar_t + # xxx `ptr UncheckedArray[WideCString]` did not work + + proc mbstowcs_s(pReturnValue: ptr csize_t, wcstr: WideCString, sizeInWords: csize_t, mbstr: cstring, count: csize_t): cint {.importc: "mbstowcs_s", header: "".} + # xxx cint vs errno_t? + + proc setEnvImpl*(name: cstring, value: cstring, overwrite: cint): cint = + const EINVAL = cint(22) + const MAX_ENV = 32767 + # xxx get it from: `var MAX_ENV {.importc: "_MAX_ENV", header:"".}: cint` + if overwrite == 0 and c_getenv(name) != nil: return 0 + if value[0] != '\0': + let e = c_putenv_s(name, value) + if e != 0: + errno = e + return -1 + return 0 + #[ + We are trying to set the value to an empty string, but `_putenv_s` deletes + entries if the value is an empty string, and just calling + SetEnvironmentVariableA doesn't update `_environ`, + so we have to do these terrible things. + ]# + if c_putenv_s(name, " ") != 0: + errno = EINVAL + return -1 + # Here lies the documentation we blatently ignore to make this work. + var s = c_getenv(name) + s[0] = '\0' + #[ + This would result in a double null termination, which normally signifies the + end of the environment variable list, so we stick a completely empty + environment variable into the list instead. + ]# + s[1] = '=' + #[ + If gWenviron is null, the wide environment has not been initialized + yet, and we don't need to try to update it. We have to do this otherwise + we'd be forcing the initialization and maintenance of the wide environment + even though it's never actually used in most programs. + ]# + if gWenviron != nil: + # var buf: array[MAX_ENV + 1, WideCString] + var buf: array[MAX_ENV + 1, Utf16Char] + let buf2 = cast[WideCString](buf[0].addr) + var len: csize_t + if mbstowcs_s(len.addr, buf2, buf.len.csize_t, name, MAX_ENV) != 0: + errno = EINVAL + return -1 + c_wgetenv(buf2)[0] = '\0'.Utf16Char + c_wgetenv(buf2)[1] = '='.Utf16Char + + # And now, we have to update the outer environment to have a proper empty value. + if setEnvironmentVariableA(name, value) == 0: + errno = EINVAL + return -1 + return 0 diff --git a/tests/stdlib/tosenv.nim b/tests/stdlib/tosenv.nim index 08850814bab1..df5759d0cb50 100644 --- a/tests/stdlib/tosenv.nim +++ b/tests/stdlib/tosenv.nim @@ -1,42 +1,58 @@ discard """ matrix: "--threads" joinable: false - targets: "c cpp" + targets: "c js cpp" """ -import os - -block: # delEnv - const dummyEnvVar = "D20210720T144752" # This env var wouldn't be likely to exist to begin with - doAssert not existsEnv(dummyEnvVar) - putEnv(dummyEnvVar, "1") - doAssert existsEnv(dummyEnvVar) - delEnv(dummyEnvVar) - doAssert not existsEnv(dummyEnvVar) - delEnv(dummyEnvVar) # deleting an already deleted env var - doAssert not existsEnv(dummyEnvVar) - -block: # putEnv - # raises OSError on invalid input - doAssertRaises(OSError, putEnv("NIM_TESTS_TOSENV_PUT=DUMMY_VALUE", "NEW_DUMMY_VALUE")) - doAssertRaises(OSError, putEnv("", "NEW_DUMMY_VALUE")) - doAssert not existsEnv("") - doAssert not existsEnv("NIM_TESTS_TOSENV_PUT=DUMMY_VALUE") - doAssert not existsEnv("NIM_TESTS_TOSENV_PUT") - -block: - doAssert getEnv("NIM_TESTS_TOSENV_NONEXISTENT", "") == "" - doAssert getEnv("NIM_TESTS_TOSENV_NONEXISTENT", " ") == " " - doAssert getEnv("NIM_TESTS_TOSENV_NONEXISTENT", "Arrakis") == "Arrakis" - -block: # bug #18533 - proc c_getenv(env: cstring): cstring {.importc: "getenv", header: "".} - var thr: Thread[void] - proc threadFunc {.thread.} = putEnv("foo", "fooVal2") - - putEnv("foo", "fooVal1") - doAssert getEnv("foo") == "fooVal1" - createThread(thr, threadFunc) - joinThreads(thr) - doAssert getEnv("foo") == $c_getenv("foo") - - doAssertRaises(OSError): delEnv("foo=bar") + +import std/os +from std/sequtils import toSeq +import stdtest/testutils + +template main = + block: # delEnv, existsEnv, getEnv, envPairs + for val in ["val", ""]: # ensures empty val works too + const key = "NIM_TESTS_TOSENV_KEY" + doAssert not existsEnv(key) + putEnv(key, val) + doAssert existsEnv(key) + doAssert getEnv(key) == val + when nimvm: discard + else: + doAssert (key, val) in toSeq(envPairs()) + delEnv(key) + when nimvm: discard + else: + doAssert (key, val) notin toSeq(envPairs()) + doAssert not existsEnv(key) + delEnv(key) # deleting an already deleted env var + doAssert not existsEnv(key) + + block: + doAssert getEnv("NIM_TESTS_TOSENV_NONEXISTENT", "") == "" + doAssert getEnv("NIM_TESTS_TOSENV_NONEXISTENT", " ") == " " + doAssert getEnv("NIM_TESTS_TOSENV_NONEXISTENT", "defval") == "defval" + + whenVMorJs: discard # xxx improve + do: + doAssertRaises(OSError, putEnv("NIM_TESTS_TOSENV_PUT=DUMMY_VALUE", "NEW_DUMMY_VALUE")) + doAssertRaises(OSError, putEnv("", "NEW_DUMMY_VALUE")) + doAssert not existsEnv("") + doAssert not existsEnv("NIM_TESTS_TOSENV_PUT=DUMMY_VALUE") + doAssert not existsEnv("NIM_TESTS_TOSENV_PUT") + +static: main() +main() + +when not defined(js): + block: # bug #18533 + proc c_getenv(env: cstring): cstring {.importc: "getenv", header: "".} + var thr: Thread[void] + proc threadFunc {.thread.} = putEnv("foo", "fooVal2") + + putEnv("foo", "fooVal1") + doAssert getEnv("foo") == "fooVal1" + createThread(thr, threadFunc) + joinThreads(thr) + doAssert getEnv("foo") == $c_getenv("foo") + + doAssertRaises(OSError): delEnv("foo=bar")