diff --git a/changedetectionio/flask_app.py b/changedetectionio/flask_app.py index c98e2a79a..639bd0a14 100644 --- a/changedetectionio/flask_app.py +++ b/changedetectionio/flask_app.py @@ -216,13 +216,37 @@ csrf = CSRFProtect() csrf.init_app(app) notification_debug_log = [] -# Locale for correct presentation of prices etc +# Locale for correct presentation of prices etc. +# +# Deliberately NOT locale.LC_ALL - LC_COLLATE must stay in the "C" locale. +# +# elementpath implements the XPath string functions on top of locale.strxfrm: +# +# def contains(self, a, b): return self.strxfrm(b) in self.strxfrm(a) +# +# Under LC_COLLATE=C, strxfrm() is the identity function and that substring test means what it +# says. Under any real locale it returns a binary collation key, and a substring of a collation +# key is not the collation key of the substring - so contains(), starts-with(), ends-with() and +# substring-before/after() silently return false for EVERY input. Every xPath filter using +# contains() then matches nothing and the watch reports "no filters were found" on a page whose +# HTML plainly contains the target (#4437). +# +# That stayed hidden until the image actually generated its locales: before then this call raised +# locale.Error, we logged a warning and stayed in C. Once en_US.UTF-8 existed the call succeeded +# and took LC_COLLATE with it. Setting the presentation categories individually keeps what this +# block is for - 1234567 still renders as "1,234,567" - without touching collation. +# +# Per XPath 3.1 the default collation is codepoint and must not consult LC_COLLATE at all, so +# this is arguably an elementpath bug; html_tools.xpath_filter() pins the collation explicitly as +# well, so a filter is correct even if an operator sets LC_COLLATE themselves. default_locale = locale.getdefaultlocale() logger.info(f"System locale default is {default_locale}") -try: - locale.setlocale(locale.LC_ALL, default_locale) -except locale.Error: - logger.warning(f"Unable to set locale {default_locale}, locale is not installed maybe?") +for _category in (locale.LC_CTYPE, locale.LC_NUMERIC, locale.LC_MONETARY, locale.LC_TIME): + try: + locale.setlocale(_category, default_locale) + except locale.Error: + logger.warning(f"Unable to set locale {default_locale} for category {_category}, " + f"locale is not installed maybe?") watch_api = Api(app, decorators=[csrf.exempt]) diff --git a/changedetectionio/forms.py b/changedetectionio/forms.py index 3bdb9e983..f08e5ddd8 100644 --- a/changedetectionio/forms.py +++ b/changedetectionio/forms.py @@ -697,14 +697,18 @@ class ValidateCSSJSONXPATHInput(object): raise ValidationError("XPath not permitted in this field!") from lxml import etree, html import elementpath - from changedetectionio.html_tools import get_safe_xpath3_parser, lxml_guard, lxml_html_parser + from changedetectionio.html_tools import get_safe_xpath3_parser, lxml_guard, lxml_html_parser, \ + XPATH_CODEPOINT_COLLATION line = line.replace('xpath:', '') try: # Runs on a Flask request thread - must share the worker's lxml lock. with lxml_guard(): tree = html.fromstring("", parser=lxml_html_parser()) - elementpath.select(tree, line.strip(), parser=get_safe_xpath3_parser()) + # Same collation the filter will actually run under, so validation + # cannot accept an expression that then behaves differently at check time. + elementpath.select(tree, line.strip(), parser=get_safe_xpath3_parser(), + default_collation=XPATH_CODEPOINT_COLLATION) except elementpath.ElementPathError as e: message = field.gettext('\'%(expression)s\' is not a valid XPath expression. (%(error)s)') raise ValidationError(message % {'expression': line, 'error': str(e)}) diff --git a/changedetectionio/html_tools.py b/changedetectionio/html_tools.py index d40f17e17..e3b34f49c 100644 --- a/changedetectionio/html_tools.py +++ b/changedetectionio/html_tools.py @@ -164,6 +164,25 @@ _DEFAULT_UNSAFE_XPATH3_FUNCTIONS = [ ] +# XPath 3.1 says the default collation is Unicode codepoint collation. elementpath instead leaves +# its collation functions pointing at locale.strxfrm / locale.strcoll, so the process-wide +# LC_COLLATE decides what contains() means: +# +# def contains(self, a, b): return self.strxfrm(b) in self.strxfrm(a) +# +# With LC_COLLATE=C, strxfrm() is the identity and that is an ordinary substring test. With +# LC_COLLATE=en_US.UTF-8 it returns a binary collation key, and a substring of a collation key is +# not the collation key of the substring - so contains(), starts-with(), ends-with() and +# substring-before/after() return false for every input, and a filter that matches 67 elements +# matches 0 (#4437). Name tests, axes and '=' are unaffected, which is what made it look like the +# page had changed layout. +# +# flask_app.py keeps LC_COLLATE in "C" for this reason, but a filter must not depend on a distant +# module's locale bookkeeping, nor on what an operator puts in LANG/LC_ALL. Pinning the collation +# per evaluation makes the filter mean the same thing in every deployment. +XPATH_CODEPOINT_COLLATION = 'http://www.w3.org/2005/xpath-functions/collation/codepoint' + + def get_safe_xpath3_parser(): """Return an XPath3Parser subclass with filesystem/environment access functions removed. @@ -383,7 +402,9 @@ def xpath_filter(xpath_filter, html_content, append_pretty_line_formatting=False # This allows //title to match elements in the default namespace namespaces[''] = tree.nsmap[None] - r = elementpath.select(tree, xpath_filter.strip(), namespaces=namespaces, parser=get_safe_xpath3_parser()) + r = elementpath.select(tree, xpath_filter.strip(), namespaces=namespaces, + parser=get_safe_xpath3_parser(), + default_collation=XPATH_CODEPOINT_COLLATION) #@note: //title/text() now works with default namespaces (fixed by registering '' prefix) #@note: //title/text() wont work where CDATA.. (use cdata_in_document_to_text first) diff --git a/changedetectionio/tests/unit/test_xpath_collation_locale.py b/changedetectionio/tests/unit/test_xpath_collation_locale.py new file mode 100644 index 000000000..cfe8fce01 --- /dev/null +++ b/changedetectionio/tests/unit/test_xpath_collation_locale.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 + +"""An xPath filter must mean the same thing whatever LC_COLLATE the process is in. + +elementpath implements the XPath string functions on top of locale.strxfrm: + + def contains(self, a, b): return self.strxfrm(b) in self.strxfrm(a) + +Under LC_COLLATE=C, strxfrm() is the identity and that is an ordinary substring test. Under a +real locale it returns a binary collation key, and a substring of a collation key is not the +collation key of the substring - so contains(), starts-with(), ends-with() and +substring-before/after() return false for EVERY input. + +That shipped the day the Docker image started generating its locales: `ENV LC_ALL=en_US.UTF-8` +became satisfiable, flask_app's setlocale() stopped failing, LC_COLLATE went with it, and every +watch whose filter used contains() reported "no filters were found" against a page whose HTML +plainly contained the target (#4437). Nothing in the codebase changed - which is why it survived +a bisect back to 0.60.2 and could only be found by diffing the containers. + +Both halves of the fix are asserted here: the filter pins the codepoint collation itself, and +flask_app leaves LC_COLLATE alone. +""" + +import locale +import unittest + +from changedetectionio import html_tools + +HTML = """<html><body> + <div id="price_box"><p>Only $9.99 per month</p></div> + <div id="other">nothing to see</div> +</body></html>""" + +# Locales that are actually present vary by image; skip rather than fail on a bare CI box. +CANDIDATE_LOCALES = ['en_US.UTF-8', 'C.UTF-8', 'en_GB.UTF-8', 'de_DE.UTF-8'] + + +def _first_available_collate_locale(): + original = locale.setlocale(locale.LC_COLLATE) + try: + for loc in CANDIDATE_LOCALES: + try: + locale.setlocale(locale.LC_COLLATE, loc) + return loc + except locale.Error: + continue + return None + finally: + locale.setlocale(locale.LC_COLLATE, original) + + +class TestXpathCollationIsLocaleIndependent(unittest.TestCase): + + def setUp(self): + self.original_collate = locale.setlocale(locale.LC_COLLATE) + + def tearDown(self): + locale.setlocale(locale.LC_COLLATE, self.original_collate) + + def test_string_functions_survive_a_utf8_collation(self): + loc = _first_available_collate_locale() + if loc is None: + self.skipTest("no UTF-8 locale generated in this environment") + + # Every one of these is strxfrm-based inside elementpath, and every one of them appears + # in filters reported against #4437. + rules = [ + '//*[self::div or self::p][contains(.,"month")]', + '//div[contains(@id, "_")]', + '//p[starts-with(., "Only")]', + '//p[ends-with(., "month")]', + ] + + locale.setlocale(locale.LC_COLLATE, 'C') + baseline = {r: html_tools.xpath_filter(xpath_filter=r, html_content=HTML).strip() for r in rules} + for rule, out in baseline.items(): + self.assertTrue(out, f"{rule} matched nothing even under LC_COLLATE=C") + + locale.setlocale(locale.LC_COLLATE, loc) + for rule in rules: + out = html_tools.xpath_filter(xpath_filter=rule, html_content=HTML).strip() + self.assertEqual( + out, baseline[rule], + f"xPath filter {rule!r} behaves differently under LC_COLLATE={loc} than under C - " + f"the collation is leaking into the filter (#4437)" + ) + + def test_flask_app_does_not_touch_lc_collate(self): + """The presentation locale must not drag LC_COLLATE along with it. + + flask_app sets LC_CTYPE/LC_NUMERIC/LC_MONETARY/LC_TIME individually rather than LC_ALL. + A future edit back to locale.LC_ALL would silently reintroduce the bug, so pin it here - + the source is the honest thing to assert, because the import has long since run. + """ + from pathlib import Path + src = Path(html_tools.__file__).parent.joinpath('flask_app.py').read_text() + self.assertNotIn( + 'locale.setlocale(locale.LC_ALL', src, + "flask_app must not setlocale(LC_ALL, ...) - it takes LC_COLLATE with it and breaks " + "every xPath contains() filter (#4437)" + ) + + +if __name__ == '__main__': + unittest.main()