Error in translation



  •  And here's our version of the translate() function:

    <font face="courier new,courier">function translate ($tag)
    {
            require_once('core/template_reader.php');
            //get language translation from db
            $templateReader = TemplateReader::getInstance();
            $translation = $templateReader->getOldTranslation($tag);
            if (!$translation)
                    $translation = $tag;


            //see if we need to use special formatting
            $funcArgs = func_get_args();
            //case 1: no special formatting
        if (count($funcArgs) == 1)
            return $translation;
        //case 2: it needs some formatting
        unset ($funcArgs[0]);
        $lsarg = join("', '", $funcArgs);
        $command = "\$trans = sprintf('$translation', '$lsarg');";
        eval($command);
        return $trans;
    }</font>

    This piece of code made me wonder, if we should start something like The "The idiot of the month award" award, a complement for The "The employee of the month award" award.



  • What's the WTF? I might have written something like this -- reasoning follows.
    First, we want to retrieve the translation message.

    After retrieving the translation, we simply check if there are NO fill-ins. If so, its done, and the expensive formatting doesn't need doing.

    If we do have fill-in, we want to push them into the translated string. I don't know if this language allows passing on the passed parameters, if there are a variable number of them, but, if I know that I can evaluate a string, I can build the appropriate structure.

    This is executed, and should result in the desired message.

    There may be bugs in the procedure (can't comment on that), but the WTF would be - the procedure does not accomodate changing the order of the fill-ins, needed by some languages. Or, there is an existing process that the developer was not aware of? Or, it has bug(s) and doesn't work?

    File this under "!!value", it needs additional information to understand the WTF.

     



  • @ratboy667 said:

    File this under "!!value", it needs additional information to understand the WTF.
     

    Isn't using the eval() function enough of a WTF?

    In PHP, there's a function vsprintf() which is about the same as sprintf() but which accepts an array as a second argument, instead of a variable number of arguments. The last four lines could have been simply return vsprintf($translation, $funcArgs), because this way we would be allowed to add apostrophes in the texts without the need of escaping them. This leads us to the second WTF - there's no sanitization done in any way; if one of the texts contains an apostrophe, the whole eval() will get fcked up. Also, there's always func_num_args() for getting the number of arguments given to a PHP function, without the need of actually retrieving those parameters as an array (allocating a variable) and then finding the length of that array with the help of another function call, especially since very few messages actually needed parameters.



  • I agree that using eval() instead of vsprintf() is a WTF, but it doesn't make the entire function a WTF. The bit with doing count(func_get_args()) is just an optimization issue and is something anybody could do by accident.

    The structure of the function looks quite reasonable - in fact, I've written an equivalent function in one of my own projects and it works almost exactly the same way as this one (though using vsprintf() and func_num_args()) plus has a few simplifications (no database, just a big associative array declared in another file) and improvements (being able to treat parameters as additional parameterless tags).



  • @Quietust said:

    I agree that using eval() instead of vsprintf() is a WTF, but it doesn't make the entire function a WTF. The bit with doing count(func_get_args()) is just an optimization issue and is something anybody could do by accident.

    The structure of the function looks quite reasonable - in fact, I've written an equivalent function in one of my own projects and it works almost exactly the same way as this one (though using vsprintf() and func_num_args()) plus has a few simplifications (no database, just a big associative array declared in another file) and improvements (being able to treat parameters as additional parameterless tags).

    There is no database and the texts are stored in text files. The function, as a whole, is not a WTF, but the way it was implemented is quite bad. There might be just the issue that if there's no translation for the text, the original text is returned... Maybe I should also add something to log missing texts.



  • It looks perfectly normal to me.  It's a system which has grown over time and is now able to arbitrarily translate almost anything.  The WTF count is low although evaluating code in translation files does expose the system to command injections, if the translation files can be modified from outside.  Hopefully this isn't the case.



  • @Quietust said:

    I agree that using eval() instead of vsprintf() is a WTF, ...

    Actually it would not be so easy here. The list of arguments contains variable names (might even contain expressions!) and eval will substitute their values. So every value in the array would first have to be evaluated on it's own before passing it to vsprintf.

    As long as the translations are not read from untrusted source (which would be an extremely big security hole), using eval is somewhat involved, but extremely convenient way to do things here.



  • @Bulb said:

    @Quietust said:

    I agree that using eval() instead of vsprintf() is a WTF, ...

    Actually it would not be so easy here. The list of arguments contains variable names (might even contain expressions!) and eval will substitute their values. So every value in the array would first have to be evaluated on it's own before passing it to vsprintf.

    As long as the translations are not read from untrusted source (which would be an extremely big security hole), using eval is somewhat involved, but extremely convenient way to do things here.

     

    And why, oh, why would you do that? If you get to fully control what you put in, why would you mash it up and add variables, just so that they would be evaluated? At the risk of messing up half of the display? Who would expect a simple translation function that tries to imitate printf() to parse variables? Why would I have to sanitize what I feed a translation function, so that the code won't fuck up?



  • @Bulb said:

    @Quietust said:

    I agree that using eval() instead of vsprintf() is a WTF, ...

    Actually it would not be so easy here. The list of arguments contains variable names (might even contain expressions!) and eval will substitute their values. So every value in the array would first have to be evaluated on it's own before passing it to vsprintf.

    As long as the translations are not read from untrusted source (which would be an extremely big security hole), using eval is somewhat involved, but extremely convenient way to do things here.

    Using eval for this is absolutely a WTF.  Using eval for pretty much anything is a massive WTF. 



  • @morbiuswilters said:

    Using eval for this is absolutely a WTF.  Using eval for pretty much anything is a massive WTF. 
    I've been meaning to write a post... Expect something soon, possibly over the weekend.

    Meanwhile, here's an appetizer:

    $sTmp = str_replace(array("|","^"),array(" || "," && "),PostfixToInfix2(InfixToPostfix($query),$fileContents,$filename));
    eval("if($sTmp) \$itmp = '1'; else \$itmp = '2';");
                                 
    if ($itmp == '1') {
        $arrFnd[ ] = $row['candidate_id'];
    }

    // $sTmp and $itmp no longer used beyond this point



  • @Zecc said:

    Filed under: [url=http://forums.thedailywtf.com/tags/How+can+I+write+5B005D00+3F00+Oh+_2600_amp_3B00_nbsp/default.aspx]How can I write []] ?[/url]

    Simple: []].



  • @rohypnol said:

    And why, oh, why would you do that? If you get to fully control what you put in, why would you mash it up and add variables, just so that they would be evaluated? At the risk of messing up half of the display? Who would expect a simple translation function that tries to imitate printf() to parse variables? Why would I have to sanitize what I feed a translation function, so that the code won't fuck up?

    There's definitely a huge WTF in here somewhere. It may not be this function though, it may be whatever made this function necessary.



  • @rohypnol said:

     And here's our version of the translate() function:

    <font face="courier new,courier">function translate ($tag)
    {
            require_once('core/template_reader.php');
            //get language translation from db
            $templateReader = TemplateReader::getInstance();
            $translation = $templateReader->getOldTranslation($tag);
            if (!$translation)
                    $translation = $tag;


            //see if we need to use special formatting
            $funcArgs = func_get_args();
            //case 1: no special formatting
        if (count($funcArgs) == 1)
            return $translation;
        //case 2: it needs some formatting
        unset ($funcArgs[0]);
        $lsarg = join("', '", $funcArgs);
        $command = "\$trans = sprintf('$translation', '$lsarg');";
        eval($command);
        return $trans;
    }</font>

    This piece of code made me wonder, if we should start something like The "The idiot of the month award" award, a complement for The "The employee of the month award" award.

     

    Am I the only one that whenever I see a post in this format:

     Here is xxxxxxxxxx:

    <block of code more than 5 lines long>

    Something not describing the problem.

     and I can't tell what is wrong with the code in the first 2 seconds, I just hit back on my browser?



  • @tster said:

    @rohypnol said:

     And here's our version of the translate() function:

    <font face="courier new,courier">function translate ($tag)
    {
            require_once('core/template_reader.php');
            //get language translation from db
            $templateReader = TemplateReader::getInstance();
            $translation = $templateReader->getOldTranslation($tag);
            if (!$translation)
                    $translation = $tag;


            //see if we need to use special formatting
            $funcArgs = func_get_args();
            //case 1: no special formatting
        if (count($funcArgs) == 1)
            return $translation;
        //case 2: it needs some formatting
        unset ($funcArgs[0]);
        $lsarg = join("', '", $funcArgs);
        $command = "\$trans = sprintf('$translation', '$lsarg');";
        eval($command);
        return $trans;
    }</font>

    This piece of code made me wonder, if we should start something like The "The idiot of the month award" award, a complement for The "The employee of the month award" award.

     

    Am I the only one that whenever I see a post in this format:

     Here is xxxxxxxxxx:

    <block of code more than 5 lines long>

    Something not describing the problem.

     and I can't tell what is wrong with the code in the first 2 seconds, I just hit back on my browser?

    TL; DR.


  • @DaveK said:

     TL; DR.

     

    What?



  • @tster said:

    @DaveK said:

     TL; DR.

     

    What?

    "Too Long, Didn't Read".  The alternative to hitting back in situations such as you were describing.


  • @Zecc said:

    I've been meaning to write a post... Expect something soon, possibly over the weekend.
    For the sake of linkage: [link]



  • @Zecc said:

    @Zecc said:

    I've been meaning to write a post... Expect something soon, possibly over the weekend.
    For the sake of linkage: [link]

    Yes, I am.  So?


  • @DaveK said:

    @Zecc said:

    @Zecc said:

    I've been meaning to write a post... Expect something soon, possibly over the weekend.
    For the sake of linkage: [link]

    Yes, I am.  So?
     

     Me too. So?

     



  • @seriousJoker said:

    @DaveK said:

    @Zecc said:

    @Zecc said:

    I've been meaning to write a post... Expect something soon, possibly over the weekend.
    For the sake of linkage: [link]

    Yes, I am.  So?

    Me too. So? 

    No reason.



  • @morbiuswilters said:

    @seriousJoker said:

    @DaveK said:

    @Zecc said:

    @Zecc said:

    I've been meaning to write a post... Expect something soon, possibly over the weekend.
    For the sake of linkage: [link]

    Yes, I am.  So?

    Me too. So? 

    No reason.

    TL;DR



  • @DaveK said:

    TL;DR!

    Consider upgrading to Firefox 3, iirc 2.0 has been unsupported for a while.

    I even installed FF3 on my microSD since university has a strange upgrade policy for software (e.g. web browser: Mozilla 1.6 => Firefox 2.0). If I'm lucky, they catch up with the latest version when I graduate in a few years.



  • @derula said:

    @DaveK said:
    TL;DR!

    Consider upgrading to Firefox 3, iirc 2.0 has been unsupported for a while.

    Do you know for a fact that it uses wider tool-tip windows?  I wouldn't want to upgrade for nothing.



  • @DaveK said:

    Do you know for a fact that it uses wider tool-tip windows?  I wouldn't want to upgrade for nothing.

    Yes, it's fixed in FF 3.



  • @Daniel Beardsmore said:

    @DaveK said:
    Do you know for a fact that it uses wider tool-tip windows?  I wouldn't want to upgrade for nothing.

    Yes, it's fixed in FF 3.

    Almost, that is. They still disappear faster than you can read what they say, forcing you to move the cursor away from and back over the link again.


Log in to reply