Merge pull request #3 from edx/fsheets/EDUCATOR-3346

Strip and clean URL field being saved
openedx · Aug 21, 2018 · b533a2e · b533a2e
2 parents 2c5e36f + df88935
commit b533a2e
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 15 deletions.
diff --git a/recommender/recommender.py b/recommender/recommender.py
@@ -7,6 +7,7 @@
 import lxml.etree as etree
 import pkg_resources
 import bleach
+import re
 
 from copy import deepcopy
 
@@ -586,7 +587,7 @@ def add_resource(self, data, _suffix=''):  # pylint: disable=unused-argument
         for field in self.resource_content_fields:
             result[field] = strip_and_clean_html_elements(data[field])
 
-        resource_id = stem_url(data['url'])
+        resource_id = stem_url(result['url'])
         self._check_redundant_resource(resource_id, 'add_resource', result)
         self._check_removed_resource(resource_id, 'add_resource', result)
 
@@ -925,11 +926,12 @@ def student_view(self, _context=None):  # pylint: disable=unused-argument
         # load continues to be as-is, pre-generation is not a performance
         # issue. If students make substantially more resources, we may want
         # to paginate, and generate in sets of 5-20 URLs per load.
-        resources = [{'id': r['id'],
+        resources = [{
+                      'id': strip_and_clean_html_elements(r['id']),
                       'title': strip_and_clean_html_elements(r['title']),
-                      "votes": r['upvotes'] - r['downvotes'],
-                      'url': r['url'],
-                      'description': self._get_onetime_url(r['description']),
+                      "votes": strip_and_clean_html_elements(r['upvotes'] - r['downvotes']),
+                      'url': strip_and_clean_url(r['url']),
+                      'description': self._get_onetime_url(strip_and_clean_html_elements(r['description'])),
                       'descriptionText': strip_and_clean_html_elements(r['descriptionText'])
                       }
                      for r in self.recommendations.values()]
@@ -1060,4 +1062,27 @@ def strip_and_clean_html_elements(data):
     """
     Clean an HTML elements and return it
     """
-    return bleach.clean(data, strip=True)
+    return bleach.clean(data, tags=[], strip=True)
+
+def strip_and_clean_url(data):
+    """
+    Clean an URL elements of HTML tags and possible javascript and return it for use
+    Ex of bleach linkify output:
+        bleach.linkify('google.com') ==> u'<a rel="nofollow" href="http://google.com">google.com</a>'
+        bleach.linkify('<a href="javascript:alert()"  google.com') ==> u''
+    Ex of bleach linkify with clean output:
+        bleach.linkify(bleach.clean('<a> href="javascript:alert()"  google.com', tags=[], strip=True))
+        ==> u' href="javascript:alert()"  <a rel="nofollow" href="http://google.com">google.com</a>'
+    """
+    clean_url = data or ''
+    clean_url = strip_and_clean_html_elements(clean_url)
+    # ensure <a> only exists after linkify call below
+    if '<a' in clean_url:
+        return ''
+
+    clean_url = bleach.linkify(clean_url)
+    if clean_url.startswith(u'<a'):
+        # The regex pulls out the href value of the generated <a>
+        return re.search('href=\"(?P<href>.*?)\"', clean_url).group('href')
+    else:
+        return ''
diff --git a/recommender/static/js/src/recommender.js b/recommender/static/js/src/recommender.js
@@ -396,20 +396,20 @@ function RecommenderXBlock(runtime, element, init_data) {
             resourceVotes: votes
         }
 
-        var newDiv = $(Mustache.render($("#recommenderResourceTemplate").html(), renderData));
-        bindResourceDependentEvent(newDiv);
-        if (IS_USER_STAFF) { bindStaffLimitedResourceDependentEvent(newDiv); }
+        var $newDiv = $(Mustache.render($("#recommenderResourceTemplate").html(), renderData));
+        bindResourceDependentEvent($newDiv);
+        if (IS_USER_STAFF) { bindStaffLimitedResourceDependentEvent($newDiv); }
 
         if ($('.recommenderResource', element).length === 0) {
-            $('.noResourceIntro', element).after(newDiv);
+            $('.noResourceIntro', element).after($newDiv);
         }
         else {
-            if (pos === -1) { $(toDiv).after(newDiv); }
-            else { $(toDiv).before(newDiv); }
+            if (pos === -1) { $(toDiv).after($newDiv); }
+            else { $(toDiv).before($newDiv); }
         }
-        addResourceDependentTooltip(newDiv);
+        addResourceDependentTooltip($newDiv);
 
-        return newDiv;
+        return $newDiv;
     }
 
     /**

diff --git a/setup.py b/setup.py
@@ -46,7 +46,7 @@ def package_data(pkg, root_list):
 
 setup(
     name='recommender-xblock',
-    version='1.3.1',
+    version='1.3.2',
     description='recommender XBlock',   # TODO: write a better description.
     long_description=README,
     author='edX',