Added escaping to activity_list_select and test

ckan/lib/helpers.py

@@ -36,6 +36,7 @@
 from werkzeug.routing import BuildError as FlaskRouteBuildError
 import i18n
 from six import string_types, text_type
+import jinja2
 
 import ckan.exceptions
 import ckan.model as model
@@ -2735,19 +2736,20 @@ def activity_list_select(pkg_activity_list, current_activity_id):
     on the "Changes" summary page.
     '''
     select_list = []
+    template = jinja2.Template(
+        u'<option value="{{activity_id}}" {{selected}}>'
+        '{{timestamp}}</option>',
+        autoescape=True)
     for activity in pkg_activity_list:
         entry = render_datetime(activity['timestamp'],
                                 with_hours=True,
                                 with_seconds=True)
-        if activity['id'] == current_activity_id:
-            select_list.append(
-                "<option value=\"" + activity['id'] +
-                "\" selected>" + entry + "</option>"
-            )
-        else:
-            select_list.append(
-                "<option value=\"" + activity['id'] + "\">" +
-                entry + "</option>"
-            )
+        select_list.append(Markup(
+            template
+            .render(activity_id=activity['id'], timestamp=entry,
+                    selected='selected'
+                    if activity['id'] == current_activity_id
+                    else '')
+        ))
 
     return select_list

ckan/templates/package/changes.html

@@ -27,12 +27,12 @@ <h1 class="page-heading">{{ _('Changes') }}</h1>
       View changes from
         <select class="form-control select-time" form="range_form" name="oldest_id">
           <pre>
-            {{ select_list1[1:]|safe }}
+            {{ select_list1[1:]|join }}
           </pre>
         </select> to
         <select class="form-control select-time" form="range_form" name="newest_id">
           <pre>
-            {{ select_list2|safe }}
+            {{ select_list2|join }}
           </pre>
         </select>
         </form>

ckan/tests/lib/test_helpers.py

@@ -790,3 +790,49 @@ def helper_as_attribute(self):
 
     def helper_as_item(self):
         return base.render('tests/helper_as_item.html')
+
+
+class TestActivityListSelect(object):
+
+    def setup(self):
+        helpers.reset_db()
+
+    def test_simple(self):
+        pkg_activity = {
+            'id': 'id1',
+            'timestamp': datetime.datetime(2018, 2, 1, 10, 58, 59),
+        }
+
+        out = h.activity_list_select([pkg_activity], '')
+
+        html = out[0]
+        eq_(str(html),
+            '<option value="id1" >February 1, 2018, 10:58:59 (UTC)'
+            '</option>')
+        assert hasattr(html, '__html__')  # shows it is safe Markup
+
+    def test_selected(self):
+        pkg_activity = {
+            'id': 'id1',
+            'timestamp': datetime.datetime(2018, 2, 1, 10, 58, 59),
+        }
+
+        out = h.activity_list_select([pkg_activity], 'id1')
+
+        html = out[0]
+        print html
+        eq_(str(html),
+            '<option value="id1" selected>February 1, 2018, 10:58:59 (UTC)'
+            '</option>')
+        assert hasattr(html, '__html__')  # shows it is safe Markup
+
+    def test_escaping(self):
+        pkg_activity = {
+            'id': '">',  # hacked somehow
+            'timestamp': datetime.datetime(2018, 2, 1, 10, 58, 59),
+        }
+
+        out = h.activity_list_select([pkg_activity], '')
+
+        html = out[0]
+        assert str(html).startswith(u'<option value="&#34;&gt;" >')