[PATCH v2] grub-core/kern/corecmd: Quote variable values when displayed by the set command

[PATCH v2] grub-core/kern/corecmd: Quote variable values when displayed by the set command

[Glenn Washburn]

Variable values may contain spaces at the end or newlines. However, when
displayed without quotes this is not obvious and can lead to confusion as
to the actual contents of variables. Also for some variables grub_env_get()
returns a NULL pointer instead of a pointer to an empty string and
previously would be printed as 'var=(null)'. Now such variables will be
displayed as 'var=""'.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/kern/corecmd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/grub-core/kern/corecmd.c b/grub-core/kern/corecmd.c
index fc54f43f2..10b595d6b 100644
--- a/grub-core/kern/corecmd.c
+++ b/grub-core/kern/corecmd.c
@@ -40,7 +40,10 @@ grub_core_cmd_set (struct grub_command *cmd __attribute__ ((unused)),
     {
       struct grub_env_var *env;
       FOR_SORTED_ENV (env)
-	grub_printf ("%s=%s\n", env->name, grub_env_get (env->name));
+	{
+	  val = grub_env_get (env->name);
+	  grub_printf ("%s=\"%s\"\n", env->name, val ? val : "");
+	}
       return 0;
     }
 
-- 
2.34.1

Re: [PATCH v2] grub-core/kern/corecmd: Quote variable values when displayed by the set command

[Daniel Kiper]

On Fri, Aug 19, 2022 at 05:38:24PM -0500, Glenn Washburn wrote:
> Variable values may contain spaces at the end or newlines. However, when
> displayed without quotes this is not obvious and can lead to confusion as
> to the actual contents of variables. Also for some variables grub_env_get()
> returns a NULL pointer instead of a pointer to an empty string and
> previously would be printed as 'var=(null)'. Now such variables will be
> displayed as 'var=""'.

Better but...

> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/kern/corecmd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/corecmd.c b/grub-core/kern/corecmd.c
> index fc54f43f2..10b595d6b 100644
> --- a/grub-core/kern/corecmd.c
> +++ b/grub-core/kern/corecmd.c
> @@ -40,7 +40,10 @@ grub_core_cmd_set (struct grub_command *cmd __attribute__ ((unused)),
>      {
>        struct grub_env_var *env;
>        FOR_SORTED_ENV (env)
> -	grub_printf ("%s=%s\n", env->name, grub_env_get (env->name));
> +	{
> +	  val = grub_env_get (env->name);
> +	  grub_printf ("%s=\"%s\"\n", env->name, val ? val : "");

Maybe I am overzealous but what will happen if value contains "$"
character, e.g. "$var", and somebody wants copy-pasta this value with
"" around?  I think '' instead of "" would be better here. IIRC '' works
in GRUB shell like it works in regular shell.

Additionally, what if the value contains '"' characters? Should we
escape them?

And a nit... I prefer s/val ?/(val != NULL) ?/.

Daniel

Re: [PATCH v2] grub-core/kern/corecmd: Quote variable values when displayed by the set command

[Glenn Washburn]

On Sat, 20 Aug 2022 01:07:35 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Fri, Aug 19, 2022 at 05:38:24PM -0500, Glenn Washburn wrote:
> > Variable values may contain spaces at the end or newlines. However, when
> > displayed without quotes this is not obvious and can lead to confusion as
> > to the actual contents of variables. Also for some variables grub_env_get()
> > returns a NULL pointer instead of a pointer to an empty string and
> > previously would be printed as 'var=(null)'. Now such variables will be
> > displayed as 'var=""'.
> 
> Better but...
> 
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/kern/corecmd.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/kern/corecmd.c b/grub-core/kern/corecmd.c
> > index fc54f43f2..10b595d6b 100644
> > --- a/grub-core/kern/corecmd.c
> > +++ b/grub-core/kern/corecmd.c
> > @@ -40,7 +40,10 @@ grub_core_cmd_set (struct grub_command *cmd __attribute__ ((unused)),
> >      {
> >        struct grub_env_var *env;
> >        FOR_SORTED_ENV (env)
> > -	grub_printf ("%s=%s\n", env->name, grub_env_get (env->name));
> > +	{
> > +	  val = grub_env_get (env->name);
> > +	  grub_printf ("%s=\"%s\"\n", env->name, val ? val : "");
> 
> Maybe I am overzealous but what will happen if value contains "$"
> character, e.g. "$var", and somebody wants copy-pasta this value with
> "" around?  I think '' instead of "" would be better here. IIRC '' works
> in GRUB shell like it works in regular shell.

Sure, single quotes seem good.

> 
> Additionally, what if the value contains '"' characters? Should we
> escape them?

Yes, this would be ideal. I don't really want to be the one to implement
this though. The use case that I care about it seeing when variables
have whitespace at the end of the value. This is more for debugging
GRUB script that is _not_ created with the intention of making the
output of set trick the user. Also I don't have grub script in
variables (though this could be reasonable with someone using exec), so
its not an issue for me.

> 
> And a nit... I prefer s/val ?/(val != NULL) ?/.

Yep, of course.

Glenn