Skip to content

Commit

Permalink
Extract platform-specifics from ENV to Crystal::System::Env and imple…
Browse files Browse the repository at this point in the history
…ment for win32 (#6333)

* Extract  Crystal::System::Env interface

* Implement Crystal::System::Env for win32

* Fix use correct bytesize of UTF-16 strings

* fixup! Implement Crystal::System::Env for win32

Remove LPCWSTR

* fixup! Extract  Crystal::System::Env interface

Refactor String#check_no_null_byte

* fixup! Implement Crystal::System::Env for win32

Use String#check_no_null_byte

* fixup! Fix use correct bytesize of UTF-16 strings

* fixup! Implement Crystal::System::Env for win32

Add spec for empty environment variable. This fails on win32 because `GetEnvironmentVariableW` doesnt differentiate between unset variable and empty string.
In contrast, `GetEnvironmentStringsW` contains empty variables.

* fixup! fixup! Implement Crystal::System::Env for win32

Fix implementation for empty string on win32 using `SetLastError`.

* Uncomment specs depending on ENV in file_specs

* fixup! Uncomment specs depending on ENV in file_specs
straight-shoota authored and RX14 committed Jul 24, 2018
1 parent 2aea034 commit 4f720ab
Showing 14 changed files with 293 additions and 82 deletions.
30 changes: 27 additions & 3 deletions spec/std/env_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "spec"
require "./spec_helper"

describe "ENV" do
it "gets non existent key raises" do
@@ -24,6 +25,11 @@ describe "ENV" do
ENV["FOO"]?.should be_nil
end

it "sets to empty string" do
(ENV["FOO_EMPTY"] = "").should eq ""
ENV["FOO_EMPTY"]?.should eq ""
end

it "does has_key?" do
ENV["FOO"] = "1"
ENV.has_key?("BAR").should be_false
@@ -52,19 +58,19 @@ describe "ENV" do

describe "[]=" do
it "disallows NUL-bytes in key" do
expect_raises(ArgumentError, "Key contains null byte") do
expect_raises(ArgumentError, "String `key` contains null byte") do
ENV["FOO\0BAR"] = "something"
end
end

it "disallows NUL-bytes in key if value is nil" do
expect_raises(ArgumentError, "Key contains null byte") do
expect_raises(ArgumentError, "String `key` contains null byte") do
ENV["FOO\0BAR"] = nil
end
end

it "disallows NUL-bytes in value" do
expect_raises(ArgumentError, "Value contains null byte") do
expect_raises(ArgumentError, "String `value` contains null byte") do
ENV["FOO"] = "BAR\0BAZ"
end
end
@@ -95,4 +101,22 @@ describe "ENV" do
end
end
end

it "handles unicode" do
ENV["TEST_UNICODE_1"] = "bar\u{d7ff}\u{10000}"
ENV["TEST_UNICODE_2"] = "\u{1234}"
ENV["TEST_UNICODE_1"].should eq "bar\u{d7ff}\u{10000}"
ENV["TEST_UNICODE_2"].should eq "\u{1234}"

values = {} of String => String
ENV.each do |key, value|
if key.starts_with?("TEST_UNICODE_")
values[key] = value
end
end
values.should eq({
"TEST_UNICODE_1" => "bar\u{d7ff}\u{10000}",
"TEST_UNICODE_2" => "\u{1234}",
})
end
end
60 changes: 28 additions & 32 deletions spec/std/file_spec.cr
Original file line number Diff line number Diff line change
@@ -582,41 +582,37 @@ describe "File" do
File.expand_path("~/a", "/tmp/gumby/ddd").should eq(File.join([home, "a"]))
end

# TODO: these specs don't compile on windows because ENV isn't ported
# TODO: remove /\A\/\// hack after this is removed from macros
{% unless flag?(:win32) %}
it "converts a pathname to an absolute pathname, using ~ (home) as base (trailing /)" do
prev_home = home
begin
ENV["HOME"] = File.expand_path(datapath)
File.expand_path("~/").should eq(home)
File.expand_path("~/..badfilename").should eq(File.join(home, "..badfilename"))
File.expand_path("..").should eq("/#{base.split('/')[0...-1].join('/')}".gsub(/\A\/\//, "/"))
File.expand_path("~/a", "~/b").should eq(File.join(home, "a"))
File.expand_path("~").should eq(home)
File.expand_path("~", "/tmp/gumby/ddd").should eq(home)
File.expand_path("~/a", "/tmp/gumby/ddd").should eq(File.join([home, "a"]))
ensure
ENV["HOME"] = prev_home
end
pending_win32 "converts a pathname to an absolute pathname, using ~ (home) as base (trailing /)" do
prev_home = home
begin
ENV["HOME"] = File.expand_path(datapath)
File.expand_path("~/").should eq(home)
File.expand_path("~/..badfilename").should eq(File.join(home, "..badfilename"))
File.expand_path("..").should eq("/#{base.split('/')[0...-1].join('/')}".gsub(%r{\A//}, "/"))
File.expand_path("~/a", "~/b").should eq(File.join(home, "a"))
File.expand_path("~").should eq(home)
File.expand_path("~", "/tmp/gumby/ddd").should eq(home)
File.expand_path("~/a", "/tmp/gumby/ddd").should eq(File.join([home, "a"]))
ensure
ENV["HOME"] = prev_home
end
end

it "converts a pathname to an absolute pathname, using ~ (home) as base (HOME=/)" do
prev_home = home
begin
ENV["HOME"] = "/"
File.expand_path("~/").should eq(home)
File.expand_path("~/..badfilename").should eq(File.join(home, "..badfilename"))
File.expand_path("..").should eq("/#{base.split('/')[0...-1].join('/')}".gsub(/\A\/\//, "/"))
File.expand_path("~/a", "~/b").should eq(File.join(home, "a"))
File.expand_path("~").should eq(home)
File.expand_path("~", "/tmp/gumby/ddd").should eq(home)
File.expand_path("~/a", "/tmp/gumby/ddd").should eq(File.join([home, "a"]))
ensure
ENV["HOME"] = prev_home
end
pending_win32 "converts a pathname to an absolute pathname, using ~ (home) as base (HOME=/)" do
prev_home = home
begin
ENV["HOME"] = "/"
File.expand_path("~/").should eq(home)
File.expand_path("~/..badfilename").should eq(File.join(home, "..badfilename"))
File.expand_path("..").should eq("/#{base.split('/')[0...-1].join('/')}".gsub(%r{\A//}, "/"))
File.expand_path("~/a", "~/b").should eq(File.join(home, "a"))
File.expand_path("~").should eq(home)
File.expand_path("~", "/tmp/gumby/ddd").should eq(home)
File.expand_path("~/a", "/tmp/gumby/ddd").should eq(File.join([home, "a"]))
ensure
ENV["HOME"] = prev_home
end
{% end %}
end
end

describe "real_path" do
23 changes: 21 additions & 2 deletions spec/std/string/utf16_spec.cr
Original file line number Diff line number Diff line change
@@ -23,31 +23,50 @@ describe "String UTF16" do
end
end

describe "from_utf16" do
describe ".from_utf16" do
it "in the range U+0000..U+D7FF" do
input = Slice[0_u16, 0x68_u16, 0x65_u16, 0x6c_u16, 0x6c_u16, 0x6f_u16, 0xd7ff_u16]
String.from_utf16(input).should eq("\u{0}hello\u{d7ff}")
String.from_utf16(input.to_unsafe).should eq({"", input.to_unsafe + 1})
end

it "in the range U+E000 to U+FFFF" do
input = Slice[0xe000_u16, 0xffff_u16]
String.from_utf16(input).should eq("\u{e000}\u{ffff}")

pointer = Slice[0xe000_u16, 0xffff_u16, 0_u16].to_unsafe
String.from_utf16(pointer).should eq({"\u{e000}\u{ffff}", pointer + 3})
end

it "in the range U+10000..U+10FFFF" do
input = Slice[0xd800_u16, 0xdc00_u16]
String.from_utf16(input).should eq("\u{10000}")

pointer = Slice[0xd800_u16, 0xdc00_u16, 0_u16].to_unsafe
String.from_utf16(pointer).should eq({"\u{10000}", pointer + 3})
end

it "in the range U+D800..U+DFFF" do
input = Slice[0xdc00_u16, 0xd800_u16]
String.from_utf16(input).should eq("\u{fffd}\u{fffd}")

pointer = Slice[0xdc00_u16, 0xd800_u16, 0_u16].to_unsafe
String.from_utf16(pointer).should eq({"\u{fffd}\u{fffd}", pointer + 3})
end

it "handles null bytes" do
slice = Slice[104_u16, 105_u16, 0_u16, 55296_u16, 56485_u16]
String.from_utf16(slice).should eq("hi\0000𐂥")
String.from_utf16(slice.to_unsafe).should eq("hi")
String.from_utf16(slice.to_unsafe).should eq({"hi", slice.to_unsafe + 3})
end

it "with pointer reads multiple strings" do
input = Slice[0_u16, 0x68_u16, 0x65_u16, 0x6c_u16, 0x6c_u16, 0x6f_u16, 0xd7ff_u16, 0_u16]
pointer = input.to_unsafe
string, pointer = String.from_utf16(pointer)
string.should eq("")
string, pointer = String.from_utf16(pointer)
string.should eq("hello\u{d7ff}")
end
end
end
19 changes: 19 additions & 0 deletions src/crystal/system/env.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module Crystal::System::Env
# Sets an environment variable or unsets it if *value* is `nil`.
# def self.set(key : String, value : String?) : Nil

# Gets an environment variable.
# def self.get(key : String) : String?

# Returns `true` if environment variable is set.
# def self.has_key?(key : String) : Bool

# Iterates all environment variables.
# def self.each(&block : String, String ->)
end

{% if flag?(:win32) %}
require "./win32/env"
{% else %}
require "./unix/env"
{% end %}
55 changes: 55 additions & 0 deletions src/crystal/system/unix/env.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
require "c/stdlib"

module Crystal::System::Env
# Sets an environment variable.
def self.set(key : String, value : String) : Nil
key.check_no_null_byte("key")
value.check_no_null_byte("value")

if LibC.setenv(key, value, 1) != 0
raise Errno.new("setenv")
end
end

# Unsets an environment variable.
def self.set(key : String, value : Nil) : Nil
key.check_no_null_byte("key")

if LibC.unsetenv(key) != 0
raise Errno.new("unsetenv")
end
end

# Gets an environment variable.
def self.get(key : String) : String?
key.check_no_null_byte("key")

if value = LibC.getenv(key)
String.new(value)
end
end

# Returns `true` if environment variable is set.
def self.has_key?(key : String) : Bool
key.check_no_null_byte("key")

!!LibC.getenv(key)
end

# Iterates all environment variables.
def self.each(&block : String, String ->)
environ_ptr = LibC.environ
while environ_ptr
environ_value = environ_ptr.value
if environ_value
key_value = String.new(environ_value).split('=', 2)
key = key_value[0]
value = key_value[1]? || ""
yield key, value
environ_ptr += 1
else
break
end
end
end
end
4 changes: 2 additions & 2 deletions src/crystal/system/win32/dir.cr
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@ module Crystal::System::Dir
handle = LibC.FindFirstFileW(dir.query, out data)
if handle != LibC::INVALID_HANDLE_VALUE
dir.handle = handle
return String.from_utf16(data.cFileName.to_unsafe)
return String.from_utf16(data.cFileName.to_unsafe)[0]
else
error = LibC.GetLastError
if error == WinError::ERROR_FILE_NOT_FOUND
@@ -37,7 +37,7 @@ module Crystal::System::Dir
else
# Use FindNextFile
if LibC.FindNextFileW(dir.handle, out data_) != 0
return String.from_utf16(data_.cFileName.to_unsafe)
return String.from_utf16(data_.cFileName.to_unsafe)[0]
else
error = LibC.GetLastError
if error == WinError::ERROR_NO_MORE_FILES
77 changes: 77 additions & 0 deletions src/crystal/system/win32/env.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
require "crystal/system/windows"
require "c/winbase"

module Crystal::System::Env
# Sets an environment variable or unsets it if *value* is `nil`.
def self.set(key : String, value : String) : Nil
key.check_no_null_byte("key")
value.check_no_null_byte("value")

if LibC.SetEnvironmentVariableW(key.to_utf16, value.to_utf16) == 0
raise WinError.new("SetEnvironmentVariableW")
end
end

# Unsets an environment variable.
def self.set(key : String, value : Nil) : Nil
key.check_no_null_byte("key")

if LibC.SetEnvironmentVariableW(key.to_utf16, nil) == 0
raise WinError.new("SetEnvironmentVariableW")
end
end

# Gets an environment variable.
def self.get(key : String) : String?
key.check_no_null_byte("key")

System.retry_wstr_buffer do |buffer, small_buf|
# `GetEnvironmentVariableW` doesn't set last error on success but we need
# a success message in order to identify if length == 0 means not found or
# the value is an empty string.
LibC.SetLastError(WinError::ERROR_SUCCESS)
length = LibC.GetEnvironmentVariableW(key.to_utf16, buffer, buffer.size)

if 0 < length < buffer.size
return String.from_utf16(buffer[0, length])
elsif small_buf && length > 0
next length
else
case last_error = LibC.GetLastError
when WinError::ERROR_SUCCESS
return ""
when WinError::ERROR_ENVVAR_NOT_FOUND
return
else
raise WinError.new("GetEnvironmentVariableW", last_error)
end
end
end
end

# Returns `true` if environment variable is set.
def self.has_key?(key : String) : Bool
key.check_no_null_byte("key")

buffer = uninitialized UInt16[1]
LibC.GetEnvironmentVariableW(key.to_utf16, buffer, buffer.size) != 0
end

# Iterates all environment variables.
def self.each(&block : String, String ->)
orig_pointer = pointer = LibC.GetEnvironmentStringsW
raise WinError.new("GetEnvironmentStringsW") if pointer.null?

begin
while !pointer.value.zero?
string, pointer = String.from_utf16(pointer)
key_value = string.split('=', 2)
key = key_value[0]
value = key_value[1]? || ""
yield key, value
end
ensure
LibC.FreeEnvironmentStringsW(orig_pointer)
end
end
end
4 changes: 2 additions & 2 deletions src/crystal/system/win32/time.cr
Original file line number Diff line number Diff line change
@@ -141,13 +141,13 @@ module Crystal::System::Time

# Normalizes the names of the standard and dst zones.
private def self.normalize_zone_names(info : LibC::TIME_ZONE_INFORMATION) : Tuple(String, String)
stdname = String.from_utf16(info.standardName.to_unsafe)
stdname = String.from_utf16(info.standardName.to_slice)

if normalized_names = WINDOWS_ZONE_NAMES[stdname]?
return normalized_names
end

dstname = String.from_utf16(info.daylightName.to_unsafe)
dstname = String.from_utf16(info.daylightName.to_slice)

if english_name = translate_zone_name(stdname, dstname)
if normalized_names = WINDOWS_ZONE_NAMES[english_name]?
39 changes: 11 additions & 28 deletions src/env.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require "c/stdlib"
require "crystal/system/env"

# `ENV` is a hash-like accessor for environment variables.
#
@@ -35,24 +35,15 @@ module ENV
#
# If *key* or *value* contains a null-byte an `ArgumentError` is raised.
def self.[]=(key : String, value : String?)
raise ArgumentError.new("Key contains null byte") if key.byte_index(0)
Crystal::System::Env.set(key, value)

if value
raise ArgumentError.new("Value contains null byte") if value.byte_index(0)

if LibC.setenv(key, value, 1) != 0
raise Errno.new("Error setting environment variable #{key.inspect}")
end
else
LibC.unsetenv(key)
end
value
end

# Returns `true` if the environment variable named *key* exists and `false`
# if it doesn't.
def self.has_key?(key : String) : Bool
!!LibC.getenv(key)
Crystal::System::Env.has_key?(key)
end

# Retrieves a value corresponding to the given *key*. Raises a `KeyError` exception if the
@@ -72,9 +63,11 @@ module ENV
# Retrieves a value corresponding to a given *key*. Return the value of the block if
# the *key* does not exist.
def self.fetch(key : String, &block : String -> String?)
value = LibC.getenv key
return String.new(value) if value
yield(key)
if value = Crystal::System::Env.get(key)
return value
else
yield key
end
end

# Returns an array of all the environment variable names.
@@ -95,7 +88,7 @@ module ENV
# the environment variable existed, otherwise returns `nil`.
def self.delete(key : String) : String?
if value = self[key]?
LibC.unsetenv(key)
Crystal::System::Env.set(key, nil)
value
else
nil
@@ -111,18 +104,8 @@ module ENV
# end
# ```
def self.each
environ_ptr = LibC.environ
while environ_ptr
environ_value = environ_ptr.value
if environ_value
key_value = String.new(environ_value).split('=', 2)
key = key_value[0]
value = key_value[1]? || ""
yield({key, value})
environ_ptr += 1
else
break
end
Crystal::System::Env.each do |key, value|
yield key, value
end
end

1 change: 0 additions & 1 deletion src/lib_c/x86_64-windows-msvc/c/stdlib.cr
Original file line number Diff line number Diff line change
@@ -10,7 +10,6 @@ lib LibC
fun div(numer : Int, denom : Int) : DivT
fun exit(status : Int) : NoReturn
fun free(ptr : Void*) : Void
fun getenv(name : Char*) : Char*
fun malloc(size : SizeT) : Void*
fun putenv(string : Char*) : Int
fun realloc(ptr : Void*, size : SizeT) : Void*
6 changes: 6 additions & 0 deletions src/lib_c/x86_64-windows-msvc/c/winbase.cr
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ require "c/int_safe"

lib LibC
fun GetLastError : DWORD
fun SetLastError(dwErrCode : DWORD)

FORMAT_MESSAGE_ALLOCATE_BUFFER = 0x00000100_u32
FORMAT_MESSAGE_IGNORE_INSERTS = 0x00000200_u32
@@ -85,4 +86,9 @@ lib LibC
INVALID_HANDLE_VALUE = HANDLE.new(-1)

fun CloseHandle(hObject : HANDLE) : BOOL

fun GetEnvironmentVariableW(lpName : LPWSTR, lpBuffer : LPWSTR, nSize : DWORD) : DWORD
fun GetEnvironmentStringsW : LPWCH
fun FreeEnvironmentStringsW(lpszEnvironmentBlock : LPWCH) : BOOL
fun SetEnvironmentVariableW(lpName : LPWSTR, lpValue : LPWSTR) : BOOL
end
1 change: 1 addition & 0 deletions src/lib_c/x86_64-windows-msvc/c/winnt.cr
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ lib LibC
alias WCHAR = UInt16
alias LPSTR = CHAR*
alias LPWSTR = WCHAR*
alias LPWCH = WCHAR*

alias HANDLE = Void*

7 changes: 5 additions & 2 deletions src/string.cr
Original file line number Diff line number Diff line change
@@ -4244,8 +4244,11 @@ class String
# Raises an `ArgumentError` if `self` has null bytes. Returns `self` otherwise.
#
# This method should sometimes be called before passing a `String` to a C function.
def check_no_null_byte
raise ArgumentError.new("String contains null byte") if byte_index(0)
def check_no_null_byte(name = nil)
if byte_index(0)
name = "`#{name}` " if name
raise ArgumentError.new("String #{name}contains null byte")
end
self
end

49 changes: 39 additions & 10 deletions src/string/utf16.cr
Original file line number Diff line number Diff line change
@@ -53,15 +53,7 @@ class String
# slice = Slice[104_u16, 105_u16, 32_u16, 55296_u16, 56485_u16]
# String.from_utf16(slice) # => "hi 𐂥"
# ```
#
# If *slice* is a pointer, the string ends when a zero value is found.
#
# ```
# slice = Slice[104_u16, 105_u16, 0_u16, 55296_u16, 56485_u16]
# String.from_utf16(slice) # => "hi\0000𐂥"
# String.from_utf16(slice.to_unsafe) # => "hi"
# ```
def self.from_utf16(slice : Slice(UInt16) | Pointer(UInt16)) : String
def self.from_utf16(slice : Slice(UInt16)) : String
bytesize = 0
size = 0

@@ -81,6 +73,41 @@ class String
end
end

# Decodes the given *slice* UTF-16 sequence into a String and returns the
# pointer after reading. The string ends when a zero value is found.
#
# ```
# slice = Slice[104_u16, 105_u16, 0_u16, 55296_u16, 56485_u16, 0_u16]
# String.from_utf16(slice) # => "hi\0000𐂥"
# pointer = slice.to_unsafe
# string, pointer = String.from_utf16(pointer) # => "hi"
# string, pointer = String.from_utf16(pointer) # => "𐂥"
# ```
#
# Invalid values are encoded using the unicode replacement char with
# codepoint `0xfffd`.
def self.from_utf16(pointer : Pointer(UInt16)) : {String, Pointer(UInt16)}
bytesize = 0
size = 0

each_utf16_char(pointer) do |char|
bytesize += char.bytesize
size += 1
end

string = String.new(bytesize) do |buffer|
pointer = each_utf16_char(pointer) do |char|
char.each_byte do |byte|
buffer.value = byte
buffer += 1
end
end
{bytesize, size}
end

{string, pointer + 1}
end

# Yields each decoded char in the given slice.
private def self.each_utf16_char(slice : Slice(UInt16))
i = 0
@@ -107,7 +134,7 @@ class String
end

# Yields each decoded char in the given pointer, stopping at the first null byte.
private def self.each_utf16_char(pointer : Pointer(UInt16))
private def self.each_utf16_char(pointer : Pointer(UInt16)) : Pointer(UInt16)
loop do
byte = pointer.value.to_i
break if byte == 0
@@ -129,5 +156,7 @@ class String

pointer = pointer + 1
end

pointer
end
end

0 comments on commit 4f720ab

Please sign in to comment.