Fix rendering of analytics using Django safe strings

The previous solution, introduced when fixing
GHSA-3jmp-r88c-34j9/CVE-2024-32405 used bulk
escaping using bleach, which led to many unsightly rendered-as-text HTML
elements.

Author: inducer

course/analytics.py

@@ -543,9 +543,6 @@ def page_analytics(pctx, flow_id, group_id, page_id):
                     grading_page_context, visit.page_data.data, visit.answer)
         if normalized_answer is None:
             normalized_answer = _("(No answer)")
-        else:
-            import bleach
-            normalized_answer = bleach.clean(normalized_answer)
 
         answer_feedback = visit.get_most_recent_feedback()
 

course/page/base.py

@@ -736,8 +736,10 @@ def normalized_answer(
             answer_data: Any
             ) -> str | None:
         """An HTML-formatted answer to be used for summarization and
-        display in analytics. Since this may include user input, it is
-        expected to be sanitized.
+        display in analytics. By default, this is escaped when included
+        in the output page, however it may use utilities such as
+        :func:`~django.utils.safestring.mark_safe` to mark the output
+        as safe and avoid escaping.
         """
         return None
 

course/page/choice.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from django.utils.html import format_html_join
+
 
 __copyright__ = "Copyright (C) 2014 Andreas Kloeckner"
 
@@ -647,19 +649,15 @@ def grade(self, page_context, page_data, answer_data, grade_data):
         return AnswerFeedback(correctness=correctness)
 
     def get_answer_html(self, page_context, idx_list, unpermute=False):
-        answer_html_list = []
         if unpermute:
             idx_list = list(set(idx_list))
-        for idx in idx_list:
-            answer_html_list.append(
-                    "<li>"
-                    + (self.process_choice_string(
+
+        return format_html_join(
+            "\n", "{}",
+            [(self.process_choice_string(
                         page_context,
-                        self.choices[idx].text))
-                    + "</li>"
-                    )
-        answer_html = "<ul>"+"".join(answer_html_list)+"</ul>"
-        return answer_html
+                        self.choices[idx].text),)
+            for idx in idx_list])
 
     def correct_answer(self, page_context, page_data, answer_data, grade_data):
         corr_idx_list = self.unpermuted_correct_indices()

course/page/code.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from django.utils.safestring import mark_safe
+
 
 __copyright__ = "Copyright (C) 2014 Andreas Kloeckner"
 
@@ -1067,7 +1069,7 @@ def normalized_answer(self, page_context, page_data, answer_data):
         normalized_answer = self.get_code_from_answer_data(answer_data)
 
         from django.utils.html import escape
-        return f"<pre>{escape(normalized_answer)}</pre>"
+        return mark_safe(f"<pre>{escape(normalized_answer)}</pre>")
 
     def normalized_bytes_answer(self, page_context, page_data, answer_data):
         if answer_data is None:

course/page/inline.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from django.utils.html import format_html_join
+
 
 __copyright__ = "Copyright (C) 2015 Andreas Kloeckner, Dong Zhuang"
 
@@ -347,8 +349,7 @@ def get_width_str(self, opt_width=0):
         return "width: " + str(max(self.width, opt_width)) + "em"
 
     def get_answer_text(self, page_context, answer):
-        from django.utils.html import escape
-        return escape(answer)
+        return answer
 
     def get_correct_answer_text(self, page_context):
 
@@ -472,8 +473,9 @@ def __init__(self, vctx, location, name, answers_desc):
     def get_answer_text(self, page_context, answer):
         if answer == "":
             return answer
-        return self.process_choice_string(
-            page_context, self.answers_desc.choices[int(answer)])
+        return mark_safe(
+            self.process_choice_string(
+                page_context, self.answers_desc.choices[int(answer)]))
 
     def get_width_str(self, opt_width=0):
         return None
@@ -979,9 +981,9 @@ def normalized_answer(self, page_context, page_data, answer_data):
 
         answer_dict = answer_data["answer"]
 
-        return ", ".join(
-            [self.answer_instance_list[idx].get_answer_text(
-                page_context, answer_dict[self.embedded_name_list[idx]])
+        return format_html_join(", ", "{}",
+            [(self.answer_instance_list[idx].get_answer_text(
+                page_context, answer_dict[self.embedded_name_list[idx]]),)
                 for idx, name in enumerate(self.embedded_name_list)]
         )
 

course/page/text.py

@@ -766,8 +766,7 @@ def normalized_answer(self, page_context, page_data, answer_data):
         if not self._is_case_sensitive():
             normalized_answer = normalized_answer.lower()
 
-        from django.utils.html import escape
-        return escape(normalized_answer)
+        return normalized_answer
 
     def normalized_bytes_answer(self, page_context, page_data, answer_data):
         if answer_data is None:

course/templates/course/analytics-page.html

@@ -38,7 +38,7 @@ <h2>{% trans "Answer Distribution" %}</h2>
   <div class="answer-analytics">
   {% for astats in answer_stats_list %}
     <div class="answer-entry">
-      {{ astats.normalized_answer|safe }}
+      {{ astats.normalized_answer }}
       {% blocktrans trimmed with astats_count=astats.count count counter=astats.count %}
         ({{ astats_count }} response)
       {% plural %}

frontend/css/style.scss

@@ -118,6 +118,11 @@ label div.relate-markup p:only-child {
   margin-bottom: 0;
 }
 
+/* remove extra spacing from ChoiceQuestion options in analytics */
+div.answer-analytics div.relate-markup p:only-child {
+  margin-bottom: 0;
+}
+
 /* }}} */
 
 /* {{{ histograms */

tests/test_pages/test_choice.py

@@ -439,12 +439,11 @@ def test_choice_item_with_always_correct(self):
         self.assertResponseContextEqual(
             resp, "correct_answer",
             "The correct answer is: "
-            "<ul><li><div class='relate-markup'><p>Sprinkles</p></div></li>"
-            "<li><div class='relate-markup'><p>Chocolate chunks</p></div></li>"
-            "<li><div class='relate-markup'><p>Almond bits</p></div></li>"
-            "</ul>"
+            "<div class='relate-markup'><p>Sprinkles</p></div>\n"
+            "<div class='relate-markup'><p>Chocolate chunks</p></div>\n"
+            "<div class='relate-markup'><p>Almond bits</p></div>"
             "Additional acceptable options are: "
-            "<ul><li><div class='relate-markup'><p>A flawed option</p></div></li></ul>")
+            "<div class='relate-markup'><p>A flawed option</p></div>")
 
     # }}}