From 425da053810f4fdfadfad63d9b92e39d081de1d3 Mon Sep 17 00:00:00 2001 From: benoit74 Date: Fri, 31 Jul 2026 12:28:45 +0000 Subject: [PATCH] Backport wabac changes up to wabac.js 2.26.6 --- CHANGELOG.md | 1 + rules/rules.yaml | 4 +- src/zimscraperlib/rewriting/js.py | 93 +++++++++++++++----------- tests/rewriting/test_html_rewriting.py | 8 +-- tests/rewriting/test_js_rewriting.py | 44 +++++++++++- 5 files changed, 104 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9707e4..35b222e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Upgrade Python and Javascript dependencies (especially wombat 3.10.6) (#319, #328) +- Backport wabac changes up to wabac.js 2.26.6 (#330) - Migrate `javascript/` from Yarn Classic to Yarn Berry (#320) - Test bogus Content-Length HTTP header behavior (#318) diff --git a/rules/rules.yaml b/rules/rules.yaml index d703986..b1c2734 100644 --- a/rules/rules.yaml +++ b/rules/rules.yaml @@ -6,8 +6,8 @@ # ones) but just rewriting to proper path. # # This file is in sync with content at commit -# https://github.com/webrecorder/wabac.js/commit/f62756661d06e721bc57ff25199c73ce51227916 -# from October 29, 2025 +# https://github.com/webrecorder/wabac.js/commit/067214314f86249ecc7610bf8ac14b7f54c5f285 +# from Apr 29, 2026 # # This file should be updated at every release of scraperlib # diff --git a/src/zimscraperlib/rewriting/js.py b/src/zimscraperlib/rewriting/js.py index 8d27a91..2743fde 100644 --- a/src/zimscraperlib/rewriting/js.py +++ b/src/zimscraperlib/rewriting/js.py @@ -14,12 +14,12 @@ This code is based on https://github.com/webrecorder/wabac.js/blob/main/src/rewrite/jsrewriter.ts Last backport of upstream changes is from wabac.js commit: -Feb 20, 2026 - 25061cb53ff113d5cff28f2f1354819f6c41034b +Jul 30, 2026 - 0564e36993f4044f17119e71dd7b2892512ee59c """ import re from collections.abc import Callable, Iterable -from typing import Any +from typing import Any, Literal from zimscraperlib.rewriting.rx_replacer import ( RxRewriter, @@ -33,6 +33,12 @@ from zimscraperlib.rewriting.url_rewriting import ArticleUrlRewriter, ZimPath # The regex used to rewrite `import ...` in module code. +IMPORT_RX = re.compile( + r"""^\s*?import\s*?[{"'*]""", +) +EXPORT_RX = re.compile( + r"""\s*?export\s*?({([\s\w,$\n]+?)}[\s;]*|default|class)\s+""", re.MULTILINE +) IMPORT_EXPORT_MATCH_RX = re.compile( r"""(^|;)\s*?(?:im|ex)port(?:['"\s]*(?:[\w*${}\s,]+from\s*)?['"\s]?['"\s])(?:.*?)['"\s]""", ) @@ -56,12 +62,16 @@ "opener", ] -GLOBALS_RX = re.compile( +WORKER_GLOBAL_OVERRIDES = ["globalThis", "self", "location"] + +GLOBALS_CONCAT_STR = ( r"(" + "|".join([r"(?:^|[^$.])\b" + x + r"\b(?:$|[^$])" for x in GLOBAL_OVERRIDES]) + ")" ) +GLOBALS_RX = re.compile(GLOBALS_CONCAT_STR) + # This will replace `this` in code. The `_____WB$wombat$check$this$function_____` # will "see" with wombat and may return a "wrapper" around `this` this_rw = "_____WB$wombat$check$this$function_____(this)" @@ -84,7 +94,7 @@ def remove_args_if_strict( return target -def add_suffix_non_prop(suffix: str) -> TransformationAction: +def add_suffix(suffix: str) -> TransformationAction: """ Create a rewrite_function which add a `suffix` to the match str. The suffix is added only if the match is not preceded by `.` or `$`. @@ -108,7 +118,7 @@ def replace_this() -> TransformationAction: return replace("this", this_rw) -def replace_this_non_prop() -> TransformationAction: +def replace_this_prop() -> TransformationAction: """ Create a rewrite_function replacing "this" by `this_rw`. @@ -117,12 +127,12 @@ def replace_this_non_prop() -> TransformationAction: def f(m_object: re.Match[str], _opts: dict[str, Any] | None) -> str: offset = m_object.start() - prev = m_object.string[offset - 1] if offset > 0 else "" - if prev == "\n": + first_char = m_object.string[offset - 1] if offset > 0 else "" + if first_char == "\n": # This detection of new line is probably buggy, plus it is hard to get the # intent of this, see https://github.com/openzim/warc2zim/issues/410 return m_object[0].replace("this", ";" + this_rw) - if prev not in ".$": + if first_char not in ".$": return m_object[0].replace("this", this_rw) return m_object[0] @@ -186,10 +196,14 @@ def create_js_rules() -> list[TransformationRule]: (re.compile(r"var\s+self"), replace("var", "let")), # rewriting `.postMessage` -> `__WB_pmw(self).postMessage` (re.compile(r"\.postMessage\b\("), add_prefix(".__WB_pmw(self)")), + # Avoid doing the below rewrite for `let/const` assignments, + # which will break the scoping + # See: https://github.com/webrecorder/wabac.js/issues/336 + (re.compile(r"(?:let|const)\s+location\s*="), m2str(lambda x: x)), # rewriting `location = ` to custom expression `(...).href =` assignement ( re.compile(r"(?:^|[^$.+*/%^-])\s?\blocation\b\s*[=]\s*(?![\s\d=>])"), - add_suffix_non_prop(check_loc), + add_suffix(check_loc), ), # rewriting `return this` (re.compile(r"\breturn\s+this\b\s*(?![\s\w.$])"), replace_this()), @@ -199,7 +213,7 @@ def create_js_rules() -> list[TransformationRule]: re.compile( rf"[^$.]\s?\bthis\b(?=(?:\.(?:{'|'.join(GLOBAL_OVERRIDES)})\b))" ), - replace_this_non_prop(), + replace_this_prop(), ), # rewrite `= this` or `, this` (re.compile(r"[=,]\s*\bthis\b\s*(?![\s\w:.$])"), replace_this()), @@ -239,7 +253,7 @@ def __init__( ): super().__init__(None) self.first_buff = self._init_local_declaration(GLOBAL_OVERRIDES) - self.last_buff = "\n}" + self.last_buff = "\n\n}" self.url_rewriter = url_rewriter self.notify_js_module = notify_js_module self.base_href = base_href @@ -277,29 +291,20 @@ def _get_module_decl(self, local_decls: Iterable[str]) -> str: f"""import {{ {", ".join(local_decls)} }} from "{wb_module_decl_url}";\n""" ) - def _detect_strict_mode(self, text: str) -> bool: + def _detect_module_or_strict(self, text: str) -> Literal["strict", "module", "lax"]: """ - Detect if the JavaScript code is in strict mode. - - Returns True if the code contains: - - "use strict"; directive - - import statements - - export statements - - class declarations + Detect if the JavaScript code mode. """ - # Check for "use strict"; directive - if '"use strict";' in text or "'use strict';" in text: - return True + if "import" in text and IMPORT_RX.search(text): + return "module" - # Check for import or export statements - if re.search(r"(?:^|\s)(?:im|ex)port\s+", text): - return True + if '"use strict";' in text: + return "strict" - # Check for class declaration - if re.search(r"\bclass\s+", text): - return True + if "export" in text and EXPORT_RX.search(text): + return "module" - return False + return "lax" def rewrite(self, text: str | bytes, opts: dict[str, Any] | None = None) -> str: """ @@ -310,18 +315,24 @@ def rewrite(self, text: str | bytes, opts: dict[str, Any] | None = None) -> str: opts = opts or {} - is_module = opts.get("isModule", False) - - # Detect and set strict mode - # Modules are always strict mode - if is_module: + if "isModule" not in opts: + match self._detect_module_or_strict(text): + case "module": + opts["isModule"] = True + opts["isStrict"] = True + case "strict": + opts["isModule"] = False + opts["isStrict"] = True + case _: + pass + + elif opts["isModule"]: opts["isStrict"] = True - elif "isStrict" not in opts: # pragma: no branch - # Detect strict mode from the code itself - opts["isStrict"] = self._detect_strict_mode(text) rules = REWRITE_JS_RULES[:] + is_module = opts.get("isModule", False) + if is_module: rules.append(self._get_esm_import_rule()) @@ -332,12 +343,16 @@ def rewrite(self, text: str | bytes, opts: dict[str, Any] | None = None) -> str: if is_module: return self._get_module_decl(GLOBAL_OVERRIDES) + new_text - if GLOBALS_RX.search(text): - new_text = self.first_buff + new_text + self.last_buff + wrap_globals = GLOBALS_RX.search(text) is not None if opts.get("inline", False): new_text = new_text.replace("\n", " ") + # This is not totally correctly handling globals, + # see https://github.com/openzim/python-scraperlib/issues/329 + if wrap_globals: + new_text = self.first_buff + new_text + self.last_buff + return new_text def _get_esm_import_rule(self) -> TransformationRule: diff --git a/tests/rewriting/test_html_rewriting.py b/tests/rewriting/test_html_rewriting.py index a011988..34eccde 100644 --- a/tests/rewriting/test_html_rewriting.py +++ b/tests/rewriting/test_html_rewriting.py @@ -160,7 +160,7 @@ def test_escaped_content(escaped_content: ContentForTests): """let frames = _____WB$wombat$assign$function_____("frames");\n""" """let opener = _____WB$wombat$assign$function_____("opener");\n""" "let arguments;\n\n" - """document.title="HELLO";\n""" + """document.title="HELLO";\n\n""" "}" ), ), @@ -186,7 +186,7 @@ def test_escaped_content(escaped_content: ContentForTests): """let frames = _____WB$wombat$assign$function_____("frames");\n""" """let opener = _____WB$wombat$assign$function_____("opener");\n""" "let arguments;\n\n" - """document.title="HELLO";\n""" + """document.title="HELLO";\n\n""" "}" ), ), @@ -211,7 +211,7 @@ def test_escaped_content(escaped_content: ContentForTests): """let frames = _____WB$wombat$assign$function_____("frames");\n""" """let opener = _____WB$wombat$assign$function_____("opener");\n""" "let arguments;\n\n" - """document.title="HELLO";\n""" + """document.title="HELLO";\n\n""" "}" ), ), @@ -847,7 +847,7 @@ def test_simple_rewrite(input_content: str, expected_output: str): "let opener = " "_____WB$wombat$assign$function_____("opener");\n" "let arguments;\n\n" - "document.location.href='./index.html';\n" + "document.location.href='./index.html';\n\n" """}">""" ), # NOTA: quotes and ampersand are escaped since we are inside HTML attr ), diff --git a/tests/rewriting/test_js_rewriting.py b/tests/rewriting/test_js_rewriting.py index 5d07d9c..ab65e81 100644 --- a/tests/rewriting/test_js_rewriting.py +++ b/tests/rewriting/test_js_rewriting.py @@ -173,6 +173,7 @@ def wrap_script(text: str) -> str: "\n" f"{text}" "\n" + "\n" "}" ) @@ -235,6 +236,27 @@ class A {} "self.__WB_check_loc(location, [])) || {}).maybeHref " """= "http://example.com/2" """, ), + # Ensure these *don't* get rewritten + WrappedTestContent( + input_='function() { const location = "http://example.com/" }', + ), + WrappedTestContent( + input_='function() { let location = "http://example.com/"; }', + ), + WrappedTestContent( + input_="function() { const location = foo.location }", + ), + WrappedTestContent( + input_="function() { const location = window.location }", + ), + # this. is still rewritten + WrappedTestContent( + input_="function() { const location = this.location; }", + expected=( + "function() { const location =" + " _____WB$wombat$check$this$function_____(this).location; }" + ), + ), WrappedTestContent(input_=" var self ", expected=" let self "), ] ) @@ -389,7 +411,19 @@ def rewrite_import_content(request: pytest.FixtureRequest): yield request.param -def test_import_rewrite(rewrite_import_content: ImportTestContent): +def test_import_rewrite_module_detect(rewrite_import_content: ImportTestContent): + url_rewriter = ArticleUrlRewriter( + article_url=HttpUrl(rewrite_import_content.article_url) + ) + assert ( + JsRewriter( + url_rewriter=url_rewriter, base_href=None, notify_js_module=None + ).rewrite(rewrite_import_content.input_str) + == rewrite_import_content.expected_str + ) + + +def test_import_rewrite_force_module(rewrite_import_content: ImportTestContent): url_rewriter = ArticleUrlRewriter( article_url=HttpUrl(rewrite_import_content.article_url) ) @@ -401,6 +435,14 @@ def test_import_rewrite(rewrite_import_content: ImportTestContent): ) +# Check that forcing isModule to False works as expected +def test_import_rewrite_force_not_module(simple_js_rewriter: JsRewriter): + assert ( + simple_js_rewriter.rewrite("""import "foo";""", opts={"isModule": False}) + == """import "foo";""" + ) + + @pytest.fixture( params=[ "return this.abc",