On 10/02/2025 21:46, Sertonix wrote:
On Mon Feb 10, 2025 at 10:37 PM CET, Harald van Dijk wrote:
On 10/02/2025 21:15, Sertonix wrote:
On Mon Feb 10, 2025 at 10:08 PM CET, Harald van Dijk wrote:
Hi,
On 10/02/2025 19:14, Sertonix wrote:
The test-test example in the comment wasn't actually fixed before as
one can see by running
env -i "test-test=test" busybox ash -c 'export -p'
This previously incorrectly resulted in
export test
You're right that that output is incorrect. However...
But now it correctly results in
export 'test-test=test'
...this is not the correct result either. As the comment in the code
says, the output is meant to be usable for 'eval'. This output is not
usable for 'eval' and would result in a "bad variable name" error.
This is not the case. I changed the variable name checking to be more
relaxed for export so it works as expected. You can try this to verify:
env -i "test-test=test" busybox ash -c 'eval "$(export -p)"'
Oh, apologies, I missed that, I tested the output in an unpatched
busybox instead. I did not see in your message that you changed that as
well.
That change still looks wrong: unless there is some special permission
that I am not seeing, 'export test-test=test' is supposed to either
result in an error, or in the specified variable being assigned in the
way that POSIX specifies variable assignments work. The way POSIX
specifies variable assignments work mean the variable behaves like any
other variable. The syntax might not always allow that variable to be
referenced, but consider
env -i ./busybox ash -c 'export foo=bar baz-=quux; export -p; set'
*If* the assignment of baz- succeeds, then the requirements for 'export'
say that it becomes a shell variable, and the requirements for 'set' say
it gets printed there as well. That does not happen with your patch.
The POSIX doc for set (and only for set) states that "the output shall
be suitable for reinput to the shell, setting or resetting, as far as
possible". The way I read this is that skipping a variable name that
can't be parsed again would be ok.
There is no provision that permits variables to be skipped though. It
simultaneously requires that all variables be printed, and that they be
printed in a format that allows reinput into the shell. The
impossibility of printing baz-=quux in a way that allows reinput into
the shell does not negate the requirement that "set shall write the
names and values of all shell variables in the collation sequence of the
current locale."
There are two ways around that conflict: either the shell can act in a
way in which baz- is never accepted as anything that makes it a shell
variable name, or the shell can add some extended syntax to permit
variables with such a name to be set so that the set command can print
them. This would be similar to how you extended the 'export' command to
print such variable names.
That said, even if you are not convinced on 'set', there is another way
to show that this patch is problematic: it also affects the printing and
argument processing of the 'readonly' command. Consider:
$ env test-test=test ./busybox ash -c 'readonly test-test; readonly -p'
readonly 'test-test=test'
$ ./busybox ash -c "readonly 'test-test=test'"
ash: readonly: line 0: test-test: bad variable name
I believe my below comment still applies, the variable needs to be
skipped when printing instead.
The variable needs to be skipped when printing instead.
Cheers,
Harald van Dijk
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
