Bug #3812
closed(mswin|mingw) ENV[]= fails silently to set huge values
Description
=begin
On mswin&mingw the size of an environment key, value pair is bounded by _MAX_ENV. Calls to putenv() or _putenv() that exceed this will fail and not set the value.
Trivial patch that adds test case and error checking to raise Errno::EINVAL on failure.
=end
Files
Updated by nobu (Nobuyoshi Nakada) about 15 years ago
=begin
Hi,
At Fri, 10 Sep 2010 02:52:07 +0900,
Peter Weldon wrote in [ruby-core:32250]:
On mswin&mingw the size of an environment key, value pair is
bounded by _MAX_ENV. Calls to putenv() or _putenv() that
exceed this will fail and not set the value.
Thank you, but alloca is not safe for such huge values. IIRC,
it could return invalid address silently.
diff --git a/hash.c b/hash.c
index 8bba586..638f493 100644
--- a/hash.c
+++ b/hash.c
@@ -2139,28 +2139,24 @@ void
ruby_setenv(const char *name, const char *value)
{
#if defined(_WIN32)
- int len;
- char *buf;
- VALUE buf;
if (strchr(name, '=')) { -
errno = EINVAL;fail:
rb_sys_fail("ruby_setenv");
} - len = strlen(name) + 1 + 1;
if (value) {
- len = strlen(name) + 1 + strlen(value) + 1;
- buf = ALLOCA_N(char, len);
- snprintf(buf, len, "%s=%s", name, value);
- putenv(buf);
- /* putenv() doesn't handle empty value */
- if (!*value)
-
SetEnvironmentVariable(name,value);
- buf = rb_sprintf("%s=%s", name, value);
}
else {
- len = strlen(name) + 1 + 1;
- buf = ALLOCA_N(char, len);
- snprintf(buf, len, "%s=", name);
- putenv(buf);
- SetEnvironmentVariable(name, 0);
- buf = rb_sprintf("%s=", name);
- }
- if (putenv(RSTRING_PTR(buf))) goto fail;
- rb_str_resize(buf);
- if (!value || !*value) {
- /* putenv() doesn't handle empty value */
- if (SetEnvironmentVariable(name,value)) goto fail;
}
#elif defined(HAVE_SETENV) && defined(HAVE_UNSETENV)
#undef setenv
--
Nobu Nakada
=end
Updated by pweldon (Peter Weldon) about 15 years ago
=begin
Thank you for the better patch. Fixes to make it compile, new test and fixed broken test:
hash.c | 25 ++++++++++---------------
test/ruby/test_env.rb | 13 +++++++++++++
test/ruby/test_require.rb | 12 ++++--------
3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/hash.c b/hash.c
index 8bba586..1e3b30b 100644
--- a/hash.c
+++ b/hash.c
@@ -2139,28 +2139,23 @@ void
ruby_setenv(const char *name, const char *value)
{
#if defined(_WIN32)
- int len;
- char *buf;
- VALUE buf;
if (strchr(name, '=')) { -
errno = EINVAL;fail:
rb_sys_fail("ruby_setenv");
}
if (value) {
- len = strlen(name) + 1 + strlen(value) + 1;
- buf = ALLOCA_N(char, len);
- snprintf(buf, len, "%s=%s", name, value);
- putenv(buf);
- /* putenv() doesn't handle empty value */
- if (!*value)
-
SetEnvironmentVariable(name,value);
- buf = rb_sprintf("%s=%s", name, value);
}
else {
- len = strlen(name) + 1 + 1;
- buf = ALLOCA_N(char, len);
- snprintf(buf, len, "%s=", name);
- putenv(buf);
- SetEnvironmentVariable(name, 0);
- buf = rb_sprintf("%s=", name);
- }
- if (putenv(RSTRING_PTR(buf))) goto fail;
- rb_str_resize(buf,0);
- if (!value || !*value) {
- /* putenv() doesn't handle empty value */
- if (!SetEnvironmentVariable(name,value)) goto fail;
}
#elif defined(HAVE_SETENV) && defined(HAVE_UNSETENV)
#undef setenv
diff --git a/test/ruby/test_env.rb b/test/ruby/test_env.rb
index 785b6bb..52b95e3 100644
--- a/test/ruby/test_env.rb
+++ b/test/ruby/test_env.rb
@@ -1,4 +1,5 @@
require 'test/unit'
+require 'rbconfig'
class TestEnv < Test::Unit::TestCase
IGNORE_CASE = /bccwin|mswin|mingw/ =~ RUBY_PLATFORM
@@ -374,4 +375,16 @@ class TestEnv < Test::Unit::TestCase
ENV.update({"baz"=>"quux","a"=>"b"}) {|k, v1, v2| v1 ? k + "" + v1 + "" + v2 : v2 }
check(ENV.to_hash.to_a, [%w(foo bar), %w(baz baz_qux_quux), %w(a b)])
end
+
- def test_huge_value
- huge_value = "bar" * 40960
- ENV["foo"] = "bar"
- if Config::CONFIG['host_os'] =~ /mswin|mingw/
-
assert_raise(Errno::EINVAL) { ENV["foo"] = huge_value } -
assert_equal("bar", ENV["foo"]) - else
-
assert_nothing_raised { ENV["foo"] = huge_value } -
assert_equal(huge_value, ENV["foo"]) - end
- end
end
diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb
index f87de90..f8e8edd 100644
--- a/test/ruby/test_require.rb
+++ b/test/ruby/test_require.rb
@@ -49,18 +49,14 @@ class TestRequire < Test::Unit::TestCase
def test_require_path_home
env_rubypath, env_home = ENV["RUBYPATH"], ENV["HOME"]
- ENV["RUBYPATH"] = "~"
- ENV["HOME"] = "/foo" * 10000
- assert_in_out_err(%w(-S test_ruby_test_require), "", [], /^.+$/)
- ENV["RUBYPATH"] = "~" + "/foo" * 10000
- ENV["HOME"] = "/foo"
- assert_in_out_err(%w(-S test_ruby_test_require), "", [], /^.+$/)
- t = Tempfile.new(["test_ruby_test_require", ".rb"])
t.puts "p :ok"
t.close
- ENV["RUBYPATH"] = "~"
- ENV["HOME"] = t.path
- assert_in_out_err(%w(-S test_ruby_test_require), "", [], /(LoadError)/)
- ENV["HOME"], name = File.split(t.path)
assert_in_out_err(["-S", name], "", %w(:ok), [])
--
=end
Updated by luislavena (Luis Lavena) about 15 years ago
=begin
I would recommend usage of RbConfig::CONFIG instead of Config::CONFIG due it's deprecation warning.
=end
Updated by nobu (Nobuyoshi Nakada) about 15 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
=begin
This issue was solved with changeset r29225.
Peter, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
=end
Updated by nobu (Nobuyoshi Nakada) about 15 years ago
- Status changed from Closed to Assigned
- Assignee set to yugui (Yuki Sonoda)
=begin
=end
Updated by nobu (Nobuyoshi Nakada) about 15 years ago
- Status changed from Assigned to Closed
- Assignee deleted (
yugui (Yuki Sonoda))
=begin
=end