hit-moodle / onlinejudge

Online judge plugin for Moodle 1.9.x
https://github.com/hit-moodle/onlinejudge/wiki
GNU General Public License v3.0
27 stars 9 forks source link

Diff function mistakes "0" as an end of tokens #2

Closed michaliskambi closed 12 years ago

michaliskambi commented 12 years ago

The diff function in onlinejudge/assignment.class.php tokenizes the $answer by strtok. It iterates over the tokens by "while ($tok)". Unfortunately, this means that the "0" token inside an $answer will be mistaken for the end of the answer, since 0 will be just converted to false by php. Consider this:

echo diff('0 1 2', 'whatever');

Clearly the result should be "wrong answer" ('wa'), but it's "presentation error" ('pe') because the first 0 inside $answer is simply treated like an end of the $answer.

Solution is to compare with FALSE using === (like manual on http://php.net/manual/en/function.strtok.php suggests). Like "while ($tok !== FALSE)".

For the same reason, a similar check later in the same function "if (!$tok || $tok !== $anstok)" should be changed into "if ($tok === FALSE || $tok !== $anstok)". Otherwise, "0" inside the $output will be treated like an end of the output.

We're using here onlinejudge for Moodle 1.9. Quickly looking at moodle-local_onlinejudge for Moodle 2.x, it shares the same bug inside diff() function in https://github.com/hit-moodle/moodle-local_onlinejudge/blob/master/judgelib.php , so probably the same fix should be repeated there.

michaliskambi commented 12 years ago

I'm not sure how to attach a file to a bug report here, so I just paste my patch here. It's simple enough to apply manually anyway :)

Index: assignment.class.php
===================================================================
--- assignment.class.php    (revision 468)
+++ assignment.class.php    (working copy)
@@ -831,14 +831,14 @@
         else {
             $tokens = Array();
             $tok = strtok($answer, " \n\r\t");
-            while ($tok) {
+            while ($tok !== FALSE) {
                 $tokens[] = $tok;
                 $tok = strtok(" \n\r\t");
             }

             $tok = strtok($output, " \n\r\t");
             foreach ($tokens as $anstok) {
-                if (!$tok || $tok !== $anstok)
+                if ($tok === FALSE || $tok !== $anstok)
                     return 'wa';
                 $tok = strtok(" \n\r\t");
             }
sunner commented 12 years ago

Thank you for the detailed analysis and working solution