From a44ecdfcd6e01ea6f38b82f6c9348a955808c87f Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 26 Mar 2013 16:45:47 -0400 Subject: [PATCH 1/2] if we parse an invalid location in the content store middleware, then return a 404, not a 500 --- common/djangoapps/contentserver/middleware.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index c5e887801e..e8674a1e9e 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -5,6 +5,7 @@ from django.http import HttpResponse, Http404, HttpResponseNotModified from xmodule.contentstore.django import contentstore from xmodule.contentstore.content import StaticContent, XASSET_LOCATION_TAG +from xmodule.modulestore import InvalidLocationError from cache_toolbox.core import get_cached_content, set_cached_content from xmodule.exceptions import NotFoundError @@ -13,7 +14,13 @@ class StaticContentServer(object): def process_request(self, request): # look to see if the request is prefixed with 'c4x' tag if request.path.startswith('/' + XASSET_LOCATION_TAG + '/'): - loc = StaticContent.get_location_from_path(request.path) + try: + loc = StaticContent.get_location_from_path(request.path) + except InvalidLocationError: + response = HttpResponse() + response.status_code = 404 + return response + # first look in our cache so we don't have to round-trip to the DB content = get_cached_content(loc) if content is None: From b0e2c82ad3619bea30674562e347cd76b9856de4 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 26 Mar 2013 20:02:29 -0400 Subject: [PATCH 2/2] actually.. return a 400 rather than a 404 because the request is malformed. Also add unit test. --- cms/djangoapps/contentstore/tests/test_contentstore.py | 4 ++++ common/djangoapps/contentserver/middleware.py | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index edb20561bc..a8cde71379 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -213,6 +213,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): resp = self.client.get(reverse('edit_unit', kwargs={'location': new_loc.url()})) self.assertEqual(resp.status_code, 200) + def test_bad_contentstore_request(self): + resp = self.client.get('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png') + self.assertEqual(resp.status_code, 400) + def test_delete_course(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index e8674a1e9e..8e9e70046d 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -17,8 +17,9 @@ class StaticContentServer(object): try: loc = StaticContent.get_location_from_path(request.path) except InvalidLocationError: + # return a 'Bad Request' to browser as we have a malformed Location response = HttpResponse() - response.status_code = 404 + response.status_code = 400 return response # first look in our cache so we don't have to round-trip to the DB