Uploaded image for project: '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
    • Status: Open
    • Priority: 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

      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

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            timhunt 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
            timhunt 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            ppichet 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
            salvetore 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
            salvetore 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
            ppichet Pierre Pichet added a comment -

            Not solved already but should remain active

            Show
            ppichet 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: