Merge pull request #5129 from edx/ned/python-zip-assets

Python zip file importable from an asset

cms/djangoapps/contentstore/views/preview.py

@@ -11,6 +11,7 @@ from edxmako.shortcuts import render_to_string
 
 from xmodule_modifiers import replace_static_urls, wrap_xblock, wrap_fragment, request_token
 from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW
+from xmodule.contentstore.django import contentstore
 from xmodule.error_module import ErrorDescriptor
 from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.modulestore.django import modulestore, ModuleI18nService
@@ -25,7 +26,7 @@ from lms.lib.xblock.field_data import LmsFieldData
 from cms.lib.xblock.field_data import CmsFieldData
 from cms.lib.xblock.runtime import local_resource_url
 
-from util.sandboxing import can_execute_unsafe_code
+from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip
 
 import static_replace
 from .session_kv_store import SessionKeyValueStore
@@ -150,6 +151,7 @@ def _preview_module_system(request, descriptor):
         replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id),
         user=request.user,
         can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)),
+        get_python_lib_zip=(lambda :get_python_lib_zip(contentstore, course_id)),
         mixins=settings.XBLOCK_MIXINS,
         course_id=course_id,
         anonymous_student_id='student',

common/djangoapps/util/sandboxing.py

 import re
 from django.conf import settings
 
+# We'll make assets named this be importable by Python code in the sandbox.
+PYTHON_LIB_ZIP = "python_lib.zip"
+
 
 def can_execute_unsafe_code(course_id):
     """
@@ -25,3 +28,13 @@ def can_execute_unsafe_code(course_id):
         if re.match(regex, course_id.to_deprecated_string()):
             return True
     return False
+
+
+def get_python_lib_zip(contentstore, course_id):
+    """Return the bytes of the python_lib.zip file, if any."""
+    asset_key = course_id.make_asset_key("asset", PYTHON_LIB_ZIP)
+    zip_lib = contentstore().find(asset_key, throw_on_not_found=False)
+    if zip_lib is not None:
+        return zip_lib.data
+    else:
+        return None

common/lib/capa/capa/capa_problem.py

@@ -13,14 +13,15 @@ Main module which shows problems (of "capa" type).
 This is used by capa_module.
 """
 
+from copy import deepcopy
 from datetime import datetime
 import logging
 import os.path
 import re
 
 from lxml import etree
+from pytz import UTC
 from xml.sax.saxutils import unescape
-from copy import deepcopy
 
 from capa.correctmap import CorrectMap
 import capa.inputtypes as inputtypes
@@ -28,10 +29,8 @@ import capa.customrender as customrender
 import capa.responsetypes as responsetypes
 from capa.util import contextualize_text, convert_files_to_filenames
 import capa.xqueue_interface as xqueue_interface
-
 from capa.safe_exec import safe_exec
 
-from pytz import UTC
 
 # extra things displayed after "show answers" is pressed
 solution_tags = ['solution']
@@ -84,6 +83,7 @@ class LoncapaSystem(object):
         anonymous_student_id,
         cache,
         can_execute_unsafe_code,
+        get_python_lib_zip,
         DEBUG,                                          # pylint: disable=invalid-name
         filestore,
         i18n,
@@ -98,6 +98,7 @@ class LoncapaSystem(object):
         self.anonymous_student_id = anonymous_student_id
         self.cache = cache
         self.can_execute_unsafe_code = can_execute_unsafe_code
+        self.get_python_lib_zip = get_python_lib_zip
         self.DEBUG = DEBUG                              # pylint: disable=invalid-name
         self.filestore = filestore
         self.i18n = i18n
@@ -645,6 +646,13 @@ class LoncapaProblem(object):
             code = unescape(script.text, XMLESC)
             all_code += code
 
+        # An asset named python_lib.zip can be imported by Python code.
+        extra_files = []
+        zip_lib = self.capa_system.get_python_lib_zip()
+        if zip_lib is not None:
+            extra_files.append(("python_lib.zip", zip_lib))
+            python_path.append("python_lib.zip")
+
         if all_code:
             try:
                 safe_exec(
@@ -652,6 +660,7 @@ class LoncapaProblem(object):
                     context,
                     random_seed=self.seed,
                     python_path=python_path,
+                    extra_files=extra_files,
                     cache=self.capa_system.cache,
                     slug=self.problem_id,
                     unsafely=self.capa_system.can_execute_unsafe_code(),
@@ -664,6 +673,7 @@ class LoncapaProblem(object):
         # Store code source in context, along with the Python path needed to run it correctly.
         context['script_code'] = all_code
         context['python_path'] = python_path
+        context['extra_files'] = extra_files or None
         return context
 
     def _extract_html(self, problemtree):  # private

common/lib/capa/capa/responsetypes.py

@@ -305,6 +305,7 @@ class LoncapaResponse(object):
                     code,
                     globals_dict,
                     python_path=self.context['python_path'],
+                    extra_files=self.context['extra_files'],
                     slug=self.id,
                     random_seed=self.context['seed'],
                     unsafely=self.capa_system.can_execute_unsafe_code(),
@@ -1480,6 +1481,7 @@ class CustomResponse(LoncapaResponse):
                             code,
                             globals_dict,
                             python_path=self.context['python_path'],
+                            extra_files=self.context['extra_files'],
                             slug=self.id,
                             random_seed=self.context['seed'],
                             unsafely=self.capa_system.can_execute_unsafe_code(),
@@ -1613,6 +1615,8 @@ class CustomResponse(LoncapaResponse):
                     self.code,
                     self.context,
                     cache=self.capa_system.cache,
+                    python_path=self.context['python_path'],
+                    extra_files=self.context['extra_files'],
                     slug=self.id,
                     random_seed=self.context['seed'],
                     unsafely=self.capa_system.can_execute_unsafe_code(),
@@ -2496,6 +2500,8 @@ class SchematicResponse(LoncapaResponse):
                 self.code,
                 self.context,
                 cache=self.capa_system.cache,
+                python_path=self.context['python_path'],
+                extra_files=self.context['extra_files'],
                 slug=self.id,
                 random_seed=self.context['seed'],
                 unsafely=self.capa_system.can_execute_unsafe_code(),

common/lib/capa/capa/safe_exec/safe_exec.py

@@ -71,7 +71,16 @@ def update_hash(hasher, obj):
 
 
 @dog_stats_api.timed('capa.safe_exec.time')
-def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None, slug=None, unsafely=False):
+def safe_exec(
+    code,
+    globals_dict,
+    random_seed=None,
+    python_path=None,
+    extra_files=None,
+    cache=None,
+    slug=None,
+    unsafely=False,
+):
     """
     Execute python code safely.
 
@@ -81,7 +90,12 @@ def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None
 
     `random_seed` will be used to see the `random` module available to the code.
 
-    `python_path` is a list of directories to add to the Python path before execution.
+    `python_path` is a list of filenames or directories to add to the Python
+    path before execution.  If the name is not in `extra_files`, then it will
+    also be copied into the sandbox.
+
+    `extra_files` is a list of (filename, contents) pairs.  These files are
+    created in the sandbox.
 
     `cache` is an object with .get(key) and .set(key, value) methods.  It will be used
     to cache the execution, taking into account the code, the values of the globals,
@@ -123,7 +137,7 @@ def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None
     try:
         exec_fn(
             code_prolog + LAZY_IMPORTS + code, globals_dict,
-            python_path=python_path, slug=slug,
+            python_path=python_path, extra_files=extra_files, slug=slug,
         )
     except SafeExecException as e:
         emsg = e.message

common/lib/capa/capa/tests/__init__.py

@@ -41,6 +41,7 @@ def test_capa_system():
         anonymous_student_id='student',
         cache=None,
         can_execute_unsafe_code=lambda: False,
+        get_python_lib_zip=lambda: None,
         DEBUG=True,
         filestore=fs.osfs.OSFS(os.path.join(TEST_DIR, "test_files")),
         i18n=gettext.NullTranslations(),

common/lib/capa/capa/tests/test_responsetypes.py

@@ -3,15 +3,19 @@
 Tests of responsetypes
 """
 
+from cStringIO import StringIO
 from datetime import datetime
 import json
 import os
 import pyparsing
 import random
-import unittest
 import textwrap
-import requests
+import unittest
+import zipfile
+
 import mock
+from pytz import UTC
+import requests
 
 from . import new_loncapa_problem, test_capa_system, load_fixture
 import calc
@@ -23,8 +27,6 @@ from capa.util import convert_files_to_filenames
 from capa.util import compare_with_tolerance
 from capa.xqueue_interface import dateformat
 
-from pytz import UTC
-
 
 class ResponseTest(unittest.TestCase):
     """Base class for tests of capa responses."""
@@ -1712,6 +1714,28 @@ class CustomResponseTest(ResponseTest):
             except ResponseError:
                 self.fail("Could not use name '{0}s' in custom response".format(module_name))
 
+    def test_python_lib_zip_is_available(self):
+        # Prove that we can import code from a zipfile passed down to us.
+
+        # Make a zipfile with one module in it with one function.
+        zipstring = StringIO()
+        zipf = zipfile.ZipFile(zipstring, "w")
+        zipf.writestr("my_helper.py", textwrap.dedent("""\
+            def seventeen():
+                return 17
+            """))
+        zipf.close()
+
+        # Use that module in our Python script.
+        script = textwrap.dedent("""
+            import my_helper
+            num = my_helper.seventeen()
+            """)
+        capa_system = test_capa_system()
+        capa_system.get_python_lib_zip = lambda: zipstring.getvalue()
+        problem = self.build_problem(script=script, capa_system=capa_system)
+        self.assertEqual(problem.context['num'], 17)
+
 
 class SchematicResponseTest(ResponseTest):
     from capa.tests.response_xml_factory import SchematicResponseXMLFactory

common/lib/xmodule/xmodule/capa_base.py

@@ -298,6 +298,7 @@ class CapaMixin(CapaFields):
             anonymous_student_id=self.runtime.anonymous_student_id,
             cache=self.runtime.cache,
             can_execute_unsafe_code=self.runtime.can_execute_unsafe_code,
+            get_python_lib_zip=self.runtime.get_python_lib_zip,
             DEBUG=self.runtime.DEBUG,
             filestore=self.runtime.filestore,
             i18n=self.runtime.service(self, "i18n"),

common/lib/xmodule/xmodule/x_module.py

@@ -1244,7 +1244,7 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # pylin
             cache=None, can_execute_unsafe_code=None, replace_course_urls=None,
             replace_jump_to_id_urls=None, error_descriptor_class=None, get_real_user=None,
             field_data=None, get_user_role=None, rebind_noauth_module_to_user=None,
-            user_location=None, **kwargs):
+            user_location=None, get_python_lib_zip=None, **kwargs):
         """
         Create a closure around the system environment.
 
@@ -1293,6 +1293,10 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # pylin
         can_execute_unsafe_code - A function returning a boolean, whether or
             not to allow the execution of unsafe, unsandboxed code.
 
+        get_python_lib_zip - A function returning a bytestring or None.  The
+            bytestring is the contents of a zip file that should be importable
+            by other Python code running in the module.
+
         error_descriptor_class - The class to use to render XModules with errors
 
         get_real_user - function that takes `anonymous_student_id` and returns real user_id,
@@ -1334,6 +1338,7 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # pylin
 
         self.cache = cache or DoNothingCache()
         self.can_execute_unsafe_code = can_execute_unsafe_code or (lambda: False)
+        self.get_python_lib_zip = get_python_lib_zip or (lambda: None)
         self.replace_course_urls = replace_course_urls
         self.replace_jump_to_id_urls = replace_jump_to_id_urls
         self.error_descriptor_class = error_descriptor_class

common/test/acceptance/pages/lms/problem.py

@@ -20,3 +20,31 @@ class ProblemPage(PageObject):
         Return the current problem name.
         """
         return self.q(css='.problem-header').text[0]
+
+    @property
+    def problem_text(self):
+        """
+        Return the text of the question of the problem.
+        """
+        return self.q(css="div.problem p").text
+
+    def fill_answer(self, text):
+        """
+        Fill in the answer to the problem.
+        """
+        self.q(css='div.problem div.capa_inputtype.textline input').fill(text)
+
+    def click_check(self):
+        """
+        Click the Check button!
+        """
+        self.q(css='div.problem input.check').click()
+        self.wait_for_ajax()
+
+    def is_correct(self):
+        """
+        Is there a "correct" status showing?
+        """
+        return self.q(css="div.problem div.capa_inputtype.textline div.correct p.status").is_present()
+
+

common/test/acceptance/tests/test_lms.py

@@ -3,6 +3,7 @@
 E2E tests for the LMS.
 """
 
+from textwrap import dedent
 from unittest import skip
 
 from .helpers import UniqueCourseTest, load_data_str
@@ -14,6 +15,7 @@ from ..pages.lms.tab_nav import TabNavPage
 from ..pages.lms.course_nav import CourseNavPage
 from ..pages.lms.progress import ProgressPage
 from ..pages.lms.dashboard import DashboardPage
+from ..pages.lms.problem import ProblemPage
 from ..pages.lms.video.video import VideoPage
 from ..pages.xblock.acid import AcidView
 from ..pages.lms.courseware import CoursewarePage
@@ -543,3 +545,80 @@ class TooltipTest(UniqueCourseTest):
         self.tab_nav.go_to_tab('Courseware')
 
         self.assertTrue(self.courseware_page.tooltips_displayed())
+
+
+class ProblemExecutionTest(UniqueCourseTest):
+    """
+    Tests of problems.
+    """
+
+    def setUp(self):
+        """
+        Initialize pages and install a course fixture.
+        """
+        super(ProblemExecutionTest, self).setUp()
+
+        self.course_info_page = CourseInfoPage(self.browser, self.course_id)
+        self.course_nav = CourseNavPage(self.browser)
+        self.tab_nav = TabNavPage(self.browser)
+
+        # Install a course with sections and problems.
+        course_fix = CourseFixture(
+            self.course_info['org'], self.course_info['number'],
+            self.course_info['run'], self.course_info['display_name']
+        )
+
+        course_fix.add_asset(['python_lib.zip'])
+
+        course_fix.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
+                    XBlockFixtureDesc('problem', 'Python Problem', data=dedent("""\
+                        <problem>
+                        <script type="loncapa/python">
+                        from number_helpers import seventeen, fortytwo
+                        oneseven = seventeen()
+
+                        def check_function(expect, ans):
+                            if int(ans) == fortytwo(-22):
+                                return True
+                            else:
+                                return False
+                        </script>
+
+                        <p>What is the sum of $oneseven and 3?</p>
+
+                        <customresponse expect="20" cfn="check_function">
+                            <textline/>
+                        </customresponse>
+                        </problem>
+                        """
+                    )),
+                )
+            )
+        ).install()
+
+        # Auto-auth register for the course
+        AutoAuthPage(self.browser, course_id=self.course_id).visit()
+
+    def test_python_execution_in_problem(self):
+        # Navigate to the problem page
+        self.course_info_page.visit()
+        self.tab_nav.go_to_tab('Courseware')
+        self.course_nav.go_to_section('Test Section', 'Test Subsection')
+
+        problem_page = ProblemPage(self.browser)
+        self.assertEqual(problem_page.problem_name, 'PYTHON PROBLEM')
+
+        # Does the page have computation results?
+        self.assertIn("What is the sum of 17 and 3?", problem_page.problem_text)
+
+        # Fill in the answer correctly.
+        problem_page.fill_answer("20")
+        problem_page.click_check()
+        self.assertTrue(problem_page.is_correct())
+
+        # Fill in the answer incorrectly.
+        problem_page.fill_answer("4")
+        problem_page.click_check()
+        self.assertFalse(problem_page.is_correct())

common/test/data/uploads/python_lib_zip/README.txt

+Use this directory to make the python_lib.zip file above:
+
+$ zip -r python_lib.zip python_lib_zip/*

common/test/data/uploads/python_lib_zip/number_helpers.py

+def seventeen():
+    return 17
+
+def fortytwo(x):
+    return 42+x

common/test/data/uploads/python_lib_zip/sub/submodule.py

+HELLO = "world"

lms/djangoapps/courseware/module_render.py

@@ -35,6 +35,7 @@ from xblock.django.request import django_to_webob_request, webob_to_django_respo
 from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
 from xmodule.exceptions import NotFoundError, ProcessingError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from xmodule.contentstore.django import contentstore
 from xmodule.modulestore.django import modulestore, ModuleI18nService
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.util.duedate import get_extended_due_date
@@ -50,7 +51,7 @@ from xmodule.lti_module import LTIModule
 from xmodule.x_module import XModuleDescriptor
 
 from util.json_request import JsonResponse
-from util.sandboxing import can_execute_unsafe_code
+from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip
 
 
 log = logging.getLogger(__name__)
@@ -530,6 +531,7 @@ def get_module_system_for_user(user, field_data_cache,
         s3_interface=s3_interface,
         cache=cache,
         can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)),
+        get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, course_id)),
         # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington)
         mixins=descriptor.runtime.mixologist._mixins,  # pylint: disable=protected-access
         wrappers=block_wrappers,

requirements/edx/github.txt

@@ -20,7 +20,7 @@
 
 # Our libraries:
 -e git+https://github.com/edx/XBlock.git@81a6d713c98d4914af96a0ca624ee7fa4903625e#egg=XBlock
--e git+https://github.com/edx/codejail.git@71f5c5616e2a73ae8cecd1ff2362774a773d3665#egg=codejail
+-e git+https://github.com/edx/codejail.git@66dd5a45e5072666ff9a70c768576e9ffd1daa4b#egg=codejail
 -e git+https://github.com/edx/diff-cover.git@v0.5.0#egg=diff_cover
 -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool
 -e git+https://github.com/edx/event-tracking.git@0.1.0#egg=event-tracking