Re: Buglet in i18n?

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sat, Oct 23, 2010 at 18:29, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Oct 22, 2010 at 08:34, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> Now eval_gettext is supposed to just interpolate $variable
>>> substitutions, right?  In particular, the quotation marks
>>> ought to be preserved.
> [...]
>> That prints:
>>
>>     foo "bar baz"
>>
>> (with double quotes)
>>
>> But what we want is:
>>
>>     foo bar baz
> [...]
>> Have I got that right
>
> No, I don't think so.  Checking /usr/bin/gettext.sh, I see that it
> uses envsubst:
>
>        # Note: This use of envsubst is much safer than using the shell built-in 'eval'
>        # would be.
>        # 1) The security problem with Chinese translations that happen to use a
>        #    character such as \xe0\x60 is avoided.
>        # 2) The security problem with malevolent translators who put in command lists
>        #    like "$(...)" or "`...`" is avoided.
>        # 3) The translations can only refer to shell variables that are already
>        #    mentioned in MSGID or MSGID-PLURAL.
>
> And:
>
>        ; echo '"foo"' | envsubst
>        "foo"
>
> envsubst(1) has more details.
>
> The idea: translators do not have to worry about quoting at all.
> $var is presumably rare enough in messages as to not matter.
>
> One problem with my mockup: it makes it hard to talk about $5.00
> solutions, unlike envsubst:
>
>        ; echo '$3.00' | envsubst
>        $3.00

Sorry for the late reply. Yes, envsubst is the way to go. What I'm
going to do when I get around to it is to pull (the GPLv2 version of)
envsubst out of gettext.git and modify it to be a minimal
git-sh-i18n--helper command.

Then just do:

    diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
    index f8dd43a..c65f9ec 100644
    --- a/git-sh-i18n.sh
    +++ b/git-sh-i18n.sh
    @@ -55,8 +55,7 @@ then
                    }

                    eval_gettext () {
    -                       gettext_eval="printf '%s' \"$1\""
    -                       printf "%s" "`eval \"$gettext_eval\"`"
    +                       printf "%s" "$1" | git-sh-i18n--helper envsubst
                    }
            fi
     else

Along with this test:

    commit 42f2eabad4434875f3dd123844461ccfc4ad220b
    Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    Date:   Sat Oct 30 08:59:51 2010 +0000

        t/t0201-gettext-fallbacks.sh: test for broken eval_gettext

        Add a test for the broken eval_gettext() variable interpolation
        behavior.

        Reported-by: Johannes Sixt <j.sixt@viscovery.net>
        Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

    diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh
    index 7a85d9b..682c602 100755
    --- a/t/t0201-gettext-fallbacks.sh
    +++ b/t/t0201-gettext-fallbacks.sh
    @@ -46,4 +46,27 @@ test_expect_success NO_GETTEXT_POISON
'eval_gettext: our eval_gettext() fallback
         test_cmp expect actual
     '

    +test_expect_success NO_GETTEXT_POISON 'eval_gettext: our
eval_gettext() fallback can interpolate whitespace variables' '
    +    git_am_cmdline="git am" &&
    +    export git_am_cmdline &&
    +    printf "test git am" >expect &&
    +    eval_gettext "test \$git_am_cmdline" >actual &&
    +    test_cmp expect actual
    +'
    +
    +test_expect_success NO_GETTEXT_POISON 'eval_gettext: git am $cmdline bug' '
    +    cmdline="git am -3" &&
    +    export cmdline &&
    +    cat >expect <<EOF &&
    +When you have resolved this problem run "git am -3 --resolved".
    +If you would prefer to skip this patch, instead run "git am -3 --skip".
    +To restore the original branch and stop patching run "git am -3 --abort".
    +EOF
    +    eval_gettext "When you have resolved this problem run
\"\$cmdline --resolved\".
    +If you would prefer to skip this patch, instead run \"\$cmdline --skip\".
    +To restore the original branch and stop patching run \"\$cmdline
--abort\"." >actual &&
    +    echo >>actual &&
    +    test_cmp expect actual
    +'
    +
     test_done

The latter of which starts passing with envsubst.