[BUG] Can't mark unset variables as read-only

[BUG] Can't mark unset variables as read-only

[Martijn Dekker]

Unlike other shells, zsh mode can't mark an unset variable as read-only.
Upon encountering the readonly command, the variable is set to the empty
string instead.

Test code:

( unset -v testv && readonly testv && test "${testv+set}" = '' ) || \
        echo "*** Can't keep unset variables as read-only." 1>&2

This succeeds in ash, dash, bash, ksh, etc. but fails in zsh, even in
'emulate sh' mode.

Actual result:
testv is set to the empty string and marked read-only.

Expected result:
testv remains unset and is marked read-only.

Real-world impact:
Testing whether a variable is set is a common way of checking whether a
certain setting or feature is active. Hardened scripts may want to set
such either-set-or-unset variables as read-only after setting up their
environment, to prevent them from being accidentally clobbered. This is
not possible in zsh.

[BUG] Can't mark unset variables as read-only

[Martijn Dekker]

Unlike other shells, zsh can't mark an unset variable as read-only.
Upon encountering the readonly command, the variable is set to the empty
string instead.

Test code:

( unset -v testv && readonly testv && test "${testv+set}" = '' ) || \
        echo "*** Can't keep unset variables as read-only." 1>&2

This succeeds in ash, dash, bash, ksh, etc. but fails in zsh, even in
'emulate sh' mode.

Actual result:
testv is set to the empty string and marked read-only.

Expected result:
testv remains unset and is marked read-only.

Real-world impact:
Testing whether a variable is set is a common way of checking whether a
certain setting or feature is active. Hardened scripts may want to set
such either-set-or-unset variables as read-only after setting up their
environment, to prevent them from being accidentally clobbered. This is
not possible in zsh.

Re: [BUG] Can't mark unset variables as read-only

[Peter Stephenson]

On Wed, 29 Apr 2015 08:35:43 +0200
Martijn Dekker <martijn@inlv.org> wrote:
> Unlike other shells, zsh can't mark an unset variable as read-only.

Yes, the standard does indeed require that ability.  As you can imagine,
something like this that completely breaks the normal programming model
of variables (a variable can't have state if it's unset because it
doesn't exist) is a nightmare to implement; however, it can at least be
limited to the POSIXBUILTINS option and maybe the cases using that are
simple enough that it mostly works.  I'm sure there are any number of
strange edge cases, though.

The output of "readonly -p" is still broken (doesn't show the unset
variables in a fashion that can be used for restoring the current state)
but it was anyway:

  typeset -ar '*'
  *=()

Er, no, I don't think that's going to work.

So that can be fixed separately.

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index dd5a80f..985d19e 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1933,6 +1933,13 @@ item(tt(-r))(
 The given var(name)s are marked readonly.  Note that if var(name) is a
 special parameter, the readonly attribute can be turned on, but cannot then
 be turned off.
+
+If the tt(POSIX_BUILTINS) option is set, the readonly attribute is
+more restrictive: unset variables can be marked readonly and cannot then
+be set; furthermore, the readonly attribute cannot be removed from any
+variable.  Note that in zsh (unlike other shells) it is still possible
+to create a local variable of the same name as this is considered a
+different variable (though this variable, too, can be marked readonly).
 )
 item(tt(-t))(
 Tags the named parameters.  Tags have no special meaning to the shell.
diff --git a/Src/builtin.c b/Src/builtin.c
index de01014..0a57489 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1931,8 +1931,12 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
      * locallevel as an unset one we use the pm struct anyway: that's
      * handled in createparam().  Here we just avoid using it for the
      * present tests if it's unset.
+     *
+     * POSIXBUILTINS horror: we need to retain the 'readonly' flag
+     * of an unset parameter.
      */
-    usepm = pm && !(pm->node.flags & PM_UNSET);
+    usepm = pm && (!(pm->node.flags & PM_UNSET) ||
+		   (isset(POSIXBUILTINS) && (pm->node.flags & PM_READONLY)));
 
     /*
      * We need to compare types with an existing pm if special,
@@ -2032,6 +2036,20 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
     else if (newspecial != NS_NONE && strcmp(pname, "SECONDS") == 0)
 	newspecial = NS_SECONDS;
 
+    if (isset(POSIXBUILTINS)) {
+	/*
+	 * Stricter rules about retaining readonly attribute in this case.
+	 */
+	if ((on & PM_READONLY) && (!usepm || (pm->node.flags & PM_UNSET)) &&
+	    !value)
+	    on |= PM_UNSET;
+	else if (usepm && (pm->node.flags & PM_READONLY) &&
+		 !(on & PM_READONLY)) {
+	    zerr("read-only variable: %s", pm->node.nam);
+	    return NULL;
+	}
+    }
+
     /*
      * A parameter will be local if
      * 1. we are re-using an existing local parameter
@@ -2078,9 +2096,15 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	}
 	if (usepm == 2)		/* do not change the PM_UNSET flag */
 	    pm->node.flags = (pm->node.flags | (on & ~PM_READONLY)) & ~off;
-	else
+	else {
+	    /*
+	     * Keep unset if using readonly in POSIX mode.
+	     */
+	    if (!(on & PM_READONLY) || !isset(POSIXBUILTINS))
+		off |= PM_UNSET;
 	    pm->node.flags = (pm->node.flags |
-			      (on & ~PM_READONLY)) & ~(off | PM_UNSET);
+			      (on & ~PM_READONLY)) & ~off;
+	}
 	if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
 	    if (typeset_setwidth(cname, pm, ops, on, 0))
 		return NULL;
@@ -2256,7 +2280,12 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	 * readonly flag
 	 */
 	pm = createparam(pname, on & ~PM_READONLY);
-	DPUTS(!pm, "BUG: parameter not created");
+	if (!pm) {
+	    if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z |
+		      PM_INTEGER | PM_EFLOAT | PM_FFLOAT))
+		zerrnam(cname, "can't change variable attribute: %s", pname);
+	    return NULL;
+	}
 	if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
 	    if (typeset_setwidth(cname, pm, ops, on, 0))
 		return NULL;
diff --git a/Src/params.c b/Src/params.c
index e8a9010..d53b6ca 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -874,10 +874,14 @@ createparam(char *name, int flags)
 	DPUTS(oldpm && oldpm->level > locallevel,
 	      "BUG: old local parameter not deleted");
 	if (oldpm && (oldpm->level == locallevel || !(flags & PM_LOCAL))) {
+	    if (isset(POSIXBUILTINS) && (oldpm->node.flags & PM_READONLY)) {
+		zerr("read-only variable: %s", name);
+		return NULL;
+	    }
 	    if (!(oldpm->node.flags & PM_UNSET) || (oldpm->node.flags & PM_SPECIAL)) {
 		oldpm->node.flags &= ~PM_UNSET;
 		if ((oldpm->node.flags & PM_SPECIAL) && oldpm->ename) {
-		    Param altpm = 
+		    Param altpm =
 			(Param) paramtab->getnode(paramtab, oldpm->ename);
 		    if (altpm)
 			altpm->node.flags &= ~PM_UNSET;
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 51ebc65..f4fb8ec 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -468,3 +468,20 @@
 0:retying arrays to same array works
 >foo bar
 >goo car
+
+ (
+   setopt POSIXBUILTINS
+   readonly pbro
+   print ${+pbro} >&2
+   (typeset pbro=3)
+   (pbro=4)
+   typeset -r pbro        # idempotent (no error)...
+   print ${+pbro} >&2     # ...so still readonly...
+   typeset +r pbro        # ...can't turn it off
+ )
+1:Readonly with POSIX_BUILTINS
+?0
+?(eval):5: read-only variable: pbro
+?(eval):6: read-only variable: pbro
+?0
+?(eval):9: read-only variable: pbro

Re: [BUG] Can't mark unset variables as read-only

[Mikael Magnusson]

On Wed, Apr 29, 2015 at 12:36 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 29 Apr 2015 08:35:43 +0200
> Martijn Dekker <martijn@inlv.org> wrote:
>> Unlike other shells, zsh can't mark an unset variable as read-only.
>
> Yes, the standard does indeed require that ability.  As you can imagine,
> something like this that completely breaks the normal programming model
> of variables (a variable can't have state if it's unset because it
> doesn't exist) is a nightmare to implement; however, it can at least be
> limited to the POSIXBUILTINS option and maybe the cases using that are
> simple enough that it mostly works.  I'm sure there are any number of
> strange edge cases, though.
>
> The output of "readonly -p" is still broken (doesn't show the unset
> variables in a fashion that can be used for restoring the current state)
> but it was anyway:
>
>   typeset -ar '*'
>   *=()
>
> Er, no, I don't think that's going to work.
>
> So that can be fixed separately.

I wanted the opposite thing the other day, sort of. I have a ZLE
widget that looks at $WIDGET, and I wanted to reuse it by calling it
from another widget, setting WIDGET to another value first. However,
ZLE makes $WIDGET readonly special, and I couldn't find any
combination of flags that let me make a non-readonly local WIDGET. Is
that possible?

% zle -N h; h() { local -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
% <press alt-h>
h: read-only variable: WIDGET
% zle -N h; h() { local +r -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
% <press alt-h>
h:local: WIDGET: can't change type of a special parameter
% zle -N h; h() { local WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
% <press alt-h>WIDGET=h

h: read-only variable: WIDGET

compared to
% () { readonly foo=3; () { local foo=5; echo $foo }; echo $foo }
5
3

-- 
Mikael Magnusson

Re: [BUG] Can't mark unset variables as read-only

[Mikael Magnusson]

On Wed, Apr 29, 2015 at 12:57 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On Wed, Apr 29, 2015 at 12:36 PM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
>> On Wed, 29 Apr 2015 08:35:43 +0200
>> Martijn Dekker <martijn@inlv.org> wrote:
>>> Unlike other shells, zsh can't mark an unset variable as read-only.
>>
>> Yes, the standard does indeed require that ability.  As you can imagine,
>> something like this that completely breaks the normal programming model
>> of variables (a variable can't have state if it's unset because it
>> doesn't exist) is a nightmare to implement; however, it can at least be
>> limited to the POSIXBUILTINS option and maybe the cases using that are
>> simple enough that it mostly works.  I'm sure there are any number of
>> strange edge cases, though.
>>
>> The output of "readonly -p" is still broken (doesn't show the unset
>> variables in a fashion that can be used for restoring the current state)
>> but it was anyway:
>>
>>   typeset -ar '*'
>>   *=()
>>
>> Er, no, I don't think that's going to work.
>>
>> So that can be fixed separately.
>
> I wanted the opposite thing the other day, sort of. I have a ZLE
> widget that looks at $WIDGET, and I wanted to reuse it by calling it
> from another widget, setting WIDGET to another value first. However,
> ZLE makes $WIDGET readonly special, and I couldn't find any
> combination of flags that let me make a non-readonly local WIDGET. Is
> that possible?
>
> % zle -N h; h() { local -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
> % <press alt-h>
> h: read-only variable: WIDGET
> % zle -N h; h() { local +r -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
> % <press alt-h>
> h:local: WIDGET: can't change type of a special parameter
> % zle -N h; h() { local WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
> % <press alt-h>WIDGET=h
>
> h: read-only variable: WIDGET
>
> compared to
> % () { readonly foo=3; () { local foo=5; echo $foo }; echo $foo }
> 5
> 3

Even when you think about something for a couple of days, you don't
realize what the answer is until after you send the question to a
public forum.

ZLE makes the scope of the WIDGET parameter start inside the widget
function already (which is confusing), so it is already local. The
answer is to start another parameter scope first,
% zle -N h; h() { () { local -h WIDGET; WIDGET=hi; echo $WIDGET } };
bindkey '^[h' h
% <press alt-h>hi

-- 
Mikael Magnusson

Re: [BUG] Can't mark unset variables as read-only

[Martijn Dekker]

Peter Stephenson schreef op 29-04-15 om 12:36:
> As you can imagine,
> something like this that completely breaks the normal programming model
> of variables (a variable can't have state if it's unset because it
> doesn't exist) is a nightmare to implement;

FWIW, I imagine a simpler way might be, rather than implementing
read-only as an attribute of variables themselves, to keep a separate
table of variable names marked as read-only which is checked against
upon variable assignment.

I know absolutely nothing about the zsh codebase, so feel free to laugh
at my suggestion. My interest is in getting the most out of
cross-platform shell programming, and I'm really only here to report a bug.

Many thanks for the patch. I'll apply it, test it, and report back.

(Also, my apologies for the initial duplicate. I thought it hadn't gone
through the first time.)

- Martijn

Re: [BUG] Can't mark unset variables as read-only

[Bart Schaefer]

On Apr 29, 11:36am, Peter Stephenson wrote:
}
} +Note that in zsh (unlike other shells) it is still possible
} +to create a local variable of the same name as this is considered a
} +different variable (though this variable, too, can be marked readonly).

Hrm, I wonder if that, combined with "typeset" automatically creating
locals when called in a function context (also a POSIX incompatibility?) 
defeats the whole purpose of an unset readonly?

Actually it probably undermines readonly for variables that are set, too,
so I probably shouldn't worry about it because it isn't changing.

Re: [BUG] Can't mark unset variables as read-only

[Peter Stephenson]

On Wed, 29 Apr 2015 06:55:56 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:

> On Apr 29, 11:36am, Peter Stephenson wrote:
> }
> } +Note that in zsh (unlike other shells) it is still possible
> } +to create a local variable of the same name as this is considered a
> } +different variable (though this variable, too, can be marked readonly).
> 
> Hrm, I wonder if that, combined with "typeset" automatically creating
> locals when called in a function context (also a POSIX incompatibility?) 
> defeats the whole purpose of an unset readonly?

I don't think so... bash makes these local, too:

$ foo=bar
$ fn() { typeset foo=something_else; echo $foo; }
$ fn
something_else
$ echo $foo
bar

So "foo" in fn is logically something entirely different and I don't
think there's any requirement for it to be treated as readonly;
it's entirely a matter for the definition of "fn".

pws

Re: [BUG] Can't mark unset variables as read-only

[Chet Ramey]

On 4/29/15 10:41 AM, Peter Stephenson wrote:
> On Wed, 29 Apr 2015 06:55:56 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> 
>> On Apr 29, 11:36am, Peter Stephenson wrote:
>> }
>> } +Note that in zsh (unlike other shells) it is still possible
>> } +to create a local variable of the same name as this is considered a
>> } +different variable (though this variable, too, can be marked readonly).
>>
>> Hrm, I wonder if that, combined with "typeset" automatically creating
>> locals when called in a function context (also a POSIX incompatibility?) 

It's not a Posix incompatibility; Posix does not specify typeset or local
variables at all.  The Posix-conformant ways of declaring variables in a
function result in global variables, as Posix specifies.

>> defeats the whole purpose of an unset readonly?

The question is whether or not you can use local variables to `shadow'
readonly variables in a previous scope.  Bash doesn't allow you to do
that.  Given this script:

readonly foo=immutable

func()
{
	local foo=bar
	echo inside: $foo
}

func
echo outside: $foo

Bash gives the following output:

./x26: line 5: local: foo: readonly variable
inside: immutable
outside: immutable

If the purpose of readonly is to make a particular name/value pair
immutable, I think that allowing a local variable to shadow it,
especially if you're going to export that local variable, is a bad thing.

> 
> I don't think so... bash makes these local, too:
> 
> $ foo=bar
> $ fn() { typeset foo=something_else; echo $foo; }
> $ fn
> something_else
> $ echo $foo
> bar
> 
> So "foo" in fn is logically something entirely different and I don't
> think there's any requirement for it to be treated as readonly;

There's no requirement, since there are no standards for local variables
or variable scoping.  Bash does what I think is right.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: [BUG] Can't mark unset variables as read-only

[Stephane Chazelas]

2015-04-29 11:33:10 -0400, Chet Ramey:
[...]
> func()
> {
> 	local foo=bar
> 	echo inside: $foo
> }
> 
> func
> echo outside: $foo
> 
> Bash gives the following output:
> 
> ./x26: line 5: local: foo: readonly variable
> inside: immutable
> outside: immutable
> 
> If the purpose of readonly is to make a particular name/value pair
> immutable, I think that allowing a local variable to shadow it,
> especially if you're going to export that local variable, is a bad thing.
[...]

I don't agree.

The whole point of having local variables is to avoid problems
with clashing namespaces.


You can have:

f() {
  local foo=1 # f's own variable
  g xxx
  echo "$foo"
}

g() {
  local foo=2
  blah blah
}

(f and g possibly in different libraries)

By declaring foo local, you make sure that you're not clashing
with someone else's foo. 

You don't have to worry on how you name your functions like you
do in POSIX sh (where you'd need to call one variable f_foo, and
the other g_foo for intance).

Above, if you do:

f() {
  readonly foo=2
  g xxx
  echo "$foo"
}

You're basically breaking g for no good reason. f's doesn't want
the value of f to be modified, but g would not have modified it
since it's declaring its *own* foo variable.

Having said that, I don't remember ever using "readonly" in a
script so I can't say I care much.

-- 
Stephane

Re: [BUG] Can't mark unset variables as read-only

[Chet Ramey]

On 4/29/15 3:09 PM, Stephane Chazelas wrote:
> 2015-04-29 11:33:10 -0400, Chet Ramey:
> [...]
>> func()
>> {
>> 	local foo=bar
>> 	echo inside: $foo
>> }
>>
>> func
>> echo outside: $foo
>>
>> Bash gives the following output:
>>
>> ./x26: line 5: local: foo: readonly variable
>> inside: immutable
>> outside: immutable
>>
>> If the purpose of readonly is to make a particular name/value pair
>> immutable, I think that allowing a local variable to shadow it,
>> especially if you're going to export that local variable, is a bad thing.
> [...]
> 
> I don't agree.

OK, we can disagree.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: [BUG] Can't mark unset variables as read-only

[Bart Schaefer]

On Apr 29, 11:33am, Chet Ramey wrote:
} Subject: Re: [BUG] Can't mark unset variables as read-only
}
} > On Wed, 29 Apr 2015 06:55:56 -0700
} > Bart Schaefer <schaefer@brasslantern.com> wrote:
} > 
} >> On Apr 29, 11:36am, Peter Stephenson wrote:
} >> }
} >> } +Note that in zsh (unlike other shells) it is still possible
} >> } +to create a local variable of the same name as this is considered a
} >> } +different variable (though this variable, too, can be marked readonly).
} >>
} >> Hrm, I wonder if that, combined with "typeset" automatically creating
} >> locals when called in a function context (also a POSIX incompatibility?) 
} 
} It's not a Posix incompatibility; Posix does not specify typeset or local
} variables at all.  The Posix-conformant ways of declaring variables in a
} function result in global variables, as Posix specifies.

Thanks for refreshing my memory there.
 
} >> defeats the whole purpose of an unset readonly?
} 
} The question is whether or not you can use local variables to `shadow'
} readonly variables in a previous scope.  Bash doesn't allow you to do
} that.

This discussion has reminded me another thing I forgot:  for variables
that have defined meaning to the shell ("special" variables) zsh allows
shadowing of a readonly special only if the "-h" (hide) flag is passed
to "local" or one of its equivalents (integer, declare, etc.).

} If the purpose of readonly is to make a particular name/value pair
} immutable, I think that allowing a local variable to shadow it,
} especially if you're going to export that local variable, is a bad thing.

Here's a curious data point:  In ksh, functions declared with POSIX
syntax "name() { ...; }" treat readonly from the surrounding scope the
way bash does.  Those declared "function name { ...; }" treat readonly
the way zsh does, allowing shadowing in the local scope.

Maybe zsh needs yet another POSIX_* option.