diff --git a/changelog.md b/changelog.md index be94f92c4fee7..a19697c8cb23a 100644 --- a/changelog.md +++ b/changelog.md @@ -97,11 +97,14 @@ The downside is that these defines now have custom logic that doesn't apply for other defines. +- `std/os`: `putEnv` now raises if the 1st argument contains a `=` + - Renamed `-d:nimCompilerStackraceHints` to `-d:nimCompilerStacktraceHints`. - In `std/dom`, `Interval` is now a `ref object`, same as `Timeout`. Definitions of `setTimeout`, `clearTimeout`, `setInterval`, `clearInterval` were updated. + ## Standard library additions and changes - `strformat`: diff --git a/lib/pure/includes/osenv.nim b/lib/pure/includes/osenv.nim index db291c5fd3773..6aaafbfdaf80d 100644 --- a/lib/pure/includes/osenv.nim +++ b/lib/pure/includes/osenv.nim @@ -40,112 +40,15 @@ when defined(nodejs): # {.error: "requires -d:nodejs".} else: - when defined(windows): - from parseutils import skipIgnoreCase proc c_getenv(env: cstring): cstring {. importc: "getenv", header: "".} - when defined(vcc): + 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: "".} - - # Environment handling cannot be put into RTL, because the `envPairs` - # iterator depends on `environment`. - - var - envComputed {.threadvar.}: bool - environment {.threadvar.}: seq[string] - - when defined(nimV2): - proc unpairedEnvAllocs*(): int = - result = environment.len - if result > 0: inc result - - when defined(windows) and not defined(nimscript): - # because we support Windows GUI applications, things get really - # messy here... - when useWinUnicode: - when defined(cpp): - proc strEnd(cstr: WideCString, c = 0'i32): WideCString {. - importcpp: "(NI16*)wcschr((const wchar_t *)#, #)", header: "".} - else: - proc strEnd(cstr: WideCString, c = 0'i32): WideCString {. - importc: "wcschr", header: "".} - else: - proc strEnd(cstr: cstring, c = 0'i32): cstring {. - importc: "strchr", header: "".} - - proc getEnvVarsC() = - if not envComputed: - environment = @[] - when useWinUnicode: - var - env = getEnvironmentStringsW() - e = env - if e == nil: return # an error occurred - while true: - var eend = strEnd(e) - add(environment, $e) - e = cast[WideCString](cast[ByteAddress](eend)+2) - if eend[1].int == 0: break - discard freeEnvironmentStringsW(env) - else: - var - env = getEnvironmentStringsA() - e = env - if e == nil: return # an error occurred - while true: - var eend = strEnd(e) - add(environment, $e) - e = cast[cstring](cast[ByteAddress](eend)+1) - if eend[1] == '\0': break - discard freeEnvironmentStringsA(env) - envComputed = true - - else: - const - useNSGetEnviron = (defined(macosx) and not defined(ios) and not defined(emscripten)) or defined(nimscript) - - when useNSGetEnviron: - # From the manual: - # Shared libraries and bundles don't have direct access to environ, - # which is only available to the loader ld(1) when a complete program - # is being linked. - # The environment routines can still be used, but if direct access to - # environ is needed, the _NSGetEnviron() routine, defined in - # , can be used to retrieve the address of environ - # at runtime. - proc NSGetEnviron(): ptr cstringArray {. - importc: "_NSGetEnviron", header: "".} - elif defined(haiku): - var gEnv {.importc: "environ", header: "".}: cstringArray - else: - var gEnv {.importc: "environ".}: cstringArray - - proc getEnvVarsC() = - # retrieves the variables of char** env of C's main proc - if not envComputed: - environment = @[] - when useNSGetEnviron: - var gEnv = NSGetEnviron()[] - var i = 0 - while gEnv[i] != nil: - add environment, $gEnv[i] - inc(i) - envComputed = true - - proc findEnvVar(key: string): int = - getEnvVarsC() - var temp = key & '=' - for i in 0..high(environment): - when defined(windows): - if skipIgnoreCase(environment[i], temp) == len(temp): return i - else: - if startsWith(environment[i], temp): return i - return -1 + proc c_unsetenv(env: cstring): cint {.importc: "unsetenv", header: "".} proc getEnv*(key: string, default = ""): string {.tags: [ReadEnvEffect].} = ## Returns the value of the `environment variable`:idx: named `key`. @@ -163,16 +66,9 @@ else: assert getEnv("unknownEnv") == "" assert getEnv("unknownEnv", "doesn't exist") == "doesn't exist" - when nimvm: - discard "built into the compiler" - else: - var i = findEnvVar(key) - if i >= 0: - return substr(environment[i], find(environment[i], '=')+1) - else: - var env = c_getenv(key) - if env == nil: return default - result = $env + let env = c_getenv(key) + if env == nil: return default + result = $env proc existsEnv*(key: string): bool {.tags: [ReadEnvEffect].} = ## Checks whether the environment variable named `key` exists. @@ -186,11 +82,7 @@ else: runnableExamples: assert not existsEnv("unknownEnv") - when nimvm: - discard "built into the compiler" - else: - if c_getenv(key) != nil: return true - else: return findEnvVar(key) >= 0 + return c_getenv(key) != nil proc putEnv*(key, val: string) {.tags: [WriteEnvEffect].} = ## Sets the value of the `environment variable`:idx: named `key` to `val`. @@ -201,33 +93,14 @@ else: ## * `existsEnv proc <#existsEnv,string>`_ ## * `delEnv proc <#delEnv,string>`_ ## * `envPairs iterator <#envPairs.i>`_ - - # Note: by storing the string in the environment sequence, - # we guarantee that we don't free the memory before the program - # ends (this is needed for POSIX compliance). It is also needed so that - # the process itself may access its modified environment variables! - when nimvm: - discard "built into the compiler" + when defined(windows): + 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: - var indx = findEnvVar(key) - if indx >= 0: - environment[indx] = key & '=' & val - else: - add environment, (key & '=' & val) - indx = high(environment) - when defined(windows) and not defined(nimscript): - when useWinUnicode: - var k = newWideCString(key) - var v = newWideCString(val) - if setEnvironmentVariableW(k, v) == 0'i32: raiseOSError(osLastError()) - else: - if setEnvironmentVariableA(key, val) == 0'i32: raiseOSError(osLastError()) - elif defined(vcc): - if c_putenv_s(key, val) != 0'i32: - raiseOSError(osLastError()) - else: - if c_setenv(key, val, 1'i32) != 0'i32: - raiseOSError(osLastError()) + 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`. @@ -238,21 +111,45 @@ else: ## * `existsEnv proc <#existsEnv,string>`_ ## * `putEnv proc <#putEnv,string,string>`_ ## * `envPairs iterator <#envPairs.i>`_ - when nimvm: - discard "built into the compiler" + template bail = raiseOSError(osLastError(), key) + when defined(windows): + #[ + # https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/putenv-s-wputenv-s?view=msvc-160 + > 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: - var indx = findEnvVar(key) - if indx < 0: return # Do nothing if the env var is not already set - when defined(windows) and not defined(nimscript): - when useWinUnicode: - var k = newWideCString(key) - if setEnvironmentVariableW(k, nil) == 0'i32: raiseOSError(osLastError()) - else: - if setEnvironmentVariableA(key, nil) == 0'i32: raiseOSError(osLastError()) + if c_unsetenv(key) != 0'i32: bail + + when defined(windows): + when useWinUnicode: + when defined(cpp): + proc strEnd(cstr: WideCString, c = 0'i32): WideCString {.importcpp: "(NI16*)wcschr((const wchar_t *)#, #)", + header: "".} else: - if c_unsetenv(key) != 0'i32: - raiseOSError(osLastError()) - environment.delete(indx) + proc strEnd(cstr: WideCString, c = 0'i32): WideCString {.importc: "wcschr", + header: "".} + else: + proc strEnd(cstr: cstring, c = 0'i32): cstring {.importc: "strchr", + header: "".} + elif defined(macosx) and not defined(ios) and not defined(emscripten): + # From the manual: + # Shared libraries and bundles don't have direct access to environ, + # which is only available to the loader ld(1) when a complete program + # is being linked. + # The environment routines can still be used, but if direct access to + # environ is needed, the _NSGetEnviron() routine, defined in + # , can be used to retrieve the address of environ + # at runtime. + proc NSGetEnviron(): ptr cstringArray {.importc: "_NSGetEnviron", + header: "".} + elif defined(haiku): + var gEnv {.importc: "environ", header: "".}: cstringArray + else: + var gEnv {.importc: "environ".}: cstringArray iterator envPairs*(): tuple[key, value: string] {.tags: [ReadEnvEffect].} = ## Iterate over all `environments variables`:idx:. @@ -265,8 +162,30 @@ else: ## * `existsEnv proc <#existsEnv,string>`_ ## * `putEnv proc <#putEnv,string,string>`_ ## * `delEnv proc <#delEnv,string>`_ - getEnvVarsC() - for i in 0..high(environment): - var p = find(environment[i], '=') - yield (substr(environment[i], 0, p-1), - substr(environment[i], p+1)) + when defined(windows): + block: + template impl(get_fun, typ, size, zero, free_fun) = + let env = get_fun() + var e = env + if e == nil: break + while true: + let eend = strEnd(e) + let kv = $e + let p = find(kv, '=') + yield (substr(kv, 0, p-1), substr(kv, p+1)) + e = cast[typ](cast[ByteAddress](eend)+size) + if typeof(zero)(eend[1]) == zero: break + discard free_fun(env) + when useWinUnicode: + impl(getEnvironmentStringsW, WideCString, 2, 0, freeEnvironmentStringsW) + else: + impl(getEnvironmentStringsA, cstring, 1, '\0', freeEnvironmentStringsA) + else: + var i = 0 + when defined(macosx) and not defined(ios) and not defined(emscripten): + var gEnv = NSGetEnviron()[] + while gEnv[i] != nil: + let kv = $gEnv[i] + inc(i) + let p = find(kv, '=') + yield (substr(kv, 0, p-1), substr(kv, p+1)) diff --git a/lib/std/private/win_setenv.nim b/lib/std/private/win_setenv.nim new file mode 100644 index 0000000000000..067e656a317f7 --- /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/destructor/tnewruntime_misc.nim b/tests/destructor/tnewruntime_misc.nim index 48ea36b7cf06b..ac061f2f7a1b2 100644 --- a/tests/destructor/tnewruntime_misc.nim +++ b/tests/destructor/tnewruntime_misc.nim @@ -7,7 +7,7 @@ axc ... destroying GenericObj[T] GenericObj[system.int] test -(allocCount: 13, deallocCount: 11) +(allocCount: 12, deallocCount: 10) 3''' """ diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index fedf2b3e3f049..dcb2d44f4578f 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -331,7 +331,7 @@ block walkDirRec: doAssert p.startsWith("walkdir_test") var s: seq[string] - for p in walkDirRec("walkdir_test", {pcFile}, {pcDir}, relative=true): + for p in walkDirRec("walkdir_test", {pcFile}, {pcDir}, relative = true): s.add(p) doAssert s.len == 2 @@ -553,19 +553,19 @@ block ospaths: doAssert joinPath("/", "") == unixToNativePath"/" doAssert joinPath("/" / "") == unixToNativePath"/" # weird test case... doAssert joinPath("/", "/a/b/c") == unixToNativePath"/a/b/c" - doAssert joinPath("foo/","") == unixToNativePath"foo/" - doAssert joinPath("foo/","abc") == unixToNativePath"foo/abc" - doAssert joinPath("foo//./","abc/.//") == unixToNativePath"foo/abc/" - doAssert joinPath("foo","abc") == unixToNativePath"foo/abc" - doAssert joinPath("","abc") == unixToNativePath"abc" + doAssert joinPath("foo/", "") == unixToNativePath"foo/" + doAssert joinPath("foo/", "abc") == unixToNativePath"foo/abc" + doAssert joinPath("foo//./", "abc/.//") == unixToNativePath"foo/abc/" + doAssert joinPath("foo", "abc") == unixToNativePath"foo/abc" + doAssert joinPath("", "abc") == unixToNativePath"abc" - doAssert joinPath("zook/.","abc") == unixToNativePath"zook/abc" + doAssert joinPath("zook/.", "abc") == unixToNativePath"zook/abc" # controversial: inconsistent with `joinPath("zook/.","abc")` # on linux, `./foo` and `foo` are treated a bit differently for executables # but not `./foo/bar` and `foo/bar` doAssert joinPath(".", "/lib") == unixToNativePath"./lib" - doAssert joinPath(".","abc") == unixToNativePath"./abc" + doAssert joinPath(".", "abc") == unixToNativePath"./abc" # cases related to issue #13455 doAssert joinPath("foo", "", "") == "foo" @@ -605,24 +605,6 @@ block getTempDir: block: # getCacheDir doAssert getCacheDir().dirExists -block osenv: - block delEnv: - const dummyEnvVar = "DUMMY_ENV_VAR" # This env var wouldn't be likely to exist to begin with - doAssert existsEnv(dummyEnvVar) == false - putEnv(dummyEnvVar, "1") - doAssert existsEnv(dummyEnvVar) == true - delEnv(dummyEnvVar) - doAssert existsEnv(dummyEnvVar) == false - delEnv(dummyEnvVar) # deleting an already deleted env var - doAssert existsEnv(dummyEnvVar) == false - block: # putEnv, bug #18502 - doAssertRaises(OSError): putEnv("DUMMY_ENV_VAR_PUT=DUMMY_VALUE", "NEW_DUMMY_VALUE") - doAssertRaises(OSError): putEnv("", "NEW_DUMMY_VALUE") - block: - doAssert getEnv("DUMMY_ENV_VAR_NONEXISTENT", "") == "" - doAssert getEnv("DUMMY_ENV_VAR_NONEXISTENT", " ") == " " - doAssert getEnv("DUMMY_ENV_VAR_NONEXISTENT", "Arrakis") == "Arrakis" - block isRelativeTo: doAssert isRelativeTo("/foo", "/") doAssert isRelativeTo("/foo/bar", "/foo") diff --git a/tests/stdlib/tosenv.nim b/tests/stdlib/tosenv.nim new file mode 100644 index 0000000000000..df5759d0cb50d --- /dev/null +++ b/tests/stdlib/tosenv.nim @@ -0,0 +1,58 @@ +discard """ + matrix: "--threads" + joinable: false + targets: "c js cpp" +""" + +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")