Moodle
  1. Moodle
  2. MDL-26823

A question mark in the correct answer formula in calculated questions results in a php error

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.0.2
    • Fix Version/s: STABLE backlog
    • Component/s: Questions
    • Labels:
    • Database:
      Any
    • Affected Branches:
      MOODLE_20_STABLE
    • Rank:
      16441

      Description

      Created a simple calculated question in quiz. set the correct answer formula to

      {x}

      +

      {y}

      ?

      The presence of the question mark causes the following to be output.

      ( ! ) Parse error: syntax error, unexpected ';' in /home/andrew/Desktop/review/moodle/question/type/calculated/questiontype.php(1547) : eval()'d code on line 1
      Call Stack

      1. Time Memory Function Location
        1 0.0010 359896 {main}( ) ../question.php:0
        2 0.2373 46893840 default_questiontype->create_editing_form( ) ../question.php:180
        3 0.2403 47619192 question_edit_calculatedsimple_form->question_edit_calculatedsimple_form( ) ../questiontype.php:213
        4 0.2407 47644624 question_edit_form->question_edit_form( ) ../edit_calculatedsimple_form.php:271
        5 0.2410 47648200 moodleform->moodleform( ) ../edit_question_form.php:79
        6 0.2447 47870104 question_edit_form->definition( ) ../formslib.php:152
        7 0.2898 53262952 question_edit_calculatedsimple_form->definition_inner( ) ../edit_question_form.php:162
        8 0.3329 54410248 question_calculated_qtype->comment_on_datasetitems( ) ../edit_calculatedsimple_form.php:417
        9 0.3334 54417088 qtype_calculated_calculate_answer( ) ../questiontype.php:1322
        10 0.3334 54418152 question_calculated_qtype->substitute_variables_and_eval( ) ../questiontype.php:2183

        ( ! ) Parse error: syntax error, unexpected ';' in /home/andrew/Desktop/review/moodle/question/type/calculated/questiontype.php(1327) : eval()'d code on line 1
        Call Stack
        # Time Memory Function Location
        1 0.0010 359896 {main}

        ( ) ../question.php:0
        2 0.2373 46893840 default_questiontype->create_editing_form( ) ../question.php:180
        3 0.2403 47619192 question_edit_calculatedsimple_form->question_edit_calculatedsimple_form( ) ../questiontype.php:213
        4 0.2407 47644624 question_edit_form->question_edit_form( ) ../edit_calculatedsimple_form.php:271
        5 0.2410 47648200 moodleform->moodleform( ) ../edit_question_form.php:79
        6 0.2447 47870104 question_edit_form->definition( ) ../formslib.php:152
        7 0.2898 53262952 question_edit_calculatedsimple_form->definition_inner( ) ../edit_question_form.php:162
        8 0.3329 54410248 question_calculated_qtype->comment_on_datasetitems( ) ../edit_calculatedsimple_form.php:417

        Activity

        Hide
        Pierre Pichet added a comment -

        Ok I can reproduce
        Somehow surprising that users have not done this error i.e. adding a ? in the formula in the past as this code is the same since at least Moodle 1,6.
        Anyway, this should not give this error.

        On 1,9 UQAM moodle as there a no php notice allowed, this just give a final result of 0.
        I should add a new validation step in the edit_form to catch this at the first level.
        As we are now on php 5, a try and catch could be used on lines 1547, and 1327 .

        Show
        Pierre Pichet added a comment - Ok I can reproduce Somehow surprising that users have not done this error i.e. adding a ? in the formula in the past as this code is the same since at least Moodle 1,6. Anyway, this should not give this error. On 1,9 UQAM moodle as there a no php notice allowed, this just give a final result of 0. I should add a new validation step in the edit_form to catch this at the first level. As we are now on php 5, a try and catch could be used on lines 1547, and 1327 .
        Hide
        Pierre Pichet added a comment - - edited

        Tim,
        The calculated formula options were defined historocally in a very open mind for teachers that understand well php coding and that will always doing things correctly (no bad hacking).
        In function qtype_calculated_find_formula_errors($formula)
        there are a check of the functions used are included in a given list and is the user put the rigth number of parameters.
        However the allowed characters are the following
        $safeoperatorchar = '-+/*%>:^~<?=&|!';
        $operatorornumber = "[$safeoperatorchar.0-9eE]";
        so that you can test using ? : sequence like

        {a}-{b} ? {a}

        :

        {b}

        The function list is well shown in the docs and included functions like
        case 'is_finite': case 'is_infinite': case 'is_nan':
        which can be used is such a ? : structure.

        However I do not see in the docs that the ? : is documented.

        In the actual function qtype_calculated_find_formula_errors the ? : is not verified.

        Do we continue to support the ? : structure ?

        Also
        the formula is applied as

        eval('$str = '.$formula.';');

        How can I put this is a
        try

        { eval('$str = '.$formula.';'); }

        catch (Exception )

        { $str = 0 ; }

        as the exception will be made by the php compiler and will interact with the php error display ?

        Show
        Pierre Pichet added a comment - - edited Tim, The calculated formula options were defined historocally in a very open mind for teachers that understand well php coding and that will always doing things correctly (no bad hacking). In function qtype_calculated_find_formula_errors($formula) there are a check of the functions used are included in a given list and is the user put the rigth number of parameters. However the allowed characters are the following $safeoperatorchar = '-+/*%>:^~<?=&|!'; $operatorornumber = " [$safeoperatorchar.0-9eE] "; so that you can test using ? : sequence like {a}-{b} ? {a} : {b} The function list is well shown in the docs and included functions like case 'is_finite': case 'is_infinite': case 'is_nan': which can be used is such a ? : structure. However I do not see in the docs that the ? : is documented. In the actual function qtype_calculated_find_formula_errors the ? : is not verified. Do we continue to support the ? : structure ? Also the formula is applied as eval('$str = '.$formula.';'); How can I put this is a try { eval('$str = '.$formula.';'); } catch (Exception ) { $str = 0 ; } as the exception will be made by the php compiler and will interact with the php error display ?
        Hide
        Pierre Pichet added a comment - - edited

        Let's play the game of adding an additional pregmatch at the end when there are only numbers left

            if (preg_match("~[^$safeoperatorchar.0-9eE]+~", $formula, $regs)) {
                return get_string('illegalformulasyntax', 'quiz', $regs[0]);
            } else {
                
                // Formula just might be valid
               // adding additional test for ? : structure 
                return false;
            }
        

        preliminary results show that this can be done by something like

        ~([-+/*%>^\~<=&|!0-9eE]+)\?([-+/*%>^\~<=&|!0-9eE]+):([-+/*%>^\~<=&|!0-9eE]+)~
        

        after testing first that there is a ? : in the formula

        more later

        Show
        Pierre Pichet added a comment - - edited Let's play the game of adding an additional pregmatch at the end when there are only numbers left if (preg_match("~[^$safeoperatorchar.0-9eE]+~", $formula, $regs)) { return get_string('illegalformulasyntax', 'quiz', $regs[0]); } else { // Formula just might be valid // adding additional test for ? : structure return false; } preliminary results show that this can be done by something like ~([-+/*%>^\~<=&|!0-9eE]+)\?([-+/*%>^\~<=&|!0-9eE]+):([-+/*%>^\~<=&|!0-9eE]+)~ after testing first that there is a ? : in the formula more later
        Hide
        Tim Hunt added a comment -

        I think we should try to support ? : if we can, however, it does make validation harder. All the other operators are a single character. ? : is two separate bits with stuff in the middle.

        Show
        Tim Hunt added a comment - I think we should try to support ? : if we can, however, it does make validation harder. All the other operators are a single character. ? : is two separate bits with stuff in the middle.
        Hide
        Pierre Pichet added a comment - - edited

        Ok to continue my coding on ? : and effectively the complex pregmatch needs to be applied "after testing first that there is a ? .... : in the formula"
        testing first if there are a ? then ? .. : then ...etc.

        Show
        Pierre Pichet added a comment - - edited Ok to continue my coding on ? : and effectively the complex pregmatch needs to be applied "after testing first that there is a ? .... : in the formula" testing first if there are a ? then ? .. : then ...etc.
        Hide
        Pierre Pichet added a comment - - edited

        The new code has to test if the ?...: combination is always valid.
        However the test cannot be done at the end of the actual formula analysis process which eliminates all function names or ()so

         -(sin(6.5)?1:3)?2:4 
        

        becomes

        -1?2:4

        I need to catch things like the supplementary ":" in

        -(sin(6.5)?1:3:)?2:4
        

        It is becoming an almost complete php compiler code with a limited function list...

        So, how could I use try and catch(exception) when using the eval('$str = '.$formula.';'); so that I let php find the complex errors.

        I know that this could interfere to the php error settings ?????????

        Show
        Pierre Pichet added a comment - - edited The new code has to test if the ?...: combination is always valid. However the test cannot be done at the end of the actual formula analysis process which eliminates all function names or ()so -(sin(6.5)?1:3)?2:4 becomes -1?2:4 I need to catch things like the supplementary ":" in -(sin(6.5)?1:3:)?2:4 It is becoming an almost complete php compiler code with a limited function list... So, how could I use try and catch(exception) when using the eval('$str = '.$formula.';'); so that I let php find the complex errors. I know that this could interfere to the php error settings ?????????
        Hide
        Pierre Pichet added a comment - - edited

        I understand that the test for the ?...: combination need to be done inside the main loop.
        But it is becoming quite complex and my first reflex is to od a ggogle search on php decoder

        Show
        Pierre Pichet added a comment - - edited I understand that the test for the ?...: combination need to be done inside the main loop. But it is becoming quite complex and my first reflex is to od a ggogle search on php decoder
        Hide
        Pierre Pichet added a comment - - edited

        The actual code to check the formula does not test for ?...:... syntax validity.
        The analysis done is related to () and their use in functions power(param1,param2) and between them as 2*(functxx(par,par)+ 3... ).
        The parenthesis ( except for the first one) need a preceeding parameter like * i.e (5-4*(3-1)).

        The requirements for the value tested ... ? ... :... are quite different and need a much more complex code to take care of all options.

        If I try the

        {a}

        +

        {b}

        ? wrong formula on Moodle 1.9 UQAM, there is no php error notice.
        The only thing is that the result value from the formula is always 0 so that the user needs to go back and correct the formula.

        In 2,0 any error of the kind give a complete error message and push the user to add a new bug to the tracker.

        This ? possible bug has not, in the last years, generate a lot of bugs probably because few users use this feature or they just corrected their formula to have a non 0 value.

        I can manage to test is the (...)?(...)...) sequence written by the author seems to respect general conditions ( not empty, closing () ).

        If we can use the a try eval() ...catch code to do the final check, this could solve this bug in a reasonable way.

        I have never build a compiler although I explored the first c compiler for the Amiga in 1985 so I cannot go further.

        This is a minor bug, what is the easiest way to solve it ?

        Show
        Pierre Pichet added a comment - - edited The actual code to check the formula does not test for ?...:... syntax validity. The analysis done is related to () and their use in functions power(param1,param2) and between them as 2*(functxx(par,par)+ 3... ). The parenthesis ( except for the first one) need a preceeding parameter like * i.e (5-4*(3-1)). The requirements for the value tested ... ? ... :... are quite different and need a much more complex code to take care of all options. If I try the {a} + {b} ? wrong formula on Moodle 1.9 UQAM, there is no php error notice. The only thing is that the result value from the formula is always 0 so that the user needs to go back and correct the formula. In 2,0 any error of the kind give a complete error message and push the user to add a new bug to the tracker. This ? possible bug has not, in the last years, generate a lot of bugs probably because few users use this feature or they just corrected their formula to have a non 0 value. I can manage to test is the (...)?(...) ...) sequence written by the author seems to respect general conditions ( not empty, closing () ). If we can use the a try eval() ...catch code to do the final check, this could solve this bug in a reasonable way. I have never build a compiler although I explored the first c compiler for the Amiga in 1985 so I cannot go further. This is a minor bug, what is the easiest way to solve it ?
        Hide
        Pierre Pichet added a comment - - edited

        In production, the PHP debug are set to no output.
        This is probably why there has been no bug report.
        The functions allowed could not give problems (hacking) and as they are execute in a eval if something goes wrong (syntax error) the return value is 0 which the user will interpret as "something is wrong in my formula.
        This 0 value will be visible when the user set the dataitems value in the 3rd page.

        Furthermore I found on http://ca3.php.net/manual/en/function.eval.php that if I use the following code

        if (eval('return true;'.$formula.';')){
          //  echo "<p> good </p>";
            eval('$str = '.$formula.';');
           } else {
               $str = 0 ;
               echo "<p> You have a syntax error in your formula </p>";
           }
        

        the user will receive a significant info.
        The interesting aspect of this test is that the $formula is not executed just analyzed for error.

        This is the easisest way to "solve" this bug.
        Furthermore, I can add a validation step in edit_calculated_form (calculated simple and calculated multi) so that the error is corrected at the start.

        I can also add some simple checks in function qtype_calculated_find_formula_errors($formula) so that at least the ? ...: combinations are at least checked.

        I am not sure also that the actual function qtype_calculated_find_formula_errors($formula) pinpoint all errors so in any case adding the validation is a good thing.

        Show
        Pierre Pichet added a comment - - edited In production, the PHP debug are set to no output. This is probably why there has been no bug report. The functions allowed could not give problems (hacking) and as they are execute in a eval if something goes wrong (syntax error) the return value is 0 which the user will interpret as "something is wrong in my formula. This 0 value will be visible when the user set the dataitems value in the 3rd page. Furthermore I found on http://ca3.php.net/manual/en/function.eval.php that if I use the following code if (eval('return true;'.$formula.';')){ // echo "<p> good </p>"; eval('$str = '.$formula.';'); } else { $str = 0 ; echo "<p> You have a syntax error in your formula </p>"; } the user will receive a significant info. The interesting aspect of this test is that the $formula is not executed just analyzed for error. This is the easisest way to "solve" this bug. Furthermore, I can add a validation step in edit_calculated_form (calculated simple and calculated multi) so that the error is corrected at the start. I can also add some simple checks in function qtype_calculated_find_formula_errors($formula) so that at least the ? ...: combinations are at least checked. I am not sure also that the actual function qtype_calculated_find_formula_errors($formula) pinpoint all errors so in any case adding the validation is a good thing.
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this issue.

        We have detected that this issue has been inactive for over a year. It was reported as affecting versions that are no longer supported.

        If you believe that this issue is still relevant to current versions (2.5 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

        Michael d.

        TW9vZGxlDQo=

        Show
        Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year. It was reported as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.5 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d. TW9vZGxlDQo=
        Hide
        Pierre Pichet added a comment -

        Not solved already but should remain active

        Show
        Pierre Pichet added a comment - Not solved already but should remain active

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated: