From 37ec01dd8dd9e4d6f3e3fc6fc573b64d945a5fa5 Mon Sep 17 00:00:00 2001
From: Greg Price <gprice@edx.org>
Date: Thu, 16 May 2013 10:05:13 -0400
Subject: [PATCH] Handle ItemNotFoundError from the modulestore to avoid 500
 errors

---
 lms/djangoapps/courseware/module_render.py    | 18 ++++++++++-
 .../courseware/tests/test_module_render.py    | 31 +++++++++++++++----
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 6f05b32778..c11fcbeeba 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -402,6 +402,11 @@ def modx_dispatch(request, dispatch, location, course_id):
            through the part before the first '?'.
       - location -- the module location. Used to look up the XModule instance
       - course_id -- defines the course context for this request.
+
+    Raises PermissionDenied if the user is not logged in. Raises Http404 if
+    the location and course_id do not identify a valid module, the module is
+    not accessible by the user, or the module raises NotFoundError. If the
+    module raises any other error, it will escape this function.
     '''
     # ''' (fix emacs broken parsing)
 
@@ -430,8 +435,19 @@ def modx_dispatch(request, dispatch, location, course_id):
                     return HttpResponse(json.dumps({'success': file_too_big_msg}))
             p[fileinput_id] = inputfiles
 
+    try:
+        descriptor = modulestore().get_instance(course_id, location)
+    except ItemNotFoundError:
+        log.warn(
+            "Invalid location for course id {course_id}: {location}".format(
+                course_id=course_id,
+                location=location
+            )
+        )
+        raise Http404
+
     model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id,
-        request.user, modulestore().get_instance(course_id, location))
+        request.user, descriptor)
 
     instance = get_module(request.user, request, location, model_data_cache, course_id, grade_bucket_type='ajax')
     if instance is None:
diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py
index 0e4ba8ba5e..94ab4b7e94 100644
--- a/lms/djangoapps/courseware/tests/test_module_render.py
+++ b/lms/djangoapps/courseware/tests/test_module_render.py
@@ -69,19 +69,38 @@ class ModuleRenderTestCase(LoginEnrollmentTestCase):
                           json.dumps({'success': 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %
                                       (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2))}))
         mock_request_3 = MagicMock()
-        mock_request_3.POST.copy.return_value = {}
+        mock_request_3.POST.copy.return_value = {'position': 1}
         mock_request_3.FILES = False
         mock_request_3.user = UserFactory()
         inputfile_2 = Stub()
         inputfile_2.size = 1
         inputfile_2.name = 'name'
-        self.assertRaises(ItemNotFoundError, render.modx_dispatch,
-                          mock_request_3, 'dummy', self.location, 'toy')
-        self.assertRaises(Http404, render.modx_dispatch, mock_request_3, 'dummy',
-                          self.location, self.course_id)
-        mock_request_3.POST.copy.return_value = {'position': 1}
         self.assertIsInstance(render.modx_dispatch(mock_request_3, 'goto_position',
                                                    self.location, self.course_id), HttpResponse)
+        self.assertRaises(
+            Http404,
+            render.modx_dispatch,
+            mock_request_3,
+            'goto_position',
+            self.location,
+            'bad_course_id'
+        )
+        self.assertRaises(
+            Http404,
+            render.modx_dispatch,
+            mock_request_3,
+            'goto_position',
+            ['i4x', 'edX', 'toy', 'chapter', 'bad_location'],
+            self.course_id
+        )
+        self.assertRaises(
+            Http404,
+            render.modx_dispatch,
+            mock_request_3,
+            'bad_dispatch',
+            self.location,
+            self.course_id
+        )
 
     def test_get_score_bucket(self):
         self.assertEquals(render.get_score_bucket(0, 10), 'incorrect')