Merge pull request #1351 from chris34/markup-changes

Markup changes

ChangeLog.rst

@@ -25,17 +25,14 @@ Inyoka Changelog
    -----------
 
 
-Unreleased AA.BB.CC (2024-MM-DD)
+Unreleased 1.0.0 (2024-10-12)
 =====================
 
 Deployment notes
 ----------------
 
 #. Update requirements
 
-✨ New features
----------------
-
 🏗 Changes
 ----------
 
@@ -47,9 +44,12 @@ Deployment notes
 * Documentation: Now possible to use Markdown
 * Documentation is now published at https://doc.inyokaproject.org/
 * Use Django's view and form for change password
-
-🗑 Deprecations
---------------
+* Restrict user defineable font faces: Only ``[font=Arial]``, ``[font=serif]``, ``[font=sans-serif]`` and ``[font=Courier]`` are allowed
+* Disallow ``<color>`` and ``<font>`` in signatures
+* InyokaMarkup: Extend filtering of control characters
+* InyokaMarkup: Remove empty paragraphs in generated HTML
+* InyokaMarkup: Dont split up long links in HTML-markup (instead rely on CSS)
+* Table of contents: Dont strip long heading text
 
 🔥 Removals
 -----------
@@ -60,12 +60,15 @@ Deployment notes
 --------
 
 * Splittopic form: Fix maximum length for title of new topic
+* Forum posts & Ikhaya comments can now start with a list (space is preserved)
 
 🔒 Security
 -----------
 
 * Add ``SECURITY.md``
 * Update requirements (at least the dependencies ``Django`` include known security fixes)
+* Markup, Edited-/Mod boxes: Escape parameters to prevent HTML injection
+* Templates: Escape more user-controllable variables to prevent HTML injections
 
 0.36.1 (2024-08-06)
 ===================

inyoka/forum/jinja2/forum/edit.html

@@ -20,7 +20,7 @@
 {% if not topic %}
   {% do tmp_crumb.append((_('New topic'), forum|url('newtopic'))) %}
 {% else %}
-  {% do tmp_crumb.append((topic.title, topic|url)) %}
+  {% do tmp_crumb.append((topic.title|e, topic|url)) %}
   {% if post %}
     {% do tmp_crumb.append((_('Edit post'), '')) %}
   {% else %}

inyoka/forum/jinja2/forum/movetopic.html

@@ -13,7 +13,7 @@
 {% for parent in topic.forum.parents|reverse %}
   {% do tmp_crumb.append((parent.name, parent|url)) %}
 {% endfor %}
-{% do tmp_crumb.extend([(topic.forum.name, topic.forum|url), (topic.title, topic|url),
+{% do tmp_crumb.extend([(topic.forum.name, topic.forum|url), (topic.title|e, topic|url),
                         (_('Move'), topic|url('move'))])%}
 {% set BREADCRUMBS = tmp_crumb + BREADCRUMBS|d([]) %}
 

inyoka/forum/jinja2/forum/postlist.html

@@ -13,12 +13,12 @@
 {% from 'macros.html' import render_pagination, render_small_pagination %}
 {% set rendered_pagination = render_pagination(pagination) %}
 
-{% set tmp_crumb = [(username, href('forum', 'author', username|e))] %}
+{% set tmp_crumb = [(username|e, href('forum', 'author', username|e))] %}
 {% if forum %}
     {% do tmp_crumb.append((forum.name, href('forum', 'author', username|e, 'forum', forum.slug))) %}
 {% endif %}
 {% if topic %}
-    {% do tmp_crumb.append((topic.title, href('forum', 'author', username|e, 'topic', topic.slug))) %}
+    {% do tmp_crumb.append((topic.title|e, href('forum', 'author', username|e, 'topic', topic.slug))) %}
 {% endif %}
 
 {% set BREADCRUMBS = tmp_crumb + BREADCRUMBS|d([]) %}

inyoka/forum/jinja2/forum/report.html

@@ -11,7 +11,7 @@
 
 {%- extends 'forum/page.html' %}
 {% set BREADCRUMBS = [(_('Report'), topic|url('report')), (topic.forum.name, topic.forum|url),
-                        (topic.title, topic|url)] + BREADCRUMBS|d([]) %}
+                        (topic.title|e, topic|url)] + BREADCRUMBS|d([]) %}
 
 {% block forum_content %}
   <form action="" method="post" enctype="multipart/form-data" class="new_topic">

inyoka/forum/jinja2/forum/revisions.html

@@ -10,7 +10,7 @@
 #}
 
 {%- extends 'forum/page.html' %}
-{% set BREADCRUMBS = [(post.topic.title, post.topic|url),
+{% set BREADCRUMBS = [(post.topic.title|e, post.topic|url),
                       (_('Old revisions'), post|url('revisions'))] + BREADCRUMBS|d([]) %}
 
 {% block forum_content %}

inyoka/forum/jinja2/forum/splittopic.html

@@ -11,7 +11,7 @@
 {% import 'macros.html' as macros %}
 {% set BREADCRUMBS = [(_('Split topic'), topic|url('split')),
                       (topic.forum.name, topic.forum|url),
-                      (topic.title, topic|url)] + BREADCRUMBS|d([]) %}
+                      (topic.title|e, topic|url)] + BREADCRUMBS|d([]) %}
 
 {% block forum_content %}
   <form action="" method="post" class="new_topic">

inyoka/forum/jinja2/forum/topic.html

@@ -24,7 +24,7 @@
 {% for parent in forum.parents|reverse %}
   {% do tmp_crumb.append((parent.name, parent|url)) %}
 {% endfor %}
-{% do tmp_crumb.extend([(forum.name, forum|url), (topic.title, topic|url)]) %}
+{% do tmp_crumb.extend([(forum.name, forum|url), (name, topic|url)]) %}
 {% set BREADCRUMBS = tmp_crumb + BREADCRUMBS|d([]) %}
 
 {% set rendered_pagination = render_pagination(pagination) %}

inyoka/markup/base.py

@@ -558,12 +558,17 @@ def parse_font(self, stream):
         Returns a `Font` node.
         """
         stream.expect('font_begin')
-        face = stream.expect('font_face').value.strip()
+        face = stream.expect('font_face').value.strip().lower()
+        allowed_font_faces = ('serif', 'sans-serif', 'arial', 'courier')
+        if face not in allowed_font_faces:
+            face = None
+
         children = []
         while stream.current.type != 'font_end':
             children.append(self.parse_node(stream))
         stream.expect('font_end')
-        return nodes.Font([face], children)
+
+        return nodes.Font(face, children)
 
     def parse_mod(self, stream):
         """
@@ -716,7 +721,7 @@ def parse_external_link(self, stream):
 
     def parse_free_link(self, stream):
         """
-        Parses an free link.
+        Parses a free link.
 
         Returns a `Link` node.
         """
@@ -725,7 +730,7 @@ def parse_free_link(self, stream):
             urlsplit(target)
         except ValueError:
             return nodes.Text(target)
-        return nodes.Link(target, shorten=True)
+        return nodes.Link(target)
 
     def parse_ruler(self, stream):
         """

inyoka/markup/lexer.py

@@ -94,9 +94,10 @@ def __init__(self, regexp, token=None, enter=None, silententer=None,
     r'(?:mailto|telnet|s?news|sips?|skype|apt):)'
 )
 
+_control_characters = r'[\x00-\x08\x0B-\x0C\x0E-\x1F\x80-\x9F]'
+
 rules = {
     'everything': ruleset(
-        rule(r'[\x00-\x08\x0B-\x0C\x0E-\x1F]', None),  # ignore control character
         include('block'),
         include('inline'),
         include('links')
@@ -107,6 +108,7 @@ def __init__(self, regexp, token=None, enter=None, silententer=None,
     ),
     'block': ruleset(
         rule('(?m)^##.*?(\n|$)', None),
+        rule(_control_characters, None),  # ignore control character
         rule(r'(?m)^#\s*(.*?)\s*:\s*', bygroups('metadata_key'),
              enter='metadata'),
         rule(r'(?m)^={1,5}\s*', enter='headline'),
@@ -121,6 +123,7 @@ def __init__(self, regexp, token=None, enter=None, silententer=None,
     ),
     'inline': ruleset(
         rule('(?s)<!--.*?-->', None),
+        rule(_control_characters, None),  # ignore control character
         rule("'''", enter='strong'),
         rule("''", enter='emphasized'),
         rule('``', enter='escaped_code'),
@@ -286,6 +289,7 @@ def __init__(self, regexp, token=None, enter=None, silententer=None,
         include('function_call')
     ),
     'parser_data': ruleset(
+        rule(_control_characters, None),  # ignore control character
         rule(r'\}\}\}', leave=1)
     ),
     'pre_data': ruleset(

inyoka/markup/macros.py

@@ -249,10 +249,7 @@ def build_node(self, tree):
 
             # in all cases we need to add the current headline to the children
             # of recent stack element
-            ml = 42 - (headline.level - 1) * 2
-            text = len(headline.text) > ml and headline.text[:ml] + '...' or \
-                   headline.text
-            caption = [nodes.Text(text)]
+            caption = [nodes.Text(headline.text)]
             link = nodes.Link('#' + headline.id, caption)
             stack[-1].children.append(nodes.ListItem([link]))
 

inyoka/markup/nodes.py

@@ -22,7 +22,7 @@
 
 from django.apps import apps
 from django.conf import settings
-from django.utils.html import escape, smart_urlquote
+from django.utils.html import escape, format_html, smart_urlquote
 from django.utils.translation import gettext as _
 from django.utils.translation import gettext_lazy
 
@@ -571,20 +571,12 @@ def __init__(
         id=None,
         style=None,
         class_=None,
-        shorten=False,
     ):
         if not children:
-            if shorten and len(url) > 50:
-                children = [
-                    Span([Text(url[:36])], class_='longlink_show'),
-                    Span([Text(url[36:-14])], class_='longlink_collapse'),
-                    Span([Text(url[-14:])]),
-                ]
-            else:
-                text = url
-                if text.startswith('mailto:'):
-                    text = text[7:]
-                children = [Text(text)]
+            text = url
+            if text.startswith('mailto:'):
+                text = text[7:]
+            children = [Text(text)]
             if title is None:
                 title = url
         Element.__init__(self, children, id, style, class_)
@@ -764,11 +756,14 @@ def __init__(self, username, children=None, id=None, style=None, class_=None):
         self.username = username
 
     def prepare_html(self):
-        yield '<div class="%s">' % self.css_class
-        yield (
-            '<p><strong>%s <a class="crosslink user" href="%s">'
-            '%s</a>:</strong></p> '
-            % (self.msg, href('portal', 'user', self.username), self.username)
+        yield format_html(
+            '<div class="{}">'
+            '<p><strong>{} <a class="crosslink user" href="{}">'
+            '{}</a>:</strong></p> ',
+            self.css_class,
+            self.msg,
+            href('portal', 'user', self.username),
+            self.username
         )
         yield from Element.prepare_html(self)
         yield '</div>'
@@ -984,15 +979,12 @@ class Color(Element):
     of backwards compatibility (this time to phpBB).
     """
 
-    allowed_in_signatures = True
-
     def __init__(self, value, children=None, id=None, style=None, class_=None):
         Element.__init__(self, children, id, style, class_)
         self.value = value
 
     def prepare_html(self):
-        style = self.style and self.style + '; ' or ''
-        style += 'color: %s' % self.value
+        style = 'color: %s' % self.value
         yield build_html_tag('span', id=self.id, style=style, class_=self.class_)
         yield from Element.prepare_html(self)
         yield '</span>'
@@ -1009,8 +1001,7 @@ def __init__(self, size, children=None, id=None, style=None, class_=None):
         self.size = size
 
     def prepare_html(self):
-        style = self.style and self.style + '; ' or ''
-        style += 'font-size: %.2f%%' % self.size
+        style = 'font-size: %.2f%%' % self.size
         yield build_html_tag('span', id=self.id, style=style, class_=self.class_)
         yield from Element.prepare_html(self)
         yield '</span>'
@@ -1022,21 +1013,17 @@ class Font(Element):
     because of backwards compatibility.
     """
 
-    allowed_in_signatures = True
-
-    def __init__(self, faces, children=None, id=None, style=None, class_=None):
+    def __init__(self, face=None, children=None, id=None, style=None, class_=None):
         Element.__init__(self, children, id, style, class_)
-        self.faces = faces
+        self.face = face
 
     def prepare_html(self):
-        style = self.style and self.style + '; ' or ''
-        style += 'font-family: %s' % ', '.join(
-            x in ('serif', 'sans-serif', 'fantasy') and x or "'%s'" % x
-            for x in self.faces
-        )
-        yield build_html_tag('span', id=self.id, style=style, class_=self.class_)
+        if self.face:
+            style = 'font-family: %s' % self.face
+            yield build_html_tag('span', id=self.id, style=style, class_=self.class_)
         yield from Element.prepare_html(self)
-        yield '</span>'
+        if self.face:
+            yield '</span>'
 
 
 class DefinitionList(Element):

inyoka/markup/transformers.py

@@ -34,7 +34,7 @@ class Transformer:
     def transform(self, tree):
         """
         This is passed a tree that should be processed.  A class can modify
-        a tree in place, the return value has to be the tree then.  Otherwise
+        a tree in place, the return value has to be the tree then.  Otherwise,
         it's safe to return a new tree.
         """
         return tree
@@ -105,7 +105,7 @@ def transform(self, parent):
                 parent.children.append(paragraph)
             else:
                 for node in paragraph:
-                    if not node.is_text_node or node.text:
+                    if (not node.is_text_node or node.text) and not node.text == '\n':
                         parent.children.append(nodes.Paragraph(paragraph))
                         break
 
@@ -219,7 +219,7 @@ def transform(self, tree, nested=False):
 
 class FootnoteSupport(Transformer):
     """
-    Looks for footnote nodes, gives them an unique id and moves the
+    Looks for footnote nodes, gives them a unique id and moves the
     text to the bottom into a list.  Without this translator footnotes
     are just <small>ed and don't have an id.
     """

inyoka/portal/forms.py

@@ -31,7 +31,6 @@
 from django.db.models.functions import Concat
 from django.forms import HiddenInput, modelformset_factory
 from django.utils.html import format_html
-from django.utils.safestring import mark_safe
 from django.utils.translation import gettext as _
 from django.utils.translation import gettext_lazy
 from guardian.shortcuts import assign_perm, get_perms, remove_perm
@@ -209,10 +208,10 @@ def clean_email(self):
         exists = User.objects.filter(email__iexact=self.cleaned_data['email'])\
                              .exists()
         if exists:
-            raise forms.ValidationError(mark_safe(
+            raise forms.ValidationError(format_html(
                 _('The given email address is already in use. If you forgot '
-                  'your password, you can <a href="%(link)s">restore it</a>.')
-                % {'link': href('portal', 'lost_password')}))
+                  'your password, you can <a href="{link}">restore it</a>.'),
+                link=href('portal', 'lost_password')))
         return self.cleaned_data['email']
 
 

inyoka/static/style/main.less

@@ -557,12 +557,7 @@ span.highlight {
 }
 
 a {
-  .longlink_collapse {
-    display: none;
-  }
-  .longlink_show::after {
-    content: "…";
-  }
+  .break-word()
 }
 div.code, pre {
   overflow: auto;
@@ -936,7 +931,7 @@ form.search {
     flex-wrap: wrap;
   }
   > ul > li {
-     /* top and bottom padding on each li → if they wrap, there's spacing between each other */
+    /* top and bottom padding on each li → if they wrap, there's spacing between each other */
     padding: 1em 0;
   }
   a {