diff --git a/packages/reflex-base/news/7048.bugfix.md b/packages/reflex-base/news/7048.bugfix.md new file mode 100644 index 00000000000..172e5ed1e72 --- /dev/null +++ b/packages/reflex-base/news/7048.bugfix.md @@ -0,0 +1 @@ +Fix the Safari dev-server cache-busting plugin rendering pages as comma-separated byte values with React Router 8. The rewritten HTML now streams through instead of being buffered, and multibyte characters split across response chunks stay intact. diff --git a/packages/reflex-base/src/reflex_base/.templates/web/vite-plugin-safari-cachebust.js b/packages/reflex-base/src/reflex_base/.templates/web/vite-plugin-safari-cachebust.js index 02b9f39555a..ac2af098afc 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/vite-plugin-safari-cachebust.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/vite-plugin-safari-cachebust.js @@ -13,6 +13,8 @@ * output already contains the file hash in the name. */ +import { StringDecoder } from "node:string_decoder"; + /** * @typedef {import('vite').Plugin} Plugin * @typedef {import('vite').ViteDevServer} ViteDevServer @@ -22,6 +24,10 @@ */ const pluginName = "vite-plugin-safari-cachebust"; +const tsParam = "__reflex_ts"; +const linkTagRe = /]*>/g; +const tsUrlRe = new RegExp(`(\\?|&)${tsParam}=\\d+`); +const regExpSpecialsRe = /[.*+?^${}()|[\]\\]/g; /** * Creates a Vite plugin that adds cache-busting for Safari browsers @@ -50,53 +56,109 @@ function isSafari(ua) { } /** - * Creates a middleware that adds cache-busting for Safari browsers - * @returns {NextHandleFunction} The middleware function + * Creates a streaming rewriter for one HTML response. + * + * Hrefs are discovered from modulepreload tags and every later + * occurrence (e.g. the ESM imports in the trailing inline script) is rewritten + * as well. Text is emitted as soon as it arrives; only a possibly-partial + * tag or href at the end of a chunk is held back, unrewritten, until + * the next chunk. + * @param {number} timestamp - The cache-bust value for this response + * @returns {{push(text: string, flush: boolean): string, count: number}} The rewriter */ -function createSafariMiddleware() { - // Set when a log message for rewriting n links has been emitted. - let _have_logged_n = -1; +function createRewriter(timestamp) { + /** @type {Map} href -> href with the cache-bust param */ + const replacements = new Map(); + /** @type {RegExp | null} Alternation of every known href, longest first */ + let hrefRe = null; + let pending = ""; /** - * Rewrites module import links in HTML content with cache-busting parameters - * @param {string} html - The HTML content to process - * @returns {string} The processed HTML content + * Registers the hrefs of every complete modulepreload tag in the text + * @param {string} text - The text to scan */ - function rewriteModuleImports(html) { - const currentTimestamp = new Date().getTime(); - const parts = html.split(/(]*>)/g); - /** @type {[string, string][]} */ - const replacements = parts - .map((chunk) => { - const match = chunk.match( - //, - ); - if (!match) return; - - const [fullMatch, href, rest] = match; - if (/^(https?:)?\/\//.test(href)) return; - - try { - const newHref = href.includes("?") - ? `${href}&__reflex_ts=${currentTimestamp}` - : `${href}?__reflex_ts=${currentTimestamp}`; - return [href, newHref]; - } catch { - // no worries; - } - }) - .filter(Boolean); - if (replacements.length && _have_logged_n !== replacements.length) { - _have_logged_n = replacements.length; - console.debug( - `[${pluginName}] Rewrote ${replacements.length} modulepreload links with __reflex_ts param.`, + function discover(text) { + let changed = false; + for (const [, href] of text.matchAll(linkTagRe)) { + if (replacements.has(href) || /^(https?:)?\/\//.test(href)) continue; + replacements.set( + href, + `${href}${href.includes("?") ? "&" : "?"}${tsParam}=${timestamp}`, ); + changed = true; } - return replacements.reduce((accumulator, [target, replacement]) => { - return accumulator.split(target).join(replacement); - }, html); + if (!changed) return; + // Longest first, so an href that prefixes or contains another ("/a.js" in + // "/a.jsx", "/a.js?v=1" or "/b/a.js") is matched whole and every + // occurrence is rewritten exactly once in a single pass. + hrefRe = new RegExp( + [...replacements.keys()] + .sort((a, b) => b.length - a.length) + .map((href) => href.replace(regExpSpecialsRe, "\\$&")) + .join("|"), + "g", + ); } + /** + * Finds how much of the end of the text must wait for the next chunk + * @param {string} text - The raw text + * @returns {number} The index at which the held-back tail starts + */ + function cutIndex(text) { + let cut = text.length; + // An unclosed tag that may turn out to be a modulepreload link. + const lt = text.lastIndexOf("<"); + if (lt !== -1 && text.indexOf(">", lt) === -1) { + const tail = text.slice(lt, lt + 5); + if (tail.length < 5 ? " keep; k--) { + if (text.startsWith(href.slice(0, k), cut - k)) { + keep = k; + break; + } + } + } + return cut - keep; + } + + return { + /** + * Feeds text through the rewriter + * @param {string} text - The newly decoded text + * @param {boolean} flush - Whether this is the end of the response + * @returns {string} The text that may be sent now + */ + push(text, flush) { + text = pending + text; + discover(text); + const cut = flush ? text.length : cutIndex(text); + pending = text.slice(cut); + text = text.slice(0, cut); + return hrefRe + ? text.replace(hrefRe, (match) => replacements.get(match)) + : text; + }, + get count() { + return replacements.size; + }, + }; +} + +/** + * Creates a middleware that adds cache-busting for Safari browsers + * @returns {NextHandleFunction} The middleware function + */ +function createSafariMiddleware() { + // Set when a log message for rewriting n links has been emitted. + let _have_logged_n = -1; + /** * Middleware function to handle Safari cache busting * @param {IncomingMessage} req - The incoming request @@ -109,9 +171,9 @@ function createSafariMiddleware() { // Remove our special cache bust query param to avoid affecting lower middleware layers. if ( req.url && - (req.url.includes("?__reflex_ts=") || req.url.includes("&__reflex_ts=")) + (req.url.includes(`?${tsParam}=`) || req.url.includes(`&${tsParam}=`)) ) { - req.url = req.url.replace(/(\?|&)__reflex_ts=\d+/, ""); + req.url = req.url.replace(tsUrlRe, ""); return next(); } @@ -127,33 +189,68 @@ function createSafariMiddleware() { return next(); } - let buffer = ""; + const rewriter = createRewriter(Date.now()); + // Chunks may be Buffer or plain Uint8Array and may split a multibyte character. + const decoder = new StringDecoder("utf-8"); + const _write = res.write.bind(res); const _end = res.end.bind(res); - res.setHeader("x-modified-by", "vite-plugin-safari-cachebust"); /** - * Overridden write method to collect chunks + * Decodes a written chunk to text + * @param {any} chunk - The chunk passed to write/end, if any + * @returns {string} The decoded text + */ + const decode = (chunk) => + typeof chunk === "string" + ? decoder.end() + chunk + : chunk + ? decoder.write(chunk) + : ""; + + /** + * Extracts the optional completion callback from write/end arguments + * @param {any[]} args - The arguments following the chunk + * @returns {((err?: Error) => void) | undefined} The callback, if given + */ + const callback = (args) => args.find((arg) => typeof arg === "function"); + + res.setHeader("x-modified-by", pluginName); + /** + * Overridden write method to rewrite chunks as they stream through * @param {any} chunk - The chunk to write * @param {...any} args - Additional arguments * @returns {boolean} Result of the write operation */ res.write = function (chunk, ...args) { - buffer += chunk instanceof Buffer ? chunk.toString("utf-8") : chunk; + const out = rewriter.push(decode(chunk), false); + const cb = callback(args); + if (out) return cb ? _write(out, cb) : _write(out); + // Everything was held back for the next chunk; nothing is queued. Node + // never invokes a write callback synchronously, so defer it likewise. + if (cb) process.nextTick(cb); return true; }; /** - * Overridden end method to process and send the final response + * Overridden end method to flush held-back text and finish the response * @param {any} chunk - The final chunk to write * @param {...any} args - Additional arguments * @returns {ServerResponse} The server response */ res.end = function (chunk, ...args) { - if (chunk) { - buffer += chunk instanceof Buffer ? chunk.toString("utf-8") : chunk; + if (typeof chunk === "function") { + args.unshift(chunk); + chunk = undefined; + } + const out = rewriter.push(decode(chunk) + decoder.end(), true); + if (rewriter.count && _have_logged_n !== rewriter.count) { + _have_logged_n = rewriter.count; + console.debug( + `[${pluginName}] Rewrote ${rewriter.count} modulepreload links with ${tsParam} param.`, + ); } - buffer = rewriteModuleImports(buffer); - return _end(buffer, ...args); + const cb = callback(args); + return cb ? _end(out, cb) : _end(out); }; return next(); }; diff --git a/tests/units/reflex_base/templates/__init__.py b/tests/units/reflex_base/templates/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/units/reflex_base/templates/test_vite_plugin_safari_cachebust.py b/tests/units/reflex_base/templates/test_vite_plugin_safari_cachebust.py new file mode 100644 index 00000000000..51cf5318f8e --- /dev/null +++ b/tests/units/reflex_base/templates/test_vite_plugin_safari_cachebust.py @@ -0,0 +1,253 @@ +"""Tests for the Safari cache-busting Vite plugin shipped in the web template.""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest +import reflex_base + +PLUGIN_PATH = ( + Path(reflex_base.__file__).parent + / ".templates" + / "web" + / "vite-plugin-safari-cachebust.js" +) + +SAFARI_UA = "Mozilla/5.0 (Macintosh) AppleWebKit/605.1.15 Safari/605.1.15" + +# Drives the plugin's middleware with a fake request/response and prints every +# body fragment handed to the underlying ``res.write``/``res.end`` so the python +# side can check both the final document and how it was streamed. +DRIVER = """ +import { pathToFileURL } from "node:url"; +const plugin = (await import(pathToFileURL(process.argv[2]).href)).default; +const { chunks, userAgent } = JSON.parse(process.argv[3]); +console.debug = () => {}; +let middleware; +plugin().configureServer({ middlewares: { use: (m) => (middleware = m) } }); +const req = { url: "/", headers: { "user-agent": userAgent, accept: "text/html" } }; +const writes = []; +const headers = {}; +const res = { + setHeader(name, value) { headers[name] = value; }, + write(chunk) { writes.push(String(chunk)); return true; }, + end(chunk) { + const end = chunk === undefined ? null : String(chunk); + process.stdout.write(JSON.stringify({ writes, end, headers })); + }, +}; +middleware(req, res, () => { + for (const [kind, text] of chunks) { + const bytes = new TextEncoder().encode(text); + if (kind === "string") res.write(text); + else if (kind === "buffer") res.write(Buffer.from(bytes)); + else if (kind === "uint8array") res.write(bytes); + else if (kind === "bytes") res.write(Buffer.from(JSON.parse(text))); + } + res.end(); +}); +""" + + +def _run_plugin( + chunks: list[tuple[str, str]], tmp_path: Path, user_agent: str = SAFARI_UA +) -> dict: + """Send the given chunks through the plugin middleware in node. + + Args: + chunks: ``(kind, text)`` pairs written to the response in order. + tmp_path: Directory used to write the driver script. + user_agent: The request user agent. + + Returns: + The recorded ``writes``, ``end`` body and response ``headers``. + """ + driver = tmp_path / "driver.mjs" + driver.write_text(DRIVER) + return json.loads( + subprocess.run( + [ + "node", + str(driver), + str(PLUGIN_PATH), + json.dumps({"chunks": chunks, "userAgent": user_agent}), + ], + check=True, + capture_output=True, + encoding="utf-8", + ).stdout + ) + + +def _body(result: dict) -> str: + """Join everything the plugin sent into the final document. + + Args: + result: The driver output. + + Returns: + The full response body. + """ + return "".join(result["writes"]) + (result["end"] or "") + + +HTML = '

ok

' + +pytestmark = pytest.mark.skipif(shutil.which("node") is None, reason="node missing") + + +@pytest.mark.parametrize("kind", ["string", "buffer", "uint8array"]) +def test_rewrites_modulepreload_for_any_chunk_type(kind: str, tmp_path: Path): + """Chunks of every type the dev server may write are decoded as text. + + React Router 8 writes plain ``Uint8Array`` chunks (not ``Buffer``), which + previously got stringified as comma-separated byte values. + + Args: + kind: The chunk type to write. + tmp_path: Pytest temporary directory. + """ + body = _body(_run_plugin([(kind, HTML)], tmp_path)) + assert body.startswith("") + assert 'href="/app.js?__reflex_ts=' in body + assert body.endswith("

ok

") + + +def test_multibyte_character_split_across_chunks(tmp_path: Path): + """A UTF-8 sequence spanning two chunks is decoded intact. + + Args: + tmp_path: Pytest temporary directory. + """ + raw = list("

é

".encode()) + result = _run_plugin( + [("bytes", json.dumps(raw[:4])), ("bytes", json.dumps(raw[4:]))], tmp_path + ) + assert _body(result) == "

é

" + + +def test_streams_and_rewrites_across_chunk_boundaries(tmp_path: Path): + """Chunks stream out as they arrive, even when a tag or href is split. + + The document is cut inside a ```` tag and inside an href used by a + later ESM import; both must still be rewritten with one shared timestamp. + + Args: + tmp_path: Pytest temporary directory. + """ + chunks = [ + '' + '
content
', + ] + result = _run_plugin([("uint8array", c) for c in chunks], tmp_path) + body = _body(result) + timestamps = set(re.findall(r"__reflex_ts=(\d+)", body)) + assert len(timestamps) == 1 + ts = timestamps.pop() + assert body == ( + '
content
' + ) + # Every chunk was sent as it arrived, holding back only the partial tag/href. + assert len(result["writes"]) == len(chunks) + assert result["writes"][0] == "" + assert "
content
" in result["writes"][1] + assert result["end"] == "" + + +def test_hrefs_that_prefix_each_other(tmp_path: Path): + """An href that is a prefix of another is not rewritten inside the longer one. + + The document is also cut right where the longer href's query begins, so + the shorter match at the chunk end must wait for the continuation. + + Args: + tmp_path: Pytest temporary directory. + """ + chunks = [ + ( + '' + '' + '' + '', + ] + body = _body(_run_plugin([("string", c) for c in chunks], tmp_path)) + ts = re.search(r"__reflex_ts=(\d+)", body) + assert ts is not None + ts = ts.group(1) + assert body == ( + f'' + f'' + f'' + f'' + ) + + +def test_hrefs_that_contain_each_other(tmp_path: Path): + """An href occurring inside a longer one is rewritten once, as the longer href. + + ``/a.js`` is a substring, but not a prefix, of ``/b/a.js``; neither URL may + end up with two cache-bust params, whichever is discovered first. + + Args: + tmp_path: Pytest temporary directory. + """ + html = ( + '' + '' + '' + ) + body = _body(_run_plugin([("string", html)], tmp_path)) + ts = re.search(r"__reflex_ts=(\d+)", body) + assert ts is not None + ts = ts.group(1) + assert body == ( + f'' + f'' + f'' + ) + + +def test_href_with_query_and_external_links(tmp_path: Path): + """Existing query strings get ``&`` and external hrefs are left alone. + + Args: + tmp_path: Pytest temporary directory. + """ + html = ( + '' + '' + '' + ) + body = _body(_run_plugin([("string", html)], tmp_path)) + assert body.count("/a.js?v=1&__reflex_ts=") == 2 + assert 'href="https://cdn.example/b.js"' in body + + +def test_non_safari_passthrough(tmp_path: Path): + """Non-Safari browsers get the response untouched. + + Args: + tmp_path: Pytest temporary directory. + """ + result = _run_plugin( + [("uint8array", HTML)], + tmp_path, + user_agent="Mozilla/5.0 Chrome/120 Safari/537.36", + ) + assert "x-modified-by" not in result["headers"] + assert result["end"] is None + assert len(result["writes"]) == 1