From 0d7fbbe105e7a62342938aa64db9b8f81f118c7e Mon Sep 17 00:00:00 2001 From: RogerHaase Date: Tue, 21 May 2024 14:06:14 -0700 Subject: [PATCH] macros with editing errors should not log tracebacks; fixes #1687 --- .../datastructures/backends/wiki_dicts.py | 2 +- src/moin/macros/Anchor.py | 18 ++++-- src/moin/macros/Date.py | 64 ++++++++++--------- src/moin/macros/DateTime.py | 17 ++++- src/moin/macros/FontAwesome.py | 7 +- src/moin/macros/GetVal.py | 21 +++--- src/moin/macros/Icon.py | 7 +- src/moin/macros/ItemList.py | 47 +++++++------- src/moin/macros/MailTo.py | 6 +- src/moin/macros/TitleIndex.py | 5 +- src/moin/macros/_base.py | 27 ++++++-- src/moin/macros/_tests/test_Anchor.py | 15 +++-- src/moin/macros/_tests/test_DateTime.py | 8 +-- src/moin/macros/_tests/test_GetVal.py | 6 +- src/moin/macros/_tests/test__base.py | 5 +- 15 files changed, 153 insertions(+), 102 deletions(-) diff --git a/src/moin/datastructures/backends/wiki_dicts.py b/src/moin/datastructures/backends/wiki_dicts.py index 6d13e07ad..5d7de325c 100644 --- a/src/moin/datastructures/backends/wiki_dicts.py +++ b/src/moin/datastructures/backends/wiki_dicts.py @@ -30,7 +30,7 @@ def _load_dict(self): wikidict = rev.meta.get(WIKIDICT, {}) return wikidict except KeyError: - flash(f'WikiDict "{dict_name}" has invalid syntax within metadata.') + flash(f'WikiDict "{dict_name}" does not exist or it has invalid syntax within metadata.') raise DictDoesNotExistError(dict_name) diff --git a/src/moin/macros/Anchor.py b/src/moin/macros/Anchor.py index bd61232f9..25e2455f2 100644 --- a/src/moin/macros/Anchor.py +++ b/src/moin/macros/Anchor.py @@ -2,18 +2,28 @@ # License: GNU GPL v2 (or any later version), see LICENSE.txt for details. """ -MoinMoin - Anchor Macro to put an anchor at the place where it is used. +MoinMoin - The Anchor Macro is used to create an anchor comprised of a span tag with +an id attribute. Per HTML5 the id must be at least 1 character with no space characters. """ - from moin.utils.tree import moin_page -from moin.macros._base import MacroInlineBase +from moin.macros._base import MacroInlineBase, fail_message +from moin.i18n import _ class Macro(MacroInlineBase): def macro(self, content, arguments, page_url, alternative): if not arguments: - raise ValueError("Anchor: you need to specify an anchor name.") + msg = _("Anchor macro failed - missing argument.") + return fail_message(msg, alternative) + + if len(arguments) > 1: + msg = _("Anchor macro failed - only 1 argument allowed.") + return fail_message(msg, alternative) anchor = arguments[0] + if " " in anchor: + msg = _("Anchor macro failed - space is not allowed in anchors.") + return fail_message(msg, alternative) + return moin_page.span(attrib={moin_page.id: anchor}) diff --git a/src/moin/macros/Date.py b/src/moin/macros/Date.py index 34a1bd6aa..fe5093a30 100644 --- a/src/moin/macros/Date.py +++ b/src/moin/macros/Date.py @@ -9,8 +9,9 @@ import time -from moin.macros._base import MacroInlineBase +from moin.macros._base import MacroInlineBase, fail_message from moin.utils import show_time +from moin.i18n import _ class MacroDateTimeBase(MacroInlineBase): @@ -19,10 +20,10 @@ def parse_time(self, args): Parse a time specification argument for usage by Date and DateTime macro. Not all ISO 8601 format variations are accepted as input. - :param args: float/int UNIX timestamp or ISO 8601 formatted date time: + :param args: float/int UNIX timestamp or null or ISO 8601 formatted date time: YYYY-MM-DDTHH:MM:SS (plus optional Z or z for UTC, or +/-HHMM) or YYYY-MM-DD HH:MM:SS (same as above but replacing T separator with " ") - :returns: UNIX timestamp (UTC) + :returns: UNIX timestamp (UTC) or raises one of AttributeError, OSError, AssertionError, ValueError, OverflowError """ if ( len(args) >= 19 @@ -32,41 +33,42 @@ def parse_time(self, args): and args[13] == ":" and args[16] == ":" ): - try: - year, month, day = int(args[0:4]), int(args[5:7]), int(args[8:10]) - hour, minute, second = int(args[11:13]), int(args[14:16]), int(args[17:19]) - tz = args[19:] # +HHMM, -HHMM or Z or nothing (then we assume Z) - tzoffset = 0 # we assume UTC no matter if there is a Z - if tz: - sign = tz[0] - if sign in "+-\u2212": # ascii plus, ascii hyphen-minus, unicode minus - tzh, tzm = int(tz[1:3]), int(tz[3:]) - tzoffset = (tzh * 60 + tzm) * 60 - if sign in "-\u2212": # ascii hyphen-minus, unicode minus - tzoffset = -tzoffset - tm = year, month, day, hour, minute, second, 0, 0, 0 - except ValueError as err: - raise ValueError(f"Bad timestamp {args!r}: {err}") + year, month, day = int(args[0:4]), int(args[5:7]), int(args[8:10]) + hour, minute, second = int(args[11:13]), int(args[14:16]), int(args[17:19]) + tz = args[19:] # +HHMM, -HHMM or Z or nothing (then we assume Z) + tzoffset = 0 # we assume UTC no matter if there is a Z + if tz: + sign = tz[0] + if sign in "+-\u2212": # ascii plus, ascii hyphen-minus, unicode minus + tzh, tzm = int(tz[1:3]), int(tz[3:]) + tzoffset = (tzh * 60 + tzm) * 60 + if sign in "-\u2212": # ascii hyphen-minus, unicode minus + tzoffset = -tzoffset + tm = year, month, day, hour, minute, second, 0, 0, 0 + # as mktime wants a localtime argument (but we only have UTC), # we adjust by our local timezone's offset - try: - tm = time.mktime(tm) - time.timezone - tzoffset - except (OverflowError, ValueError): - tm = 0 # incorrect, but we avoid an ugly backtrace + tm = time.mktime(tm) - time.timezone - tzoffset else: - # try raw seconds since epoch in UTC - try: - tm = float(args) - except ValueError as err: - raise ValueError(f"Bad timestamp {args!r}: {err}") + tm = float(args) return tm class Macro(MacroDateTimeBase): + """ + Return a date formatted per user settings or an error message if input is invalid. + """ + def macro(self, content, arguments, page_url, alternative): + if arguments is None: - tm = None + tm = None # show today's date else: - stamp = arguments[0] - tm = self.parse_time(stamp) - return show_time.format_date(tm) + tm = arguments[0] + try: + if tm: + tm = self.parse_time(tm) + return show_time.format_date(tm) + except (AttributeError, OSError, AssertionError, ValueError, OverflowError): + err_msg = _("Invalid input parameter: null, float, int, or ISO 8601 formats are accepted.") + return fail_message(err_msg, alternative) diff --git a/src/moin/macros/DateTime.py b/src/moin/macros/DateTime.py index 73ec13a3d..0bdde12b2 100644 --- a/src/moin/macros/DateTime.py +++ b/src/moin/macros/DateTime.py @@ -6,15 +6,26 @@ adapted to the TZ settings of the user viewing the content. """ +from moin.macros._base import fail_message from moin.macros.Date import MacroDateTimeBase from moin.utils import show_time +from moin.i18n import _ class Macro(MacroDateTimeBase): + """ + Return a date and time formatted per user settings or an error message if input is invalid. + """ + def macro(self, content, arguments, page_url, alternative): if arguments is None: tm = None else: - stamp = arguments[0] - tm = self.parse_time(stamp) - return show_time.format_date_time(tm) + tm = arguments[0] + try: + if tm: + tm = self.parse_time(tm) + return show_time.format_date_time(tm) + except (AttributeError, OSError, AssertionError, ValueError, OverflowError): + err_msg = _("Invalid input parameter: null, float, int, or ISO 8601 formats are accepted.") + return fail_message(err_msg, alternative) diff --git a/src/moin/macros/FontAwesome.py b/src/moin/macros/FontAwesome.py index 5d7d63e77..71a752505 100644 --- a/src/moin/macros/FontAwesome.py +++ b/src/moin/macros/FontAwesome.py @@ -20,14 +20,17 @@ from moin.utils.tree import moin_page -from moin.macros._base import MacroInlineBase +from moin.macros._base import MacroInlineBase, fail_message +from moin.i18n import _ class Macro(MacroInlineBase): def macro(self, content, arguments, page_url, alternative): args = arguments[0] if arguments else "" if not args: - raise ValueError("Missing font name") + err_msg = _("Missing font name, syntax is <>") + return fail_message(err_msg, alternative) + args = args.split(",") fonts = args[0].split() color = args[1].strip() if len(args) > 1 else "" diff --git a/src/moin/macros/GetVal.py b/src/moin/macros/GetVal.py index 86563ec5f..4588e176e 100644 --- a/src/moin/macros/GetVal.py +++ b/src/moin/macros/GetVal.py @@ -8,7 +8,7 @@ from flask import g as flaskg -from moin.macros._base import MacroInlineBase +from moin.macros._base import MacroInlineBase, fail_message from moin.datastructures.backends import DictDoesNotExistError from moin.i18n import _ @@ -21,18 +21,23 @@ def macro(self, content, arguments, page_url, alternative): item_name = args[0].strip() key = args[1].strip() except (IndexError, AssertionError): - raise ValueError(_("GetVal: invalid parameters, try <>")) + err_msg = _("Invalid parameters, try <>") + return fail_message(err_msg, alternative) + if not flaskg.user.may.read(str(item_name)): - raise ValueError(_("GetVal: permission to read denied: ") + item_name) + err_msg = _("Permission to read was denied: {item_name}").format(item_name=item_name) + return fail_message(err_msg, alternative) + try: d = flaskg.dicts[item_name] except DictDoesNotExistError: - raise ValueError(_("GetVal: dict not found: ") + item_name) + err_msg = _("WikiDict not found: {item_name}").format(item_name=item_name) + return fail_message(err_msg, alternative) + result = d.get(key, "") if not result: - raise ValueError( - _("GetVal macro is invalid, {item_name} missing key: {key_name}").format( - item_name=item_name, key_name=key - ) + err_msg = _("Macro is invalid, {item_name} is missing key: {key_name}").format( + item_name=item_name, key_name=key ) + return fail_message(err_msg, alternative) return result diff --git a/src/moin/macros/Icon.py b/src/moin/macros/Icon.py index d77dedc0d..9c04e4c0e 100644 --- a/src/moin/macros/Icon.py +++ b/src/moin/macros/Icon.py @@ -14,7 +14,7 @@ from flask import url_for from moin.utils.tree import html -from moin.macros._base import MacroInlineBase +from moin.macros._base import MacroInlineBase, fail_message from moin.i18n import _ @@ -22,8 +22,9 @@ class Macro(MacroInlineBase): def macro(self, content, arguments, page_url, alternative): icon = arguments[0] if arguments else "" if not icon: - raise ValueError("Missing icon name") + msg = _("Icon macro failed due to missing icon name.") + return fail_message(msg, alternative) src = url_for("static", filename="img/icons/" + icon) - reason = _("Icon not rendered, verify name is valid") + reason = _("Icon not rendered, invalid name") alt = f"<> - {reason}" return html.img(attrib={html.src: src, html.alt: alt, html.class_: "moin-icon-macro"}) diff --git a/src/moin/macros/ItemList.py b/src/moin/macros/ItemList.py index 9809483b5..410133269 100644 --- a/src/moin/macros/ItemList.py +++ b/src/moin/macros/ItemList.py @@ -65,9 +65,8 @@ from flask import request from flask import g as flaskg from moin.i18n import _ -from moin.utils.tree import moin_page from moin.utils.interwiki import split_fqname -from moin.macros._base import MacroPageLinkListBase, get_item_names +from moin.macros._base import MacroPageLinkListBase, get_item_names, fail_message class Macro(MacroPageLinkListBase): @@ -89,15 +88,16 @@ def macro(self, content, arguments, page_url, alternative): try: key, val = (x.strip() for x in arg.split("=")) except ValueError: - raise ValueError( - _( - 'ItemList macro: Argument "{arg}" does not follow = format ' - "(arguments, if more than one, must be comma-separated)." - ).format(arg=arg) - ) + err_msg = _( + 'ItemList macro: Argument "{arg}" does not follow = format ' + "(arguments, if more than one, must be comma-separated)." + ).format(arg=arg) + return fail_message(err_msg, alternative) if len(val) < 2 or (val[0] != "'" and val[0] != '"') and val[-1] != val[0]: - raise ValueError(_("ItemList macro: The key's value must be bracketed by matching quotes.")) + err_msg = _("The key's value must be bracketed by matching quotes.") + return fail_message(err_msg, alternative) + val = val[1:-1] # strip out the doublequote characters if key == "item": @@ -112,15 +112,16 @@ def macro(self, content, arguments, page_url, alternative): elif val == "True": ordered = True else: - raise ValueError( - _('ItemList macro: The value must be "True" or "False". (got "{val}")').format(val=val) - ) + err_msg = _('The value must be "True" or "False". (got "{val}")').format(val=val) + return fail_message(err_msg, alternative) + elif key == "display": display = val # let 'create_pagelink_list' throw an exception if needed elif key == "skiptag": skiptag = val else: - raise KeyError(_('ItemList macro: Unrecognized key "{key}".').format(key=key)) + err_msg = _('Unrecognized key "{key}".').format(key=key) + return fail_message(err_msg, alternative) # use curr item if not specified if item is None: @@ -131,11 +132,8 @@ def macro(self, content, arguments, page_url, alternative): # verify item exists and current user has read permission if item != "": if not flaskg.storage.get_item(**(split_fqname(item).query)): - message = _("Item does not exist or read access blocked by ACLs: {0}").format(item) - admonition = moin_page.div( - attrib={moin_page.class_: "important"}, children=[moin_page.p(children=[message])] - ) - return admonition + err_msg = _("Item does not exist or read access blocked by ACLs: {0}").format(item) + return fail_message(err_msg, alternative) # process subitems children = get_item_names(item, startswith=startswith, skiptag=skiptag) @@ -143,16 +141,15 @@ def macro(self, content, arguments, page_url, alternative): try: regex_re = re.compile(regex, re.IGNORECASE) except re.error as err: - raise ValueError(_("ItemList macro: Error in regex {0!r}: {1}").format(regex, err)) + err_msg = _("Error in regex {0!r}: {1}").format(regex, err) + return fail_message(err_msg, alternative) + newlist = [] for child in children: if regex_re.search(child.fullname): newlist.append(child) children = newlist if not children: - empty_list = moin_page.list(attrib={moin_page.item_label_generate: ordered and "ordered" or "unordered"}) - item_body = moin_page.list_item_body(children=[_("")]) - item = moin_page.list_item(children=[item_body]) - empty_list.append(item) - return empty_list - return self.create_pagelink_list(children, ordered, display) + return fail_message(_("No matching items were found"), alternative, severity="attention") + + return self.create_pagelink_list(children, alternative, ordered, display) diff --git a/src/moin/macros/MailTo.py b/src/moin/macros/MailTo.py index fa5960019..7503155d1 100644 --- a/src/moin/macros/MailTo.py +++ b/src/moin/macros/MailTo.py @@ -10,7 +10,7 @@ from flask import g as flaskg from moin.utils.tree import moin_page, xlink -from moin.macros._base import MacroInlineBase +from moin.macros._base import MacroInlineBase, fail_message from moin.mail.sendmail import decodeSpamSafeEmail from moin.i18n import _ @@ -28,8 +28,8 @@ def macro(self, content, arguments, page_url, alternative): email = arguments[0] assert len(email) >= 5 except (AttributeError, AssertionError): - raise ValueError(_("MailTo: invalid format, try: <>")) - + err_msg = _("Invalid format, try: <>") + return fail_message(err_msg, alternative) try: text = arguments[1] except IndexError: diff --git a/src/moin/macros/TitleIndex.py b/src/moin/macros/TitleIndex.py index 380819bba..864392dd8 100644 --- a/src/moin/macros/TitleIndex.py +++ b/src/moin/macros/TitleIndex.py @@ -11,7 +11,7 @@ <> """ -from moin.macros._base import MacroMultiLinkListBase, get_item_names +from moin.macros._base import MacroMultiLinkListBase, get_item_names, fail_message from moin.i18n import _ from moin.utils.tree import moin_page from moin.utils.interwiki import split_fqname @@ -23,7 +23,8 @@ def macro(self, content, arguments, page_url, alternative): namespace = split_fqname(str(page_url.path)).namespace if arguments: - raise ValueError(_("TitleList macro does not have any arguments.")) + err_msg = _("TitleList macro does not support any arguments.") + return fail_message(err_msg, alternative) children = get_item_names(namespace) if not children: diff --git a/src/moin/macros/_base.py b/src/moin/macros/_base.py index 94984cb77..248a132c8 100644 --- a/src/moin/macros/_base.py +++ b/src/moin/macros/_base.py @@ -85,13 +85,24 @@ def extract_h1(item_name): return _("No heading found in item: {item_name}").format(item_name=item_name) -def fail_message(msg, severity="error"): +def fail_message(msg, alternative, severity="error"): """ Return an error message in admonition-like format. - Severity may be: attention, caution, danger, error, hint, important, note, or tip. + :param msg: error message + :param alternative: full text of macro as passed in MacroBase __call__ + :param Severity: attention, caution, danger, error, hint, important, note, or tip + :returns: formatted text of macro, error message """ - return html.div(attrib={html.class_: f"{severity} moin-nowiki"}, children=msg) + if severity not in "attention caution danger error hint important note tip".split(): + severity = "error" + msg = ( + _("Invalid severity, must be one of: ") + "attention, caution, danger, error, hint, important, note, or tip" + ) + + altern = html.p(children=[html.strong(children=[alternative])]) + message = html.p(children=[msg]) + return html.div(attrib={html.class_: f"{severity} moin-nowiki"}, children=[altern, message]) class MacroBase: @@ -119,7 +130,8 @@ class MacroBlockBase(MacroBase): def __call__(self, content, arguments, page_url, alternative, context_block): if context_block: return self.macro(content, arguments, page_url, alternative) - raise ValueError(_("Block macros cannot be used inline")) + err_msg = _("Block macros cannot be used inline") + return fail_message(err_msg, alternative) def macro(self, content, arguments, page_url, alternative): raise NotImplementedError @@ -153,13 +165,15 @@ def __call__(self, content, arguments, page_url, alternative, context_block): class MacroPageLinkListBase(MacroBlockBase): - def create_pagelink_list(self, pagenames, ordered=False, display="FullPath"): + def create_pagelink_list(self, pagenames, alternative, ordered=False, display="FullPath"): """Creates an ET with a list of pagelinks from a list of pagenames. Parameters: pagenames: a list of pages, each being like a flask request.path[1:] + alternative: full text of macro call + ordered: Should the list be ordered or unordered list (
    or
      )? Options: @@ -205,7 +219,8 @@ def create_pagelink_list(self, pagenames, ordered=False, display="FullPath"): elif display == "ItemTitle": linkname = extract_h1(pagename.fullname) else: - raise KeyError(_('Unrecognized display value "{display}".').format(display=display)) + err_msg = _('Unrecognized display value "{display}".').format(display=display) + return fail_message(err_msg, alternative) pagelink = moin_page.a(attrib={xlink.href: url}, children=[linkname]) item_body = moin_page.list_item_body(children=[pagelink]) diff --git a/src/moin/macros/_tests/test_Anchor.py b/src/moin/macros/_tests/test_Anchor.py index 4dded9ac2..56f1d2353 100644 --- a/src/moin/macros/_tests/test_Anchor.py +++ b/src/moin/macros/_tests/test_Anchor.py @@ -5,18 +5,21 @@ Test for macros.Anchor """ -import pytest - from moin.macros.Anchor import Macro +from moin.utils.tree import moin_page def test_Macro(): macro_obj = Macro() - with pytest.raises(ValueError): - macro_obj.macro("content", None, "page_url", "alternative") - - arguments = [("test_argument1", "test_argument2"), "test_argument3"] + arguments = ["my_anchor"] result = macro_obj.macro("content", arguments, "page_url", "alternative") test_anchor = list(result.attrib.values()) # test_anchor[0] since it returns a list assert test_anchor[0] == arguments[0] + assert result.attrib[moin_page.id] == "my_anchor" + + arguments = [] + result = macro_obj.macro("content", arguments, "page_url", "alternative") + test_anchor = list(result.attrib.values()) + # expecting error message with class of 'error nowiki' + assert "error" in test_anchor[0] diff --git a/src/moin/macros/_tests/test_DateTime.py b/src/moin/macros/_tests/test_DateTime.py index 3c75ba4da..f0bbfe0b1 100644 --- a/src/moin/macros/_tests/test_DateTime.py +++ b/src/moin/macros/_tests/test_DateTime.py @@ -10,8 +10,6 @@ from flask import g as flaskg -import pytest - from moin.macros.DateTime import Macro from moin.utils import utcfromtimestamp from moin.utils.show_time import format_date_time @@ -40,8 +38,10 @@ def test_Macro(): assert result == expected arguments = ["incorrect_argument"] - with pytest.raises(ValueError): - macro_obj.macro("content", arguments, "page_url", "alternative") + result = macro_obj.macro("content", arguments, "page_url", "alternative") + attr = list(result.attrib.values()) + # expecting error message with class of 'error nowiki' + assert "error" in attr[0] # the following are different ways to specify the same moment expected = "2019-10-07 18:30:00z" diff --git a/src/moin/macros/_tests/test_GetVal.py b/src/moin/macros/_tests/test_GetVal.py index 3c4f5f7c9..cd32362ea 100644 --- a/src/moin/macros/_tests/test_GetVal.py +++ b/src/moin/macros/_tests/test_GetVal.py @@ -25,8 +25,10 @@ def test_dict(self): def test_Macro(self, test_dict): macro_obj = Macro() arguments = [test_dict] - with pytest.raises(ValueError): - macro_obj.macro("content", arguments, "page_url", "alternative") + result = macro_obj.macro("content", arguments, "page_url", "alternative") + attr = list(result.attrib.values()) + # expecting error message with class of 'error nowiki' + assert "error" in attr[0] if not flaskg.user.may.read(arguments[0]): with pytest.raises(ValueError): diff --git a/src/moin/macros/_tests/test__base.py b/src/moin/macros/_tests/test__base.py index 7421d2709..1b28fff84 100644 --- a/src/moin/macros/_tests/test__base.py +++ b/src/moin/macros/_tests/test__base.py @@ -8,6 +8,7 @@ import pytest from moin.macros._base import MacroBase, MacroBlockBase, MacroInlineBase, MacroInlineOnlyBase, MacroPageLinkListBase +from moin.utils.tree import html class TestMacroBase: @@ -27,8 +28,8 @@ class Test_MacroBlockBase(MacroBlockBase): """inherited class from MacroBlockBase""" macroblockbase_obj = Test_MacroBlockBase() - with pytest.raises(ValueError): - macroblockbase_obj.__call__("content", "arguments", "page_url", "alternative", context_block=False) + result = macroblockbase_obj.__call__("content", "arguments", "page_url", "alternative", context_block=False) + assert "error" in result.attrib[html.class_] with pytest.raises(NotImplementedError): macroblockbase_obj.__call__("content", "arguments", "page_url", "alternative", "context_block")