diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 0131783ce8..a16fad00a9 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/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', diff --git a/common/djangoapps/util/sandboxing.py b/common/djangoapps/util/sandboxing.py index e3ec2eef23..5fea7ed25c 100644 --- a/common/djangoapps/util/sandboxing.py +++ b/common/djangoapps/util/sandboxing.py @@ -1,6 +1,9 @@ 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 diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index e4997db1c6..f3eb115ad6 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/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 diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index fb7fa62f2b..6f8e7596ad 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/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(), diff --git a/common/lib/capa/capa/safe_exec/safe_exec.py b/common/lib/capa/capa/safe_exec/safe_exec.py index 01551ffcb8..b25f7b47a2 100644 --- a/common/lib/capa/capa/safe_exec/safe_exec.py +++ b/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 diff --git a/common/lib/capa/capa/tests/__init__.py b/common/lib/capa/capa/tests/__init__.py index bbcef25bb4..d91a037bfc 100644 --- a/common/lib/capa/capa/tests/__init__.py +++ b/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(), diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 86a6401408..b6f15eac90 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index e68b243449..4a03e99d96 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/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"), diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index fd560814ea..92776eabd8 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/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 diff --git a/common/test/acceptance/pages/lms/problem.py b/common/test/acceptance/pages/lms/problem.py index 7e39b3d696..dbe6364d71 100644 --- a/common/test/acceptance/pages/lms/problem.py +++ b/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() + + diff --git a/common/test/acceptance/tests/test_lms.py b/common/test/acceptance/tests/test_lms.py index 97ab8eb54f..2cf870a798 100644 --- a/common/test/acceptance/tests/test_lms.py +++ b/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("""\ + + + +

What is the sum of $oneseven and 3?

+ + + + + + """ + )), + ) + ) + ).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()) diff --git a/common/test/data/uploads/python_lib.zip b/common/test/data/uploads/python_lib.zip new file mode 100644 index 0000000000..a41d94177f Binary files /dev/null and b/common/test/data/uploads/python_lib.zip differ diff --git a/common/test/data/uploads/python_lib_zip/README.txt b/common/test/data/uploads/python_lib_zip/README.txt new file mode 100644 index 0000000000..6f14f21719 --- /dev/null +++ b/common/test/data/uploads/python_lib_zip/README.txt @@ -0,0 +1,3 @@ +Use this directory to make the python_lib.zip file above: + +$ zip -r python_lib.zip python_lib_zip/* diff --git a/common/test/data/uploads/python_lib_zip/number_helpers.py b/common/test/data/uploads/python_lib_zip/number_helpers.py new file mode 100644 index 0000000000..a79b13b529 --- /dev/null +++ b/common/test/data/uploads/python_lib_zip/number_helpers.py @@ -0,0 +1,5 @@ +def seventeen(): + return 17 + +def fortytwo(x): + return 42+x diff --git a/common/test/data/uploads/python_lib_zip/sub/__init__.py b/common/test/data/uploads/python_lib_zip/sub/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/test/data/uploads/python_lib_zip/sub/submodule.py b/common/test/data/uploads/python_lib_zip/sub/submodule.py new file mode 100644 index 0000000000..1175840777 --- /dev/null +++ b/common/test/data/uploads/python_lib_zip/sub/submodule.py @@ -0,0 +1 @@ +HELLO = "world" diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index b8accd95a8..de45e8beed 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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, diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 28f4da6232..908ca69c3c 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -19,7 +19,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