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
  • Loading branch information
@straight-shoota @RX14
straight-shoota authored and RX14 committed Jul 24, 2018
1 parent 2aea034 commit 4f720ab
Show file tree
Hide file tree
Showing 14 changed files with 293 additions and 82 deletions.
30 changes: 27 additions & 3 deletions spec/std/env_spec.cr
View file
@@ -1,4 +1,5 @@
require "spec"
require "./spec_helper"

describe "ENV" do
it "gets non existent key raises" do
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
View file
Expand Up @@ -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
Expand Down
23 changes: 21 additions & 2 deletions spec/std/string/utf16_spec.cr
View file
Expand Up @@ -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
View file
@@ -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
View file
@@ -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
View file
Expand Up @@ -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
Expand All @@ -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
Expand Down
77 changes: 77 additions & 0 deletions src/crystal/system/win32/env.cr
View file
@@ -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
View file
Expand Up @@ -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]?
Expand Down

0 comments on commit 4f720ab

Please sign in to comment.