Merge pull request #3699 from edx/flowerhack/fix-safe-exec

Compare stringified course_ids to regexes for courses allowed to execute...

common/djangoapps/util/sandboxing.py

@@ -16,7 +16,12 @@ def can_execute_unsafe_code(course_id):
     # a list of regexes configured on the server.
     # If this is not defined in the environment variables then default to the most restrictive, which
     # is 'no unsafe courses'
+    # TODO: This should be a database configuration, where we can mark individual courses as being
+    # safe/unsafe. Someone in the future should switch us over to that rather than using regexes
+    # in a settings file
+    # To others using this: the code as-is is brittle and likely to be changed in the future,
+    # as per the TODO, so please consider carefully before adding more values to COURSES_WITH_UNSAFE_CODE
     for regex in getattr(settings, 'COURSES_WITH_UNSAFE_CODE', []):
-        if re.match(regex, course_id):
+        if re.match(regex, course_id.to_deprecated_string()):
             return True
     return False

common/djangoapps/util/tests/test_sandboxing.py

@@ -5,6 +5,7 @@ Tests for sandboxing.py in util app
 from django.test import TestCase
 from util.sandboxing import can_execute_unsafe_code
 from django.test.utils import override_settings
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 class SandboxingTest(TestCase):
@@ -16,19 +17,19 @@ class SandboxingTest(TestCase):
         """
         Test to make sure that a non-match returns false
         """
-        self.assertFalse(can_execute_unsafe_code('edX/notful/empty'))
+        self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'notful', 'empty')))
 
     @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/full/.*'])
     def test_sandbox_inclusion(self):
         """
         Test to make sure that a match works across course runs
         """
-        self.assertTrue(can_execute_unsafe_code('edX/full/2012_Fall'))
-        self.assertTrue(can_execute_unsafe_code('edX/full/2013_Spring'))
+        self.assertTrue(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2012_Fall')))
+        self.assertTrue(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2013_Spring')))
 
     def test_courses_with_unsafe_code_default(self):
         """
         Test that the default setting for COURSES_WITH_UNSAFE_CODE is an empty setting, e.g. we don't use @override_settings in these tests
         """
-        self.assertFalse(can_execute_unsafe_code('edX/full/2012_Fall'))
-        self.assertFalse(can_execute_unsafe_code('edX/full/2013_Spring'))
+        self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2012_Fall')))
+        self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2013_Spring')))