Forcing strings in %ENV (was Re: [perl.git] branch blead, updated.v5.17.2-11-g1203306)

On Tue, Jul 24, 2012 at 09:39:53AM +0200, Chip Salzenberg wrote:
> In perl.git, the branch blead has been updated
> 
> <http://perl5.git.perl.org/perl.git/commitdiff/1203306491d341ed2f463fbd53a687cff1675d65?hp=a9844598bfc0b23a08d80e539ba4e03fbe392971>
> 
> - Log -----------------------------------------------------------------
> commit 1203306491d341ed2f463fbd53a687cff1675d65
> Author: Chip Salzenberg <chip@pobox.com>
> Date:   Tue Jul 24 00:39:46 2012 -0700
> 
>     ensure that the env var SV after C<{FOO}='x'> is PV only

> +    if (SvOK(sv)) {
> +	s = SvPV_const(sv,len);
> +	SvPOK_only(sv); /* environment variables are strings, period */
> +    }
> +    my_setenv(key, s); /* does the deed */
> +

Doing SvPOK_only() doesn't work in all cases - it's causing assertion
failures on references and globs. I think it needs to be SvPV_force() - ie
the appended patch.

However, even then I'm not sure if things are perfect. Why does undef get
special casing?

$ ./perl -wle '$ENV{a} = undef; print exists $ENV{a} ? "e" : "."; print defined $ENV{a} ? "d" : "."'
e
.

[ie no warnings, and not forced to the empty string]


Also, doing this, I found something I didn't know. This is OS X:

$ ./env zero foo one =bar two ==baz
getenv("zero") is "foo"
getenv("one") is "bar"
getenv("two") is "=baz"

FreeBSD and Linux both give

$ ./env zero foo one =bar two ==baz
getenv("zero") is "foo"
getenv("one") is "=bar"
getenv("two") is "==baz"

(test program attached)

All do this:

$ ./env bad= foo
setenv: Invalid argument

which means that part of the OS X man page is wrong:

     int
     setenv(const char *name, const char *value, int overwrite);

     int
     unsetenv(const char *name);

DESCRIPTION
     These functions set, unset and fetch environment variables from the host
     environment list.  For compatibility with differing environment conven-
     tions, the given arguments name and value may be appended and prepended,
     respectively, with an equal sign ``=''.

Great.

Nicholas Clark

diff --git a/ext/Devel-Peek/t/Peek.t b/ext/Devel-Peek/t/Peek.t
index 1d0de0d..0b8f938 100644
--- a/ext/Devel-Peek/t/Peek.t
+++ b/ext/Devel-Peek/t/Peek.t
@@ -543,7 +543,7 @@ do_test('scalar with pos magic',
 # environment variables may be invisibly case-forced, hence the (?i:PATH)
 # C<scalar(@ARGV)> is turned into an IV on VMS hence the (?:IV)?
 # Perl 5.18 ensures all env vars end up as strings only, hence the (?:,pIOK)?
-# Perl 5.18 ensures even magic vars have public OK, hence the (?:,POK)?
+# Perl 5.18 ensures even magic vars have public OK, hence the (?:,IOK,POK)?
 # VMS is setting FAKE and READONLY flags.  What VMS uses for storing
 # ENV hashes is also not always null terminated.
 #
@@ -551,7 +551,7 @@ do_test('tainted value in %ENV',
         $ENV{PATH}=@ARGV,  # scalar(@ARGV) is a handy known tainted value
 'SV = PVMG\\($ADDR\\) at $ADDR
   REFCNT = 1
-  FLAGS = \\(GMG,SMG,RMG(?:,POK)?(?:,pIOK)?,pPOK\\)
+  FLAGS = \\(GMG,SMG,RMG(?:,IOK,POK)?(?:,pIOK)?,pPOK\\)
   IV = 0
   NV = 0
   PV = $ADDR "0"\\\0
diff --git a/mg.c b/mg.c
index 2705109..89c628d 100644
--- a/mg.c
+++ b/mg.c
@@ -1170,8 +1170,8 @@ Perl_magic_setenv(pTHX_ SV *sv, MAGIC *mg)
     PERL_ARGS_ASSERT_MAGIC_SETENV;
 
     if (SvOK(sv)) {
-	s = SvPV_const(sv,len);
-	SvPOK_only(sv); /* environment variables are strings, period */
+        /* environment variables are strings, period */
+	s = SvPV_force(sv,len);
     }
     my_setenv(key, s); /* does the deed */
 
diff --git a/t/op/magic.t b/t/op/magic.t
index 643eeb6..105e61d 100644
--- a/t/op/magic.t
+++ b/t/op/magic.t
@@ -5,7 +5,7 @@ BEGIN {
     chdir 't' if -d 't';
     @INC = '../lib';
     require './test.pl';
-    plan (tests => 156);
+    plan (tests => 160);
 }
 
 # Test that defined() returns true for magic variables created on the fly,
@@ -584,7 +584,7 @@ SKIP: {
 # ^^^^^^^^^ New tests go here ^^^^^^^^^
 
 SKIP: {
-    skip("%ENV manipulations fail or aren't safe on $^O", 4)
+    skip("%ENV manipulations fail or aren't safe on $^O", 8)
 	if $Is_VMS || $Is_Dos;
 
  SKIP: {
@@ -649,6 +649,14 @@ SKIP: {
 	       || ($^O eq 'freebsd' && $ps =~ m/^(?:perl: )?x(?: \(perl\))?$/),
 		       'altering $0 is effective (testing with `ps`)');
 	}
+
+	$ENV{foo} = [];
+	is(ref $ENV{foo}, '', '%ENV forces values to plain strings');
+	like($ENV{foo}, qr/\AARRAY\(0x[0-9a-f]+\)\z/,
+	     '%ENV forces values to plain strings');
+	$ENV{foo} = *STDOUT;
+	is(ref \$ENV{foo}, 'SCALAR', '%ENV forces globs to plain strings');
+	is($ENV{foo}, '*main::STDOUT', '%ENV forces globs to plain strings');
 }
 
 # test case-insignificance of %ENV (these tests must be enabled only

• Forcing strings in %ENV (was Re: [perl.git] branch blead, updated.v5.17.2-11-g1203306) by Nicholas Clark
• Re: Forcing byte strings in %ENV, completed, 2/2 (was Re: [perl.git]branch blead, updated. v5.17.2-11-g1203306) by Rev. Chip
• Re: Forcing byte strings in %ENV, completed, 1/2 (was Re: [perl.git]branch blead, updated. v5.17.2-11-g1203306) by Rev. Chip