maths / moodle-qtype_stack

Stack question type for Moodle
GNU General Public License v3.0
138 stars 147 forks source link

Fatal errors if a student types ² and Insert stars assuming single-character variable names is used #1107

Closed timhunt closed 4 months ago

timhunt commented 5 months ago
  1. Create a STACK question like this (or import this one example-question.xml.txt.):
    • Name: Test
    • Question text: Input (x) squared. [[input:ans1]] [[validation:ans1]]
    • Input: Ans 1 -> Tans: x^2
    • Input: Ans 1 -> Insert stars: Insert stars assuming single-character variable names
    • PRT1 -> Node 1: Alg equiv test between ans1 and x^2
  2. Save the question, and preview it.
  3. Type in: x²

Expected result: A nice validation message like "This answer is invalid. CAS commands may not contain the following characters: ²." which is what happens without the insert stars option.

Actual result: ... this does seem to depend slightly on the Maxima back-end, but some sort of fatal error.

With goemaxima that is: stack_cas_session: tried to parse the value x*\², but got the following exception Expected "#pm#", "%not ", "'", "''", "(", "+", "+-", "-", "? ", "?", "?? ", "[", "do", "for", "from", "if", "in", "next", "not ", "not", "nounnot ", "nounnot", "step", "thru", "unless", "while", "{", "|", boolean, float, identifier, integer, string or whitespace but "\" found.

With MaximaPool is (incorrectly) "CAS failed to return any data due to timeout." - that happens instantly.

Worse, if you add this question to a quiz, start an attempt, input x². Not only do you get the error in STACK's validation, but this also causes fatal errors when Moodle's quiz autosave tries to save the response after a minute or so. leading to the warning "Network connection lost. Anything entered on this page in the last minute might not be saved. ..." being displayed.

This unit test might show the problem.

    public function test_validate_student_response_single_var_chars_unicode_superscript() {
        $options = new stack_options();
        $el = stack_input_factory::make('algebraic', 'sans1', '');
        $el->set_parameter('insertStars', 2);
        $state = $el->validate_student_response(['sans1' => 'x²'], $options, 'x^2',
                new stack_cas_security());
        $this->assertEquals(stack_input::INVALID, $state->status);
        // The rest needs to be updated once we know what the expected result is.
        $this->assertEquals('missing_stars', $state->note);
        $this->assertEquals('', $state->errors);
        $this->assertEquals('(3*x+1)*(x+a*b)', $state->contentsmodified);
        $this->assertEquals('\[ \left(3\cdot x+1\right)\cdot \left(x+a\cdot b\right) \]', $state->contentsdisplayed);
        $this->assertEquals('\( \left[ a , b , x \right]\) ', $state->lvars);
    }

Here is the CAS session that is sent:

block([stack_seed],
  stack_randseed(0),
  stack_seed:0,
  texput_decimal("."),
  make_multsgn("dot"),
  make_complexJ("i"),
  make_arccos("cos-1"),
  make_logic("lang"),
  sqrtdispflag:true,
  simp:false,
  assume_pos:false,
  assume_real:false,
  lmxchar:"[",
  _RESPONSE:["stack_map"],
  _VALUES:["stack_map"],
  _LATEX:["stack_map"],
  _DVALUES:["stack_map"],
  _CS2v("__stackmaximaversion",stackmaximaversion),
  %stmt:"s0",
  _EC(errcatch(__s0:sans1:stack_validate([x*²], true,x^2,"~a",0)),""),
  _CS2l("__s0",__s0),
  %stmt:"s1",
  _EC(errcatch(__s1:sans1:stack_validate([x*²], true,x^2,"~a",0)),""),
  _CS2l("__s1",__s1),
  %stmt:"s2",
  _EC(errcatch(__s2:x*²),""),
  _CS2l("__s2",__s2),
  %stmt:"s3",
  _EC(errcatch(__s3:(%_C(stack_validate_listofvars),stack_validate_listofvars(sans1))),""),
  _CS2l("__s3",__s3),
  _CS2dvv("__s0",__s0),
  _CS2dvv("__s1",__s1),
  _CS2dvv("__s2",__s2),
  _CS2dvv("__s3",__s3),
  _RESPONSE:stackmap_set(_RESPONSE,"presentation",_LATEX),
  _RESPONSE:stackmap_set(_RESPONSE,"display",_DVALUES),
  _CS2out()
)$

Which causes:

Maxima restarted.
(%i2) 
(%o2) "/var/lib/maxima/2023121100/maximalocal.mac"
(%i3) 
syntax error: ² is not an infix operator
 -- an error. To debug this try: debugmode(true);
syntax error: ² is not an infix operator
 -- an error. To debug this try: debugmode(true);
syntax error: ² is not an infix operator
 -- an error. To debug this try: debugmode(true);
syntax error: Too many )'s
 -- an error. To debug this try: debugmode(true);
sangwinc commented 5 months ago

Sorry Tim, I think I've caused the problem here: https://github.com/maths/moodle-qtype_stack/blame/master/stack/maximaparser/corrective_parser.php#L112

timhunt commented 5 months ago

I don't think it si a problem character is banned. We want students to type x^2. The problem is that Insert stars assuming single-character variable names seems to skip the inital check, and then blows up badly later, rather than being handled well.

But, I have to say I think this bug is beyond me to fix. I am hoping your or Matti will see what is going on.

sangwinc commented 4 months ago

I've committed a fix here: https://github.com/maths/moodle-qtype_stack/tree/iss1107 This does "forbid" the use of superscript unicode. The difficulty is that, while x^2 is unambiguous, we have to parse the supercript characters, e.g. x^22. So, forbidding these at validation time is safer than partially accepting them. If someone is motivated to parse these situations, then please send in a pull request! In the mean time, this fix prevents the catastrophic crash at lease which is a marginal improvement.

sangwinc commented 4 months ago

I've updated the validation mechanism which should prevent this getting as far as Maxima. On reflection I think this is an overall improvement on things.