diff --git a/src/moin/_tests/test_wikiutil.py b/src/moin/_tests/test_wikiutil.py index 732971e50..a121e4124 100644 --- a/src/moin/_tests/test_wikiutil.py +++ b/src/moin/_tests/test_wikiutil.py @@ -6,13 +6,15 @@ MoinMoin - moin.wikiutil Tests """ +from __future__ import annotations import pytest from flask import current_app as app - from moin.constants.chartypes import CHARS_SPACES from moin import wikiutil +from moin.wikiutil import WikiLinkAnalyzer, WikiLinkInfo +from typing import cast class TestCleanInput: @@ -253,4 +255,25 @@ def testfile_headers(): assert result == expected +@pytest.mark.parametrize( + "url,expected", + [ + # internal item links + ("users/roland", WikiLinkInfo(True, "frontend.show_item", "users/roland")), + ("+index/all", WikiLinkInfo(True, "frontend.index", "all", True)), + ("+history/users/roland", WikiLinkInfo(True, "frontend.history", "users/roland")), + # internal global link (not linking to a wiki item) + ("all", WikiLinkInfo(True, "frontend.global_views", None, True)), + # link without matching moin route + ("+invalid/help", WikiLinkInfo(False)), + # external link + ("http://google.com/hello", WikiLinkInfo(False)), + ], +) +def test_classify_link(url, expected): + link_analyzer = cast(WikiLinkAnalyzer, app.link_analyzer) + result = link_analyzer(url) + assert result == expected + + coverage_modules = ["moin.wikiutil"] diff --git a/src/moin/app.py b/src/moin/app.py index e65660b19..b7615ed46 100644 --- a/src/moin/app.py +++ b/src/moin/app.py @@ -32,10 +32,11 @@ from moin.utils import monkeypatch # noqa from moin.utils.clock import Clock from moin import auth, user, config -from moin.constants.misc import ANON, VALID_ITEMLINK_VIEWS +from moin.constants.misc import ANON from moin.i18n import i18n_init from moin.themes import setup_jinja_env, themed_error from moin.storage.middleware import protecting, indexing, routing +from moin.wikiutil import WikiLinkAnalyzer from moin import log @@ -168,7 +169,9 @@ class ItemNameConverter(PathConverter): app.register_blueprint(serve, url_prefix="/+serve") - app.view_endpoints = get_endpoints(app) + # create wiki link analyzer after having registered all routes + app.link_analyzer = WikiLinkAnalyzer(app) + clock.stop("create_app register") clock.start("create_app flask-cache") # 'SimpleCache' caching uses a dict and is not thread safe according to the docs. @@ -199,16 +202,6 @@ class ItemNameConverter(PathConverter): return app -def get_endpoints(app): - """Get dict with views and related endpoints allowed as itemlink""" - view_endpoints = {} - for rule in app.url_map.iter_rules(): - view = rule.rule.split("/")[1] - if view in VALID_ITEMLINK_VIEWS and rule.rule == f"/{view}/": - view_endpoints[view] = rule.endpoint - return view_endpoints - - def destroy_app(app): deinit_backends(app) diff --git a/src/moin/constants/misc.py b/src/moin/constants/misc.py index 3a498b31e..04710909d 100644 --- a/src/moin/constants/misc.py +++ b/src/moin/constants/misc.py @@ -71,9 +71,6 @@ LOCKED = 1 # true, current user has obtained or renewed lock LOCK = "lock" -# Valid views allowed for itemlinks -VALID_ITEMLINK_VIEWS = ["+meta", "+history", "+download", "+highlight", "+slideshow"] - # Transient attribute added/removed to/from flask session. Used when a User Settings # form creates a flash message but then redirects the page making the flash message a # very short flash message. diff --git a/src/moin/converters/link.py b/src/moin/converters/link.py index 49673b2a7..09fbd61b1 100644 --- a/src/moin/converters/link.py +++ b/src/moin/converters/link.py @@ -16,7 +16,6 @@ from emeraldtree.ElementTree import Element -from moin.constants.misc import VALID_ITEMLINK_VIEWS from moin.utils.interwiki import is_known_wiki, url_for_item from moin.utils.iri import Iri, IriPath from moin.utils.mime import type_moin_document @@ -189,27 +188,28 @@ def handle_wiki_links(self, elem: Element, input: Iri, to_tag=ConverterBase._tag elem.set(to_tag, link) def handle_wikilocal_links(self, elem: Element, input: Iri, page: Iri | None, to_tag=ConverterBase._tag_xlink_href): - view_name = "" + endpoint = "frontend.show_item" if input.path: item_name = str(input.path) - # Remove view from item_name before searching - if item_name.startswith("+"): - view_name = item_name.split("/")[0] - if view_name in VALID_ITEMLINK_VIEWS: - item_name = item_name.split(f"{view_name}/")[1] - if page: - # this can be a relative path, make it absolute: - item_name = str(self.absolute_path(Iri(path=item_name).path, page.path)) - if not flaskg.storage.has_item(item_name): - # XXX these index accesses slow down the link converter quite a bit + info = app.link_analyzer(item_name) + if not info.is_valid: elem.set(moin_page.class_, "moin-nonexistent") + elif not info.is_global and info.item_name: + endpoint = info.endpoint + if endpoint == "frontend.show_item": + # item_name can be a relative path, make it absolute: + if page: + item_name = str(self.absolute_path(IriPath(item_name), page.path)) + else: + item_name = info.item_name + if not flaskg.storage.has_item(item_name): + # XXX these index accesses slow down the link converter quite a bit + elem.set(moin_page.class_, "moin-nonexistent") else: + # link to current item item_name = str(page.path[1:]) if page else "" - endpoint, rev, query = self._get_do_rev(input.query) - if view_name in app.view_endpoints.keys(): - # Other views will be shown with class moin-nonexistent as non-existent links - endpoint = app.view_endpoints[view_name] + _, rev, query = self._get_do_rev(input.query) url = url_for_item(item_name, rev=rev, endpoint=endpoint) if not page: @@ -261,10 +261,15 @@ def handle_wikilocal_links(self, elem: Element, input: Iri, page: Iri, to_tag=Co :param page: the iri of the page where the link is """ path = input.path - if not path or ":" in path: + if not path: return - - path = self.absolute_path(path, page.path) + info = app.link_analyzer(str(path)) + if not info.is_valid or info.is_global or not info.item_name: + return + if info.endpoint == "frontend.show_item": + path = self.absolute_path(IriPath(input.path), page.path) + else: + path = IriPath(info.item_name) self.links.add(str(path)) def handle_wikilocal_transclusions(self, elem: Element, input: Iri, page): diff --git a/src/moin/themes/__init__.py b/src/moin/themes/__init__.py index d87c84cad..fd187ba63 100644 --- a/src/moin/themes/__init__.py +++ b/src/moin/themes/__init__.py @@ -26,7 +26,7 @@ from moin import wikiutil, user from moin.constants.keys import USERID, ADDRESS, HOSTNAME, REVID, ITEMID, NAME_EXACT, ASSIGNED_TO, NAME, NAMESPACE from moin.constants.contenttypes import CONTENTTYPES_MAP, CONTENTTYPE_MARKUP, CONTENTTYPE_TEXT, CONTENTTYPE_MOIN_19 -from moin.constants.misc import VALID_ITEMLINK_VIEWS, FLASH_REPEAT, ICON_MAP +from moin.constants.misc import FLASH_REPEAT, ICON_MAP from moin.constants.namespaces import NAMESPACE_DEFAULT, NAMESPACE_USERS, NAMESPACE_USERPROFILES, NAMESPACE_ALL from moin.constants.rights import SUPERUSER from moin.search import SearchForm @@ -525,7 +525,7 @@ def item_exists(self, itemname): """ return self.storage.has_item(itemname) - def itemlink_exists(self, itemlink): + def itemlink_exists(self, itemlink: str): """ Check whether the item pointed to by the given itemlink exists or not @@ -533,11 +533,14 @@ def itemlink_exists(self, itemlink): :returns: whether item pointed to by the link exists or not """ item_name = itemlink - if itemlink.startswith("+"): - view_name = itemlink.split("/")[0] - if view_name in VALID_ITEMLINK_VIEWS: - item_name = itemlink.split(f"{view_name}/")[1] - return self.storage.has_item(item_name) + info = app.link_analyzer(item_name) + if not info.is_valid: + return False + if info.is_global: + return True + if info.item_name: + item_name = info.item_name + return self.item_exists(item_name) def variables_css(self): """ diff --git a/src/moin/wikiutil.py b/src/moin/wikiutil.py index 67d1658f3..d5084dc24 100644 --- a/src/moin/wikiutil.py +++ b/src/moin/wikiutil.py @@ -18,6 +18,7 @@ import urllib from flask import current_app as app +from werkzeug.routing.exceptions import NoMatch, RoutingException from moin.constants.contenttypes import CHARSET from moin.constants.misc import URI_SCHEMES, CLEAN_INPUT_TRANSLATION_MAP, ITEM_INVALID_CHARS_REGEX @@ -27,6 +28,11 @@ from moin import log +from typing import NamedTuple, TYPE_CHECKING + +if TYPE_CHECKING: + from werkzeug.routing.map import MapAdapter + logging = log.getLogger(__name__) @@ -206,6 +212,48 @@ def AllParentNames(itemname): return result_names +class WikiLinkInfo(NamedTuple): + is_valid: bool + endpoint: str | None = None + item_name: str | None = None + is_global: bool = False + + +class WikiLinkAnalyzer: + """ + Helper class for analyzing wiki links. + + This class helps analyzing wiki internal links (don't use it for external links). + A MapAdapter instance is used to identify a route matching the provided link. + WikiLinkInfo members returned provide details about an analyzed link. + + Note: "item_name" may contain a colon character. In method inline_link_repl of class + Converter (moinwiki_in.py) it is assumed that if no interwiki name matching the + item name part before the colon does exist, the item name refers to a local + wiki page. + """ + + __slots__ = "map_adapter" + + def __init__(self, app): + self.map_adapter: MapAdapter = app.url_map.bind("127.0.0.1") # address is irrelevant + + def __call__(self, link: str) -> WikiLinkInfo: + if not link: + return WikiLinkInfo(False) + try: + # find moin rule matching link and the corresponding variable mappings + rule, vars = self.map_adapter.match(link, return_rule=True) + except (NoMatch, RoutingException): + return WikiLinkInfo(False) + item_name = vars.get("item_name", None) + if item_name and item_name.startswith("+"): + return WikiLinkInfo(False) + # set indicator if link refers to a real wiki item or not + is_global = item_name == "all" or rule.endpoint == "frontend.global_views" + return WikiLinkInfo(True, rule.endpoint, item_name, is_global) + + ############################################################################# # Misc #############################################################################