You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2025/08/21 20:02:42 UTC
(allura) branch db/fix_markdown_cleaning updated: more fixes
This is an automated email from the ASF dual-hosted git repository.
brondsem pushed a commit to branch db/fix_markdown_cleaning
in repository https://gitbox.apache.org/repos/asf/allura.git
The following commit(s) were added to refs/heads/db/fix_markdown_cleaning by this push:
new 47e84844e more fixes
47e84844e is described below
commit 47e84844e37f0b20fb937ddd341f9c3eb23a546d
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Thu Aug 21 16:01:44 2025 -0400
more fixes
---
Allura/allura/lib/macro.py | 2 +-
Allura/allura/lib/utils.py | 27 ++++++++++++-------
Allura/allura/public/nf/js/allura-base.js | 2 +-
.../templates/widgets/project_screenshots.html | 6 ++---
Allura/allura/tests/functional/test_newforge.py | 11 +++++---
Allura/allura/tests/test_globals.py | 14 ++++++++--
Allura/development.ini | 5 ++++
.../forgeimporters/github/tests/test_wiki.py | 31 +---------------------
8 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/Allura/allura/lib/macro.py b/Allura/allura/lib/macro.py
index e073c35b6..61dbea85a 100644
--- a/Allura/allura/lib/macro.py
+++ b/Allura/allura/lib/macro.py
@@ -288,7 +288,7 @@ def get_projects_for_macro(
for p in M.Project.query.find(q):
if h.has_access(p, 'read'):
total = total + 1
- response = '<p class="macro_projects_total">%s Projects</p>%s' % \
+ response = '<p>%s Projects</p>%s' % \
(total, response)
return response
diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py
index 75ead7cd3..b9625114f 100644
--- a/Allura/allura/lib/utils.py
+++ b/Allura/allura/lib/utils.py
@@ -564,20 +564,27 @@ def __init__(self, *args, **kwargs):
self.valid_class_values = {
'markdown_content', # our wrapper class
# standard extensions:
- 'codehilite', 'footnote', 'checklist', 'toc', 'footnote-ref', 'footnote-backref', 'codehilitetable',
- 'user-mention', 'macro_projects_total', 'proj_icon', 'list', 'card', 'feature', 'box', 'notch', 'desc',
- 'strikethrough',
- 'alink', 'notfound', # ForgeLinkPattern
- # our macros:
+ 'codehilite', 'footnote', 'checklist', 'toc', 'footnote-ref', 'footnote-backref',
+ # macros:
+ 'user-mention', 'proj_icon', 'list', 'card', 'feature', 'box', 'notch', 'desc',
'neighborhood_feed_entry', 'md-users-list', 'md-users-list-more',
- # codehilite classes:
- 'p', 'n', 'hll', 'c', 'k', 'o', 'cm', 'cp', 'c1', 'cs', 'gd', 'ge', 'gr', 'gh', 'gi', 'go', 'gp', 'gs', 'gu', 'gt', 'kc', 'kd', 'kn', 'kp', 'kr', 'kt', 'm', 's', 'na', 'nb', 'nc', 'no', 'nd', 'ni', 'ne', 'nf', 'nl', 'nn', 'nt', 'nv', 'ow', 'w', 'mf', 'mh', 'mi', 'mo', 'sb', 'sc', 'sd', 's2', 'se', 'sh', 'si', 'sx', 'sr', 's1', 'ss', 'bp', 'vc', 'vg', 'vi', 'il', 'code_block', 'lineno',
- }
- self.valid_partial_class_prefixes = tuple(aslist(tg.config.get('markdown_valid_partial_class_prefixes', None)))
+ # ForgeLinkPattern:
+ 'strikethrough', 'alink', 'notfound',
+ # codehilite classes, and our extra related classes:
+ 'p', 'n', 'hll', 'c', 'k', 'o', 'cm', 'cp', 'c1', 'cs', 'gd', 'ge', 'gr', 'gh', 'gi', 'go', 'gp', 'gs',
+ 'gu', 'gt', 'kc', 'kd', 'kn', 'kp', 'kr', 'kt', 'm', 's', 'na', 'nb', 'nc', 'no', 'nd', 'ni', 'ne', 'nf',
+ 'nl', 'nn', 'nt', 'nv', 'ow', 'w', 'mf', 'mh', 'mi', 'mo', 'sb', 'sc', 'sd', 's2', 'se', 'sh', 'si', 'sx',
+ 'sr', 's1', 'ss', 'bp', 'vc', 'vg', 'vi', 'il', 'code_block', 'lineno', 'codehilitetable', 'linenodiv',
+ 'linenos', 'ch', 'code', 'normal', 'err',
+ # helpful classes:
+ 'error', 'notice', 'success', 'ok', 'info',
+ 'fa',
+ } | set(aslist(tg.config.get('safe_html.classes', [])))
+ self.valid_partial_class_prefixes = tuple(['fa-'] + aslist(tg.config.get('safe_html.class_prefixes', None)))
self.valid_id_prefixes = {
'h-', # see toc_slugify_with_prefix
'fn:', 'fnref:', # from footnotes extension
- }
+ } | set(aslist(tg.config.get('safe_html.id_prefixes', [])))
self._prev_token_was_ok_iframe = False
def sanitize_token(self, token):
diff --git a/Allura/allura/public/nf/js/allura-base.js b/Allura/allura/public/nf/js/allura-base.js
index c6db0ce0a..54aabdf56 100644
--- a/Allura/allura/public/nf/js/allura-base.js
+++ b/Allura/allura/public/nf/js/allura-base.js
@@ -245,7 +245,7 @@ $(function(){
if (document.querySelector(new_hash)) {
window.location.hash = new_hash;
} else {
- new_hash = window.location.hash.replace('#h-', '#user-content-');
+ new_hash = window.location.hash.replace('#', '#user-content-');
if (document.querySelector(new_hash)) {
window.location.hash = new_hash;
}
diff --git a/Allura/allura/templates/widgets/project_screenshots.html b/Allura/allura/templates/widgets/project_screenshots.html
index 5aa5059d2..278d183b9 100644
--- a/Allura/allura/templates/widgets/project_screenshots.html
+++ b/Allura/allura/templates/widgets/project_screenshots.html
@@ -23,9 +23,9 @@
{% endif %}
<div class="{% if draggable %}sortable{% endif %}">
{% for ss in screenshots %}
- <div data-ss-id="{{ ss._id }}" class="screenshot">
- <div class="image">
- <a class="lightbox" href="{{project.url()}}screenshot/{{h.urlquote(ss.filename)}}" ><img src="{{project.url()}}screenshot/{{h.urlquote(ss.filename)}}/thumb" alt="Screenshot thumbnail"/></a>
+ <div data-ss-id="{{ ss._id }}" {% if edit %}class="screenshot"{% endif %}>
+ <div {% if edit %}class="image"{% endif %}>
+ <a {% if edit %}class="lightbox"{% endif %} href="{{project.url()}}screenshot/{{h.urlquote(ss.filename)}}" ><img src="{{project.url()}}screenshot/{{h.urlquote(ss.filename)}}/thumb" alt="Screenshot thumbnail"/></a>
{% if not edit %}
<br>
{{ss.caption}}
diff --git a/Allura/allura/tests/functional/test_newforge.py b/Allura/allura/tests/functional/test_newforge.py
index 92b4cfe01..69767118c 100644
--- a/Allura/allura/tests/functional/test_newforge.py
+++ b/Allura/allura/tests/functional/test_newforge.py
@@ -14,10 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
-
+import logging
from urllib.parse import quote
+from testfixtures import LogCapture
+
from allura.tests import TestController
from allura.tests import decorators as td
from allura import model as M
@@ -43,10 +44,14 @@ def test_markdown_to_html(self):
status=400)
def test_markdown_syntax(self):
- r = self.app.get('/nf/markdown_syntax')
+ with LogCapture(level=logging.INFO) as logs:
+ r = self.app.get('/nf/markdown_syntax')
r.mustcontain('Markdown Syntax')
r.mustcontain('href="http://someurl"')
+ markdown_invalid_classes = [log[2] for log in logs if 'invalid class' in log[2]]
+ assert not markdown_invalid_classes
+
def test_markdown_syntax_dialog(self):
r = self.app.get('/nf/markdown_syntax_dialog')
r.mustcontain('<h1>Markdown Syntax Guide</h1>')
diff --git a/Allura/allura/tests/test_globals.py b/Allura/allura/tests/test_globals.py
index 76f8e2ecf..7cc585a6f 100644
--- a/Allura/allura/tests/test_globals.py
+++ b/Allura/allura/tests/test_globals.py
@@ -188,10 +188,10 @@ def test_macro_projects(self):
assert 'alt="Test Project Logo"' in r, r
assert 'alt="A Subproject Logo"' in r, r
r = g.markdown_wiki.convert('[[projects show_total=True sort=random]]')
- assert '<p class="macro_projects_total">3 Projects' in r, r
+ assert '<p>3 Projects' in r, r
r = g.markdown_wiki.convert(
'[[projects show_total=True private=True sort=random]]')
- assert '<p class="macro_projects_total">1 Projects' in r, r
+ assert '<p>1 Projects' in r, r
assert 'alt="Test 2 Logo"' in r, r
assert 'alt="Test Project Logo"' not in r, r
assert 'alt="A Subproject Logo"' not in r, r
@@ -590,6 +590,16 @@ def test_markdown_invalid_script_in_link2(self):
'''href='http://"><img%20src=x%20onerror=alert(document.cookie)>' '''
'rel="nofollow">xss</a></p></div>' == r)
+ def test_markdown_invalid_classes_ids(self):
+ r = g.markdown.convert("# Test message")
+ assert r == '<div class="markdown_content"><h1 id="h-test-message">Test message</h1></div>'
+
+ r = g.markdown.convert('<p class="markdown-class notice" id="markdown-id">This is a test</p>')
+ assert r == '<div class="markdown_content"><p class="notice" id="user-content-markdown-id">This is a test</p>\n</div>'
+
+ r = g.markdown.convert('<i class="fa fa-cog">gear icon</i>')
+ assert r == '<div class="markdown_content"><p><i class="fa fa-cog">gear icon</i></p></div>'
+
def test_markdown_extremely_slow(self):
# regex-as-re-globally package mitigates this being slow; also see `NOBRACKET` comments
g.markdown.convert(inspect.cleandoc('''bonjour, voila ce que j'obtient en voulant ajouter un utilisateur a un groupe de sécurite, que ce soit sur un groupe pre-existant, ou sur un groupe crée.
diff --git a/Allura/development.ini b/Allura/development.ini
index a29b4027b..2405801b8 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -662,6 +662,11 @@ markdown_render_max_length = 200000
; Don't add rel=nofollow to these domains when generating links from Markdown content
;nofollow_exempt_domains =
+; when sanitizing HTML generated from markdown and other sources, allow these classes & ids, in addition to the built-in defaults:
+;safe_html.classes =
+;safe_html.class_prefixes =
+;safe_html.id_prefixes =
+
; By default project admins can soft-delete their projects.
; A soft-deleted project will still be in the database and visible to admins, but not to others.
allow_project_delete = true
diff --git a/ForgeImporters/forgeimporters/github/tests/test_wiki.py b/ForgeImporters/forgeimporters/github/tests/test_wiki.py
index e85eb5f7a..a137a8e0d 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_wiki.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_wiki.py
@@ -28,42 +28,13 @@
from forgeimporters.github.wiki import GitHubWikiImporter
from forgeimporters.github.utils import GitHubMarkdownConverter
from forgeimporters.github import GitHubOAuthMixin
-from forgewiki import model as WM
-from alluratest.controller import setup_basic_test, setup_unit_test
-from allura.tests import decorators as td
-from allura.lib import helpers as h
-from tg import tmpl_context as c, app_globals as g
-
-from Allura.allura.tests.model.test_artifact import Checkmessage
+from alluratest.controller import setup_basic_test
# important to be distinct from 'test' which ForgeWiki uses, so that the
# tests can run in parallel and not clobber each other
test_project_with_wiki = 'test2'
with_wiki = with_tool(test_project_with_wiki, 'wiki', 'w', 'wiki')
-class TestWikiPage:
-
- def setup_method(self):
- setup_basic_test()
- setup_unit_test()
- self.setup_with_tools()
-
- @td.with_wiki
- def setup_with_tools(self):
- h.set_context('test', 'wiki', neighborhood='Projects')
- Checkmessage.query.remove({})
- WM.Page.query.remove({})
- WM.PageHistory.query.remove({})
- M.Shortlink.query.remove({})
- c.user = M.User.query.get(username='test-admin')
- Checkmessage.project = c.project
- Checkmessage.app_config = c.app.config
-
- def test_markdown_excluded_attributes(self):
- pg = WM.Page(title='TestPage1')
- pg.text = """# Test message <p class="markdown-class" id="markdown-id">This is a test</p>"""
- assert "markdown-class" not in pg.html_text
- assert "markdown-id" not in pg.html_text
class TestGitHubWikiImporter: