diff --git a/cms/envs/common.py b/cms/envs/common.py
index 11c0842b8d0f934e16eb80529dfd4d572e0f95df..62a1a60b2ac5b625d96fe31e1bdef2f58b7fdc66 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -215,6 +215,9 @@ FEATURES = {
# Show Language selector
'SHOW_LANGUAGE_SELECTOR': False,
+
+ # Set this to False to facilitate cleaning up invalid xml from your modulestore.
+ 'ENABLE_XBLOCK_XML_VALIDATION': True,
}
ENABLE_JASMINE = False
diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py
index ce7b6e8f4f5d326288560331bce446368aa03c70..783687fc8e5918011edf0ca8a1344b8713b62c89 100644
--- a/common/lib/xmodule/xmodule/capa_base.py
+++ b/common/lib/xmodule/xmodule/capa_base.py
@@ -6,28 +6,27 @@ import hashlib
import json
import logging
import os
-import traceback
+import re
import struct
import sys
-import re
+import traceback
+from django.conf import settings
# We don't want to force a dependency on datadog, so make the import conditional
try:
import dogstats_wrapper as dog_stats_api
except ImportError:
dog_stats_api = None
+from pytz import utc
from capa.capa_problem import LoncapaProblem, LoncapaSystem
-from capa.responsetypes import StudentInputError, \
- ResponseError, LoncapaProblemError
+from capa.responsetypes import StudentInputError, ResponseError, LoncapaProblemError
from capa.util import convert_files_to_filenames, get_inner_html_from_xpath
-from .progress import Progress
-from xmodule.exceptions import NotFoundError
-from xblock.fields import Scope, String, Boolean, Dict, Integer, Float
-from .fields import Timedelta, Date
-from django.utils.timezone import UTC
+from xblock.fields import Boolean, Dict, Float, Integer, Scope, String, XMLString
from xmodule.capa_base_constants import RANDOMIZATION, SHOWANSWER
-from django.conf import settings
+from xmodule.exceptions import NotFoundError
+from .fields import Date, Timedelta
+from .progress import Progress
from openedx.core.djangolib.markup import HTML, Text
@@ -42,6 +41,8 @@ NUM_RANDOMIZATION_BINS = 20
# Never produce more than this many different seeds, no matter what.
MAX_RANDOMIZATION_BINS = 1000
+FEATURES = getattr(settings, 'FEATURES', {})
+
def randomization_bin(seed, problem_id):
"""
@@ -76,7 +77,7 @@ class ComplexEncoder(json.JSONEncoder):
"""
Extend the JSON encoder to correctly handle complex numbers
"""
- def default(self, obj):
+ def default(self, obj): # pylint: disable=method-hidden
"""
Print a nicely formatted complex number, or default to the JSON encoder
"""
@@ -157,7 +158,12 @@ class CapaFields(object):
{"display_name": _("Per Student"), "value": RANDOMIZATION.PER_STUDENT}
]
)
- data = String(help=_("XML data for the problem"), scope=Scope.content, default="<problem></problem>")
+ data = XMLString(
+ help=_("XML data for the problem"),
+ scope=Scope.content,
+ enforce_type=FEATURES.get('ENABLE_XBLOCK_XML_VALIDATION', True),
+ default="<problem></problem>"
+ )
correct_map = Dict(help=_("Dictionary with the correctness of current student answers"),
scope=Scope.user_state, default={})
input_state = Dict(help=_("Dictionary for maintaining the state of inputtypes"), scope=Scope.user_state)
@@ -257,11 +263,13 @@ class CapaMixin(CapaFields):
)
)
# create a dummy problem with error message instead of failing
- problem_text = (u'<problem><text><span class="inline-error">'
- u'Problem {url} has an error:</span>{msg}</text></problem>'.format(
- url=self.location.to_deprecated_string(),
- msg=msg)
- )
+ problem_text = (
+ u'<problem><text><span class="inline-error">'
+ u'Problem {url} has an error:</span>{msg}</text></problem>'.format(
+ url=self.location.to_deprecated_string(),
+ msg=msg,
+ )
+ )
self.lcp = self.new_lcp(self.get_state_for_lcp(), text=problem_text)
else:
# add extra info and raise
@@ -349,7 +357,7 @@ class CapaMixin(CapaFields):
"""
Set the module's last submission time (when the problem was submitted)
"""
- self.last_submission_time = datetime.datetime.now(UTC())
+ self.last_submission_time = datetime.datetime.now(utc)
def get_score(self):
"""
@@ -803,7 +811,7 @@ class CapaMixin(CapaFields):
Is it now past this problem's due date, including grace period?
"""
return (self.close_date is not None and
- datetime.datetime.now(UTC()) > self.close_date)
+ datetime.datetime.now(utc) > self.close_date)
def closed(self):
"""
@@ -1093,7 +1101,7 @@ class CapaMixin(CapaFields):
metric_name = u'capa.check_problem.{}'.format
# Can override current time
- current_time = datetime.datetime.now(UTC())
+ current_time = datetime.datetime.now(utc)
if override_time is not False:
current_time = override_time
@@ -1128,8 +1136,9 @@ class CapaMixin(CapaFields):
# Wait time between resets: check if is too soon for submission.
if self.last_submission_time is not None and self.submission_wait_seconds != 0:
- if (current_time - self.last_submission_time).total_seconds() < self.submission_wait_seconds:
- remaining_secs = int(self.submission_wait_seconds - (current_time - self.last_submission_time).total_seconds())
+ seconds_since_submission = (current_time - self.last_submission_time).total_seconds()
+ if seconds_since_submission < self.submission_wait_seconds:
+ remaining_secs = int(self.submission_wait_seconds - seconds_since_submission)
msg = _(u'You must wait at least {wait_secs} between submissions. {remaining_secs} remaining.').format(
wait_secs=self.pretty_print_seconds(self.submission_wait_seconds),
remaining_secs=self.pretty_print_seconds(remaining_secs))
@@ -1343,7 +1352,7 @@ class CapaMixin(CapaFields):
log.warning('Input id %s is not mapped to an input type.', input_id)
answer_response = None
- for response, responder in self.lcp.responders.iteritems():
+ for responder in self.lcp.responders.itervalues():
if input_id in responder.answer_ids:
answer_response = responder
@@ -1406,8 +1415,10 @@ class CapaMixin(CapaFields):
if not self.lcp.supports_rescoring():
event_info['failure'] = 'unsupported'
self.track_function_unmask('problem_rescore_fail', event_info)
+ # pylint: disable=line-too-long
# Translators: 'rescoring' refers to the act of re-submitting a student's solution so it can get a new score.
raise NotImplementedError(_("Problem's definition does not support rescoring."))
+ # pylint: enable=line-too-long
if not self.done:
event_info['failure'] = 'unanswered'
@@ -1485,8 +1496,10 @@ class CapaMixin(CapaFields):
self.track_function_unmask('save_problem_fail', event_info)
return {
'success': False,
+ # pylint: disable=line-too-long
# Translators: 'closed' means the problem's due date has passed. You may no longer attempt to solve the problem.
- 'msg': _("Problem is closed.")
+ 'msg': _("Problem is closed."),
+ # pylint: enable=line-too-long
}
# Problem submitted. Student should reset before saving
@@ -1538,8 +1551,10 @@ class CapaMixin(CapaFields):
self.track_function_unmask('reset_problem_fail', event_info)
return {
'success': False,
+ # pylint: disable=line-too-long
# Translators: 'closed' means the problem's due date has passed. You may no longer attempt to solve the problem.
'msg': _("You cannot select Reset for a problem that is closed."),
+ # pylint: enable=line-too-long
}
if not self.is_submitted():
diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py
index 182f1cf5494cc27aadf6c5ca8ff047dc020486b7..abeb12b143cf2f573fa6a166568335ac01c87afa 100644
--- a/common/lib/xmodule/xmodule/tests/test_capa_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py
@@ -81,14 +81,7 @@ class CapaFactory(object):
)
@classmethod
- def create(cls,
- attempts=None,
- problem_state=None,
- correct=False,
- xml=None,
- override_get_score=True,
- **kwargs
- ):
+ def create(cls, attempts=None, problem_state=None, correct=False, xml=None, override_get_score=True, **kwargs):
"""
All parameters are optional, and are added to the created problem if specified.
@@ -228,7 +221,6 @@ class CapaModuleTest(unittest.TestCase):
useful as unit-code coverage for this current implementation. I don't see a layer where LoncapaProblem
is tested directly
"""
- from capa.correctmap import CorrectMap
student_answers = {'1_2_1': 'abcd'}
correct_map = CorrectMap(answer_id='1_2_1', correctness="correct", npoints=0.9)
module = CapaFactory.create(correct=True, override_get_score=False)
@@ -623,6 +615,7 @@ class CapaModuleTest(unittest.TestCase):
module.submit_problem(get_request_dict)
+ # pylint: disable=line-too-long
# _http_post is called like this:
# _http_post(
# 'http://example.com/xqueue/xqueue/submit/',
@@ -639,9 +632,10 @@ class CapaModuleTest(unittest.TestCase):
# <open file u'/home/ned/edx/edx-platform/common/test/data/uploads/textbook.pdf', mode 'r' at 0x49c5a50>,
# },
# )
+ # pylint: enable=line-too-long
self.assertEqual(xqueue_interface._http_post.call_count, 1)
- _, kwargs = xqueue_interface._http_post.call_args
+ _, kwargs = xqueue_interface._http_post.call_args # pylint: disable=unpacking-non-sequence
self.assertItemsEqual(fpaths, kwargs['files'].keys())
for fpath, fileobj in kwargs['files'].iteritems():
self.assertEqual(fpath, fileobj.name)
@@ -674,7 +668,7 @@ class CapaModuleTest(unittest.TestCase):
module.handle('xmodule_handler', request, 'problem_check')
self.assertEqual(xqueue_interface._http_post.call_count, 1)
- _, kwargs = xqueue_interface._http_post.call_args
+ _, kwargs = xqueue_interface._http_post.call_args # pylint: disable=unpacking-non-sequence
self.assertItemsEqual(fnames, kwargs['files'].keys())
for fpath, fileobj in kwargs['files'].iteritems():
self.assertEqual(fpath, fileobj.name)
@@ -2487,18 +2481,15 @@ class CapaDescriptorTest(unittest.TestCase):
def test_invalid_xml_handling(self):
"""
- Tests to confirm that invalid XML does not throw a wake-up-ops level error.
- See TNL-5057 for quick fix, TNL-5245 for full resolution.
+ Tests to confirm that invalid XML throws errors during xblock creation,
+ so as not to allow bad data into modulestore.
"""
sample_invalid_xml = textwrap.dedent("""
<problem>
</proble-oh no my finger broke and I can't close the problem tag properly...
""")
- descriptor = self._create_descriptor(sample_invalid_xml, name="Invalid XML")
- try:
- descriptor.has_support(None, "multi_device")
- except etree.XMLSyntaxError:
- self.fail("Exception raised during XML parsing, this method should be resilient to such errors")
+ with self.assertRaises(etree.XMLSyntaxError):
+ self._create_descriptor(sample_invalid_xml, name="Invalid XML")
class ComplexEncoderTest(unittest.TestCase):
diff --git a/common/test/acceptance/tests/studio/test_import_export.py b/common/test/acceptance/tests/studio/test_import_export.py
index 216d1ae60551d3c344e341600e78b62c8e8900d5..d368a6c345b24c83f92fdc43966d62c8b91f5e3c 100644
--- a/common/test/acceptance/tests/studio/test_import_export.py
+++ b/common/test/acceptance/tests/studio/test_import_export.py
@@ -7,17 +7,14 @@ from datetime import datetime
from flaky import flaky
from abc import abstractmethod
-from bok_choy.promise import EmptyPromise
from common.test.acceptance.tests.studio.base_studio_test import StudioLibraryTest, StudioCourseTest
-from common.test.acceptance.fixtures.course import XBlockFixtureDesc
from common.test.acceptance.pages.studio.import_export import (
ExportLibraryPage,
ExportCoursePage,
ImportLibraryPage,
ImportCoursePage)
from common.test.acceptance.pages.studio.library import LibraryEditPage
-from common.test.acceptance.pages.studio.container import ContainerPage
from common.test.acceptance.pages.studio.overview import CourseOutlinePage
from common.test.acceptance.pages.lms.courseware import CoursewarePage
from common.test.acceptance.pages.lms.staff_view import StaffPage
@@ -86,84 +83,6 @@ class TestLibraryExport(ExportTestMixin, StudioLibraryTest):
self.assertEqual(self.export_page.header_text, 'Library Export')
-class BadExportMixin(object):
- """
- Test mixin for bad exports.
- """
- def test_bad_export(self):
- """
- Scenario: I should receive an error when attempting to export a broken course or library.
- Given that I have a course or library
- No error modal should be showing
- When I click the export button
- An error modal should be shown
- When I click the modal's action button
- I should arrive at the edit page for the broken component
- """
- # No error should be there to start.
- self.assertFalse(self.export_page.is_error_modal_showing())
- self.export_page.click_export()
- self.export_page.wait_for_error_modal()
- self.export_page.click_modal_button()
- self.edit_page.wait_for_page()
-
-
-@attr(shard=7)
-class TestLibraryBadExport(BadExportMixin, StudioLibraryTest):
- """
- Verify exporting a bad library causes an error.
- """
-
- def setUp(self):
- """
- Set up the pages and start the tests.
- """
- super(TestLibraryBadExport, self).setUp()
- self.export_page = ExportLibraryPage(self.browser, self.library_key)
- self.edit_page = LibraryEditPage(self.browser, self.library_key)
- self.export_page.visit()
-
- def populate_library_fixture(self, library_fixture):
- """
- Create a library with a bad component.
- """
- library_fixture.add_children(
- XBlockFixtureDesc("problem", "Bad Problem", data='<'),
- )
-
-
-@attr(shard=7)
-class TestCourseBadExport(BadExportMixin, StudioCourseTest):
- """
- Verify exporting a bad course causes an error.
- """
- ready_method = 'wait_for_component_menu'
-
- def setUp(self): # pylint: disable=arguments-differ
- super(TestCourseBadExport, self).setUp()
- self.export_page = ExportCoursePage(
- self.browser,
- self.course_info['org'], self.course_info['number'], self.course_info['run'],
- )
- self.edit_page = ContainerPage(self.browser, self.unit.locator)
- self.export_page.visit()
-
- def populate_course_fixture(self, course_fixture):
- """
- Populate the course with a unit that has a bad problem.
- """
- self.unit = XBlockFixtureDesc('vertical', 'Unit')
- course_fixture.add_children(
- XBlockFixtureDesc('chapter', 'Main Section').add_children(
- XBlockFixtureDesc('sequential', 'Subsection').add_children(
- self.unit.add_children(
- XBlockFixtureDesc("problem", "Bad Problem", data='<')
- )
- )
- )
- )
-
-
@attr(shard=7)
class ImportTestMixin(object):
"""
diff --git a/lms/envs/common.py b/lms/envs/common.py
index f380bb25774cb4983cfb10cd543807323e96976c..195908667028cfa5c515f5ceaec78ee3b81c38a2 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -361,6 +361,9 @@ FEATURES = {
# Note: This has no effect unless ANALYTICS_DASHBOARD_URL is already set,
# because without that setting, the tab does not show up for any courses.
'ENABLE_CCX_ANALYTICS_DASHBOARD_URL': False,
+
+ # Set this to False to facilitate cleaning up invalid xml from your modulestore.
+ 'ENABLE_XBLOCK_XML_VALIDATION': True,
}
# Ignore static asset files on import which match this pattern
diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt
index e38fbc4b6366a0b2fd655177740f713633509edf..7d98714efbbd4e60991c88356b1d9fa33a6b3719 100644
--- a/requirements/edx/github.txt
+++ b/requirements/edx/github.txt
@@ -71,7 +71,7 @@ git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266==0.0.5-edx
git+https://github.com/edx/lettuce.git@0.2.20.002#egg=lettuce==0.2.20.002
# Our libraries:
-git+https://github.com/edx/XBlock.git@xblock-0.4.12#egg=XBlock==0.4.12
+git+https://github.com/edx/XBlock.git@xblock-0.4.13#egg=XBlock==0.4.13
-e git+https://github.com/edx/codejail.git@6b17c33a89bef0ac510926b1d7fea2748b73aadd#egg=codejail
-e git+https://github.com/edx/event-tracking.git@0.2.1#egg=event-tracking==0.2.1
-e git+https://github.com/edx/django-splash.git@v0.2#egg=django-splash==0.2