also looking at this
fix(router): stop executing data scripts on a fragment swap
#1802
2 files
+183
-0
| @@ -503,6 +503,22 @@ else { | ||
| 503 | 503 | var fragCrosswindHrefs=[]; |
| 504 | 504 | var fragCrosswindCSS=null; |
| 505 | 505 | var cleanFrag=html.replace(new RegExp('<scr'+'ipt\\\\b([^>]*)>([\\\\s\\\\S]*?)<\\\\/scr'+'ipt>','gi'),function(m,attrs,code){ |
| 506 | // A data block is not code, and everything below this line assumes it | |
| 507 | // is: the body gets stashed in fragScripts, re-emitted as a pending | |
| 508 | // placeholder, then wrapped in braces and executed. Wrapping a JSON | |
| 509 | // object in braces reparses it as a labelled block, so its first ':' | |
| 510 | // is a SyntaxError on every single fragment swap | |
| 511 | // (stacksjs/stx#1801). @structuredData renders at its call site, | |
| 512 | // which inside a layout is @section('content') — i.e. in the routed | |
| 513 | // container — so the natural way to write it hit this. | |
| 514 | // | |
| 515 | // Left exactly as found rather than stripped: crawlers read | |
| 516 | // structured data wherever it sits, and the browser will not execute | |
| 517 | // a non-JS type anyway. Mirrors prepareRoutedBodyScripts' notion of | |
| 518 | // executable, which this path never had. | |
| 519 | var typeMatch=attrs.match(/\\btype\\s*=\\s*(?:"([^"]*)"|'([^']*)'|([^\\s>]+))/i); | |
| 520 | var scriptType=((typeMatch&&(typeMatch[1]||typeMatch[2]||typeMatch[3]))||'').trim().toLowerCase(); | |
| 521 | if(scriptType&&scriptType!=='text/javascript'&&scriptType!=='application/javascript'&&scriptType!=='module')return m; | |
| 506 | 522 | if(code&&code.trim()&&!isSignalsRuntimeScript({hasAttribute:function(name){return name==='data-stx-runtime'&&attrs.indexOf('data-stx-runtime')!==-1}},code)){ |
| 507 | 523 | var slot='fragment-'+(++fragScriptId); |
| 508 | 524 | var scoped=/(?:^|\\s)data-stx-scoped(?:\\s|=|$)/i.test(attrs); |
| @@ -0,0 +1,167 @@ | ||
| 1 | /** | |
| 2 | * A data block is not code (stacksjs/stx#1801). | |
| 3 | * | |
| 4 | * `doFragSwap` pulls scripts out of an incoming fragment with a regex over the | |
| 5 | * raw HTML, and every match was treated as JavaScript: stashed, re-emitted as a | |
| 6 | * pending placeholder, then wrapped in braces and executed. Wrapping a JSON | |
| 7 | * object in braces reparses it as a labelled block, so its first `:` throws | |
| 8 | * `SyntaxError: Unexpected token ':'` — once per fragment swap, on every | |
| 9 | * navigation into the page. | |
| 10 | * | |
| 11 | * It stayed hidden because the swap still completes and the page looks correct; | |
| 12 | * it is only visible with the console open. The reporter found it after a full | |
| 13 | * stage of work had already shipped over it. | |
| 14 | * | |
| 15 | * `prepareRoutedBodyScripts` classifies by `type` and the head/body collection | |
| 16 | * requires a setup marker, so both of those paths were already safe. The regex | |
| 17 | * path never had the check — which is why the two obvious suspects looked | |
| 18 | * innocent when the issue was filed. | |
| 19 | * | |
| 20 | * `@structuredData` renders at its call site, and inside a layout that is | |
| 21 | * `@section('content')` — i.e. within the routed container. So the natural way | |
| 22 | * to write it triggered this, including the breadcrumb example in the guide. | |
| 23 | */ | |
| 24 | import { afterEach, describe, expect, it } from 'bun:test' | |
| 25 | import { Window } from 'very-happy-dom' | |
| 26 | import { getRouterScript } from '../src/client' | |
| 27 | ||
| 28 | const originalGlobals = { | |
| 29 | window: globalThis.window, | |
| 30 | document: globalThis.document, | |
| 31 | location: globalThis.location, | |
| 32 | history: globalThis.history, | |
| 33 | fetch: globalThis.fetch, | |
| 34 | CustomEvent: globalThis.CustomEvent, | |
| 35 | Event: globalThis.Event, | |
| 36 | DOMParser: globalThis.DOMParser, | |
| 37 | } | |
| 38 | ||
| 39 | afterEach(() => { | |
| 40 | Object.assign(globalThis, originalGlobals) | |
| 41 | }) | |
| 42 | ||
| 43 | const PAGE = ` | |
| 44 | <html> | |
| 45 | <head> | |
| 46 | <meta name="stx-layout" content="layouts/app.stx"> | |
| 47 | <meta name="stx-layout-group" content="app"> | |
| 48 | </head> | |
| 49 | <body><main>Home</main></body> | |
| 50 | </html> | |
| 51 | ` | |
| 52 | ||
| 53 | /** Same layout group on both sides, so navigation takes the fragment path. */ | |
| 54 | function installRouter(fragmentHtml: string) { | |
| 55 | const window = new Window({ url: 'http://localhost/' }) | |
| 56 | window.document.write(PAGE) | |
| 57 | ;(window as any).stx = {} | |
| 58 | ;(window as any).__stxRouterConfig = { | |
| 59 | cache: false, | |
| 60 | prefetch: false, | |
| 61 | progress: false, | |
| 62 | viewTransitions: false, | |
| 63 | } | |
| 64 | ||
| 65 | Object.assign(globalThis, { | |
| 66 | window, | |
| 67 | document: window.document, | |
| 68 | location: window.location, | |
| 69 | history: window.history, | |
| 70 | CustomEvent: window.CustomEvent, | |
| 71 | Event: window.Event, | |
| 72 | DOMParser: window.DOMParser, | |
| 73 | fetch: async () => new Response(fragmentHtml, { | |
| 74 | status: 200, | |
| 75 | headers: { | |
| 76 | 'Content-Type': 'text/html', | |
| 77 | 'X-STX-Fragment': 'true', | |
| 78 | 'X-STX-Layout': 'layouts/app.stx', | |
| 79 | 'X-STX-Layout-Group': 'app', | |
| 80 | }, | |
| 81 | }), | |
| 82 | }) | |
| 83 | ||
| 84 | new Function(getRouterScript())() | |
| 85 | return window as Window & { stxRouter: any } | |
| 86 | } | |
| 87 | ||
| 88 | async function navigate(window: any, url: string) { | |
| 89 | await window.stxRouter.navigate(url) | |
| 90 | await new Promise(r => setTimeout(r, 200)) | |
| 91 | } | |
| 92 | ||
| 93 | const LD_JSON = '{"@type":"BreadcrumbList","itemListElement":[{"@type":"ListItem","position":1}]}' | |
| 94 | ||
| 95 | describe('router — non-executable scripts in a swapped fragment', () => { | |
| 96 | it('does not brace-wrap a JSON-LD payload', async () => { | |
| 97 | // The wrap is what threw. Reproduce it directly so the premise is pinned: | |
| 98 | // if this ever stops throwing, the guard below is testing nothing. | |
| 99 | expect(() => new Function(`{${LD_JSON}}`)).toThrow() | |
| 100 | ||
| 101 | const window = installRouter( | |
| 102 | `<section><script type="application/ld+json">${LD_JSON}<\/script><h1>Pricing</h1></section>`, | |
| 103 | ) | |
| 104 | await navigate(window, '/pricing') | |
| 105 | ||
| 106 | const injected = Array.from(window.document.querySelectorAll('script[data-stx-page]')) | |
| 107 | .map((s: any) => s.textContent || '') | |
| 108 | expect(injected.some(t => t.includes('BreadcrumbList'))).toBe(false) | |
| 109 | }) | |
| 110 | ||
| 111 | it('leaves the structured data in the document for crawlers', async () => { | |
| 112 | // Stripping it would trade a console error for an SEO regression, so the | |
| 113 | // guard preserves the node verbatim rather than dropping it. | |
| 114 | const window = installRouter( | |
| 115 | `<section><script type="application/ld+json">${LD_JSON}<\/script><h1>Pricing</h1></section>`, | |
| 116 | ) | |
| 117 | await navigate(window, '/pricing') | |
| 118 | ||
| 119 | const ld = window.document.querySelector('script[type="application/ld+json"]') | |
| 120 | expect(ld).toBeTruthy() | |
| 121 | expect(ld?.textContent).toContain('BreadcrumbList') | |
| 122 | // And it must not have been rewritten into the router's pending placeholder. | |
| 123 | expect(ld?.getAttribute('data-stx-route-script')).toBeNull() | |
| 124 | }) | |
| 125 | ||
| 126 | it('survives repeated navigation, which is when the error fired', async () => { | |
| 127 | const window = installRouter( | |
| 128 | `<section><script type="application/ld+json">${LD_JSON}<\/script><h1>Page</h1></section>`, | |
| 129 | ) | |
| 130 | await navigate(window, '/pricing') | |
| 131 | await navigate(window, '/features') | |
| 132 | ||
| 133 | const injected = Array.from(window.document.querySelectorAll('script[data-stx-page]')) | |
| 134 | .map((s: any) => s.textContent || '') | |
| 135 | expect(injected.some(t => t.includes('BreadcrumbList'))).toBe(false) | |
| 136 | }) | |
| 137 | ||
| 138 | it('still executes real page scripts in the same fragment', async () => { | |
| 139 | // The guard must not become a blanket skip: a JSON-LD block sitting next to | |
| 140 | // a page script is the actual shape of an SEO'd page. | |
| 141 | const window = installRouter( | |
| 142 | `<section>` | |
| 143 | + `<script type="application/ld+json">${LD_JSON}<\/script>` | |
| 144 | + `<script data-stx-run="always">SENTINEL_EXEC()<\/script>` | |
| 145 | + `</section>`, | |
| 146 | ) | |
| 147 | await navigate(window, '/pricing') | |
| 148 | ||
| 149 | const injected = Array.from(window.document.querySelectorAll('script[data-stx-page]')) | |
| 150 | .map((s: any) => s.textContent || '') | |
| 151 | expect(injected.some(t => t.includes('SENTINEL_EXEC'))).toBe(true) | |
| 152 | }) | |
| 153 | ||
| 154 | it('treats other data types the same way', async () => { | |
| 155 | // importmap and speculationrules are JSON too, and an importmap executed as | |
| 156 | // JS would break module resolution for the whole document. | |
| 157 | const window = installRouter( | |
| 158 | `<section><script type="importmap">{"imports":{"a":"/a.js"}}<\/script><h1>Page</h1></section>`, | |
| 159 | ) | |
| 160 | await navigate(window, '/pricing') | |
| 161 | ||
| 162 | const injected = Array.from(window.document.querySelectorAll('script[data-stx-page]')) | |
| 163 | .map((s: any) => s.textContent || '') | |
| 164 | expect(injected.some(t => t.includes('imports'))).toBe(false) | |
| 165 | expect(window.document.querySelector('script[type="importmap"]')).toBeTruthy() | |
| 166 | }) | |
| 167 | }) | |