Re: escape strings in grub.cfg
On Mon, Apr 12, 2010 at 10:19 PM, BVK Chaitanya wrote: > On Mon, Apr 12, 2010 at 5:39 AM, Colin Watson wrote: >> >> In other words, only ' is special (as a terminator) when reading a >> single-quoted string, and the sequences " $ \\ \" \n are special inside >> a double-quoted string. (Incidentally, I think there's another bug >> here; if $ is special, \$ should be special as well.) >> > > > Nice catch! There is indeed a bug. We cannot escape $ in dquote > strings currently, this needs to be fixed. > FYI, this bug is fixed in mainline. -- bvk.chaitanya ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: escape strings in grub.cfg
Hi,
On Apr/13/2010, Colin Watson wrote:
> On Mon, Apr 12, 2010 at 01:09:06AM +0100, Colin Watson wrote:
> > With my suggestion, I'd recommend:
> >
> > gettext_quoted () {
> > gettext "$@" | sed "s/'/'''/g"
> > }
> >
> > ... and then make sure it's enclosed in '' rather than "" wherever it's
> > used (or modify the sed to put single-quotes at the start and end, if
> > that works out neater).
>
> I've gone ahead and committed a patch based on this to trunk, and I'll
> backport it to Ubuntu Lucid shortly as well. Testing welcome!
thanks Colin, I'll do some tests as well.
--
Carles Pina i Estany
http://pinux.info
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: escape strings in grub.cfg
On Mon, Apr 12, 2010 at 01:09:06AM +0100, Colin Watson wrote:
> With my suggestion, I'd recommend:
>
> gettext_quoted () {
> gettext "$@" | sed "s/'/'''/g"
> }
>
> ... and then make sure it's enclosed in '' rather than "" wherever it's
> used (or modify the sed to put single-quotes at the start and end, if
> that works out neater).
I've gone ahead and committed a patch based on this to trunk, and I'll
backport it to Ubuntu Lucid shortly as well. Testing welcome!
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: escape strings in grub.cfg
On Mon, Apr 12, 2010 at 12:23:01PM -0500, richardvo...@gmail.com wrote:
> On Sun, Apr 11, 2010 at 7:09 PM, Colin Watson wrote:
> > On Sat, Apr 10, 2010 at 10:10:15PM +0100, Carles Pina i Estany wrote:
> >> +gettext_escape_double_quotes ()
> >> +{
> >> + echo -n $(gettext $@) | sed 's/\"/\\\"/'
> >> +}
> >
> > This is underquoted, and in any case the 'echo -n' is redundant since
> > gettext itself already writes to standard output and $() substitution
> > strips trailing newlines. $@ requires special treatment, and that sed
> > s/// command needs /g. With my suggestion, I'd recommend:
> >
> > gettext_quoted () {
> > gettext "$@" | sed "s/'/'''/g"
> > }
>
> I haven't tried it myself, but you just said the newline is stripped
> by $() and then took out the $(), seems like the newline would be
> present again.
In context, gettext_quoted would always be called from inside $(), so
this isn't a problem.
(In general I find that it's best to write shell functions such that
they emit trailing newlines on their output, and let the caller strip it
if necessary; this usually comes out quite naturally. Of course there
are exceptions.)
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: escape strings in grub.cfg
On Sun, Apr 11, 2010 at 7:09 PM, Colin Watson wrote:
> On Sat, Apr 10, 2010 at 10:10:15PM +0100, Carles Pina i Estany wrote:
>> Last weekend, the Asturian team found a bug in Grub. Thanks for the
>> debugging. The string that appears in grub.cfg for "Loading the initram"
>> in Asturian is "Cargando'l discu RAM inicial...". So grub.cfg looks
>> like:
>> echo Cargando'l discu RAM inicial...
>>
>> This is syntactically invalid because the character "'".
>>
>> Easy work around: generate the line like:
>> echo "Cargando'l discu RAM inicial..."
>>
>> New problem: in some language, this string could have a double quote
>> inside.
>>
>> New solution: escape the double quotes strings when they are used in
>> gettext and always use these strings with double quotes.
>>
>> See the attached patch.
>>
>> I'm not convinced of the patch, does anybody has a cleaner idea/patch?
>
> This doesn't cover everything - you've just moved the goalposts a bit.
> Here's the relevant bit of the lexer:
>
> {
> \' {
> yy_pop_state (yyscanner);
> ARG (GRUB_SCRIPT_ARG_TYPE_SQSTR);
> }
> [^\']+ { COPY (yytext, yyleng); }
> }
>
> {
> \" {
> yy_pop_state (yyscanner);
> ARG (GRUB_SCRIPT_ARG_TYPE_DQSTR);
> }
> \$ {
> yy_push_state (VAR, yyscanner);
> ARG (GRUB_SCRIPT_ARG_TYPE_DQSTR);
> }
> { COPY ("\\", 1); }
> \\\" { COPY ("\"", 1); }
> \\\n { /* ignore */ }
> [^\"$\\\n]+ { COPY (yytext, yyleng); }
> (.|\n) { COPY (yytext, yyleng); }
> }
>
> In other words, only ' is special (as a terminator) when reading a
> single-quoted string, and the sequences " $ \\ \" \n are special inside
> a double-quoted string. (Incidentally, I think there's another bug
> here; if $ is special, \$ should be special as well.)
>
> Given this, wouldn't you be better off using single-quotes instead?
> You'd then end up with:
>
> echo 'Cargando'\''l discu RAM inicial...'
>
> ... where the construction used to escape ' is "close quotes, literal ',
> reopen quotes". (Tested in grub-emu.)
>
>> +gettext_escape_double_quotes ()
>> +{
>> + echo -n $(gettext $@) | sed 's/\"/\\\"/'
>> +}
>
> This is underquoted, and in any case the 'echo -n' is redundant since
> gettext itself already writes to standard output and $() substitution
> strips trailing newlines. $@ requires special treatment, and that sed
> s/// command needs /g. With my suggestion, I'd recommend:
>
> gettext_quoted () {
> gettext "$@" | sed "s/'/'''/g"
> }
I haven't tried it myself, but you just said the newline is stripped
by $() and then took out the $(), seems like the newline would be
present again.
>
> ... and then make sure it's enclosed in '' rather than "" wherever it's
> used (or modify the sed to put single-quotes at the start and end, if
> that works out neater).
>
> --
> Colin Watson [cjwat...@ubuntu.com]
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: escape strings in grub.cfg
On Mon, Apr 12, 2010 at 5:39 AM, Colin Watson wrote: > > In other words, only ' is special (as a terminator) when reading a > single-quoted string, and the sequences " $ \\ \" \n are special inside > a double-quoted string. (Incidentally, I think there's another bug > here; if $ is special, \$ should be special as well.) > Nice catch! There is indeed a bug. We cannot escape $ in dquote strings currently, this needs to be fixed. -- bvk.chaitanya ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: escape strings in grub.cfg
On Sat, Apr 10, 2010 at 10:10:15PM +0100, Carles Pina i Estany wrote:
> Last weekend, the Asturian team found a bug in Grub. Thanks for the
> debugging. The string that appears in grub.cfg for "Loading the initram"
> in Asturian is "Cargando'l discu RAM inicial...". So grub.cfg looks
> like:
> echoCargando'l discu RAM inicial...
>
> This is syntactically invalid because the character "'".
>
> Easy work around: generate the line like:
> echo"Cargando'l discu RAM inicial..."
>
> New problem: in some language, this string could have a double quote
> inside.
>
> New solution: escape the double quotes strings when they are used in
> gettext and always use these strings with double quotes.
>
> See the attached patch.
>
> I'm not convinced of the patch, does anybody has a cleaner idea/patch?
This doesn't cover everything - you've just moved the goalposts a bit.
Here's the relevant bit of the lexer:
{
\'{
yy_pop_state (yyscanner);
ARG (GRUB_SCRIPT_ARG_TYPE_SQSTR);
}
[^\']+{ COPY (yytext, yyleng); }
}
{
\"{
yy_pop_state (yyscanner);
ARG (GRUB_SCRIPT_ARG_TYPE_DQSTR);
}
\${
yy_push_state (VAR, yyscanner);
ARG (GRUB_SCRIPT_ARG_TYPE_DQSTR);
}
{ COPY ("\\", 1); }
\\\" { COPY ("\"", 1); }
\\\n { /* ignore */ }
[^\"$\\\n]+ { COPY (yytext, yyleng); }
(.|\n){ COPY (yytext, yyleng); }
}
In other words, only ' is special (as a terminator) when reading a
single-quoted string, and the sequences " $ \\ \" \n are special inside
a double-quoted string. (Incidentally, I think there's another bug
here; if $ is special, \$ should be special as well.)
Given this, wouldn't you be better off using single-quotes instead?
You'd then end up with:
echo'Cargando'\''l discu RAM inicial...'
... where the construction used to escape ' is "close quotes, literal ',
reopen quotes". (Tested in grub-emu.)
> +gettext_escape_double_quotes ()
> +{
> + echo -n $(gettext $@) | sed 's/\"/\\\"/'
> +}
This is underquoted, and in any case the 'echo -n' is redundant since
gettext itself already writes to standard output and $() substitution
strips trailing newlines. $@ requires special treatment, and that sed
s/// command needs /g. With my suggestion, I'd recommend:
gettext_quoted () {
gettext "$@" | sed "s/'/'''/g"
}
... and then make sure it's enclosed in '' rather than "" wherever it's
used (or modify the sed to put single-quotes at the start and end, if
that works out neater).
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
escape strings in grub.cfg
Hello,
Last weekend, the Asturian team found a bug in Grub. Thanks for the
debugging. The string that appears in grub.cfg for "Loading the initram"
in Asturian is "Cargando'l discu RAM inicial...". So grub.cfg looks
like:
echoCargando'l discu RAM inicial...
This is syntactically invalid because the character "'".
Easy work around: generate the line like:
echo"Cargando'l discu RAM inicial..."
New problem: in some language, this string could have a double quote
inside.
New solution: escape the double quotes strings when they are used in
gettext and always use these strings with double quotes.
See the attached patch.
I'm not convinced of the patch, does anybody has a cleaner idea/patch?
Cheers,
PS: I haven't double checked, but lines like initrd
/boot/initrd.img-2.6.32-18-generic will fail if the path has spaces. I'll
address it in the future.
PS2: I haven't checked the patch yet, let's comment on the idea. Before
committing I would write the changelog and check it.
--
Carles Pina i Estany
http://pinux.info
=== modified file 'util/grub-mkconfig_lib.in'
--- util/grub-mkconfig_lib.in 2010-02-25 13:30:50 +
+++ util/grub-mkconfig_lib.in 2010-04-10 20:47:18 +
@@ -188,3 +188,8 @@ version_find_latest ()
done
echo "$a"
}
+
+gettext_escape_double_quotes ()
+{
+ echo -n $(gettext $@) | sed 's/\"/\\\"/'
+}
=== modified file 'util/grub.d/10_linux.in'
--- util/grub.d/10_linux.in 2010-04-08 09:54:44 +
+++ util/grub.d/10_linux.in 2010-04-10 20:49:06 +
@@ -56,9 +56,9 @@ linux_entry ()
recovery="$3"
args="$4"
if ${recovery} ; then
-title="$(gettext "%s, with Linux %s (recovery mode)")"
+title="$(gettext_escape_double_quotes "%s, with Linux %s (recovery mode)")"
else
-title="$(gettext "%s, with Linux %s")"
+title="$(gettext_escape_double_quotes "%s, with Linux %s")"
fi
printf "menuentry \"${title}\" ${CLASS} {\n" "${os}" "${version}"
save_default_entry | sed -e "s/^/\t/"
@@ -83,12 +83,12 @@ EOF
fi
printf '%s\n' "${prepare_boot_cache}"
cat << EOF
- echo $(printf "$(gettext "Loading Linux %s ...")" ${version})
+ echo \"$(printf "$(gettext_escape_double_quotes "Loading Linux %s ...")" ${version})\"
linux ${rel_dirname}/${basename} root=${linux_root_device_thisversion} ro ${args}
EOF
if test -n "${initrd}" ; then
cat << EOF
- echo $(gettext "Loading initial ramdisk ...")
+ echo \"$(gettext_escape_double_quotes "Loading initial ramdisk ...")\"
initrd ${rel_dirname}/${initrd}
EOF
fi
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
