Merge pull request #563 from MITx/kimth/check-grader-html

Check grader message has proper XML structure

common/lib/capa/capa/responsetypes.py

@@ -1187,7 +1187,7 @@ class CodeResponse(LoncapaResponse):
 
         (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) 
         if not valid_score_msg:
-            oldcmap.set(self.answer_id, msg='Error: Invalid grader reply.')
+            oldcmap.set(self.answer_id, msg='Invalid grader reply. Please contact the course staff.')
             return oldcmap
         
         correctness = 'correct' if correct else 'incorrect'
@@ -1225,20 +1225,35 @@ class CodeResponse(LoncapaResponse):
         Returns (valid_score_msg, correct, score, msg):
             valid_score_msg: Flag indicating valid score_msg format (Boolean)
             correct:         Correctness of submission (Boolean)
-            score:           # TODO: Implement partial grading
+            score:           Points to be assigned (numeric, can be float)
             msg:             Message from grader to display to student (string)
         '''
-        fail = (False, False, -1, '')
+        fail = (False, False, 0, '')
         try:
             score_result = json.loads(score_msg)
         except (TypeError, ValueError):
+            log.error("External grader message should be a JSON-serialized dict. Received score_msg = %s" % score_msg)
             return fail
         if not isinstance(score_result, dict):
+            log.error("External grader message should be a JSON-serialized dict. Received score_result = %s" % score_result)
             return fail
         for tag in ['correct', 'score', 'msg']:
-            if not score_result.has_key(tag):
+            if tag not in score_result:
+                log.error("External grader message is missing one or more required tags: 'correct', 'score', 'msg'")
                 return fail
-        return (True, score_result['correct'], score_result['score'], score_result['msg'])
+
+        # Next, we need to check that the contents of the external grader message 
+        #   is safe for the LMS.
+        # 1) Make sure that the message is valid XML (proper opening/closing tags)
+        # 2) TODO: Is the message actually HTML?
+        msg = score_result['msg']
+        try:
+            etree.fromstring(msg)
+        except etree.XMLSyntaxError as err:
+            log.error("Unable to parse external grader message as valid XML: score_msg['msg']=%s" % msg)
+            return fail
+            
+        return (True, score_result['correct'], score_result['score'], msg)
         
 
 #-----------------------------------------------------------------------------

common/lib/xmodule/xmodule/tests/__init__.py

@@ -285,6 +285,7 @@ class StringResponseWithHintTest(unittest.TestCase):
 class CodeResponseTest(unittest.TestCase):
     '''
     Test CodeResponse
+    TODO: Add tests for external grader messages
     '''
     @staticmethod
     def make_queuestate(key, time):
@@ -318,6 +319,7 @@ class CodeResponseTest(unittest.TestCase):
 
             self.assertEquals(test_lcp.is_queued(), True)
 
+
     def test_update_score(self):
         '''
         Test whether LoncapaProblem.update_score can deliver queued result to the right subproblem
@@ -336,8 +338,9 @@ class CodeResponseTest(unittest.TestCase):
                 old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuestate=queuestate))
 
             # Message format common to external graders
-            correct_score_msg = json.dumps({'correct':True, 'score':1, 'msg':'MESSAGE'})
-            incorrect_score_msg = json.dumps({'correct':False, 'score':0, 'msg':'MESSAGE'})
+            grader_msg = '<span>MESSAGE</span>' # Must be valid XML
+            correct_score_msg = json.dumps({'correct':True, 'score':1, 'msg': grader_msg})
+            incorrect_score_msg = json.dumps({'correct':False, 'score':0, 'msg': grader_msg})
 
             xserver_msgs = {'correct': correct_score_msg,
                             'incorrect': incorrect_score_msg,}
@@ -362,7 +365,7 @@ class CodeResponseTest(unittest.TestCase):
                     new_cmap = CorrectMap()
                     new_cmap.update(old_cmap)
                     npoints = 1 if correctness=='correct' else 0
-                    new_cmap.set(answer_id=answer_id, npoints=npoints, correctness=correctness, msg='MESSAGE', queuestate=None)
+                    new_cmap.set(answer_id=answer_id, npoints=npoints, correctness=correctness, msg=grader_msg, queuestate=None)
 
                     test_lcp.update_score(xserver_msgs[correctness], queuekey=1000 + i)
                     self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict())