diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index f99707ce519..05d8d2473d6 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -7,6 +7,7 @@ **Bugfixes:** +- Fixed an issue where the Cypress script was injected into a commented-out ``, ``, or `` tag inside an HTML comment, corrupting the served page. Commented-out tags are no longer considered injection points. Fixes [#33000](https://github.com/cypress-io/cypress/issues/33000). - Fixed an issue where, on Windows, enhancing a test failure stack could throw a secondary `TypeError: Cannot read properties of undefined (reading 'replaceAll')` and mask the original error. Fixed in [#34252](https://github.com/cypress-io/cypress/pull/34252). - Fixed an issue where [`experimentalMemoryManagement`](https://on.cypress.io/experiments) could fail to prevent the browser from running out of memory and crashing when Cypress was running inside a memory-limited container. Memory is now managed correctly in these environments. Fixes [#34104](https://github.com/cypress-io/cypress/issues/34104). Addressed in [#34123](https://github.com/cypress-io/cypress/pull/34123). diff --git a/packages/proxy/lib/http/util/rewriter.ts b/packages/proxy/lib/http/util/rewriter.ts index 6512b79eb98..d04b580adc5 100644 --- a/packages/proxy/lib/http/util/rewriter.ts +++ b/packages/proxy/lib/http/util/rewriter.ts @@ -68,6 +68,75 @@ function getHtmlToInject (opts: InjectionOpts & SecurityOpts) { } } +// replaces HTML comments with same-length whitespace so the injection point +// regexes can't match tags inside comments, while every index stays aligned +// with the original html. a small scanner tracks tag and quoted-attribute +// state so ``, `--!>` or abruptly (``, ``), and +// an unterminated comment blanks the rest of the document +const maskHtmlComments = (html: string) => { + const masked: string[] = [] + let i = 0 + let inTag = false + let quote = '' + + while (i < html.length) { + const ch = html[i] + + if (inTag) { + if (quote) { + if (ch === quote) { + quote = '' + } + } else if (ch === '"' || ch === '\'') { + quote = ch + } else if (ch === '>') { + inTag = false + } + + masked.push(ch) + i++ + + continue + } + + if (html.startsWith('', i + 4) + const bangClose = html.indexOf('--!>', i + 4) + + if (normalClose !== -1 && (bangClose === -1 || normalClose < bangClose)) { + end = normalClose + 3 + } else if (bangClose !== -1) { + end = bangClose + 4 + } else { + end = html.length + } + } + + masked.push(' '.repeat(end - i)) + i = end + + continue + } + + if (ch === '<' && /[a-zA-Z!/?]/.test(html[i + 1] || '')) { + inTag = true + } + + masked.push(ch) + i++ + } + + return masked.join('') +} + const insertBefore = (originalString, match, stringToInsert) => { const index = match.index || 0 @@ -93,7 +162,11 @@ export async function html (html: string, opts: SecurityOpts & InjectionOpts) { return html } - const bootstrapMatch = html.match(bootstrapScriptRe) + // search a comment-masked copy so a commented-out tag can't become the + // injection point, then splice into the original html by index + const searchableHtml = maskHtmlComments(html) + + const bootstrapMatch = searchableHtml.match(bootstrapScriptRe) if (bootstrapMatch) { const contentToInject = htmlToInject.replace(/^]*>|<\/script>$/g, '') @@ -104,31 +177,34 @@ export async function html (html: string, opts: SecurityOpts & InjectionOpts) { openTag = openTag.replace(/>$/, ` nonce="${opts.cspNonce}">`) } - return html.replace(bootstrapScriptRe, `${openTag}${contentToInject}${bootstrapMatch[3]}`) + const start = bootstrapMatch.index || 0 + const end = start + bootstrapMatch[0].length + + return `${html.slice(0, start)}${openTag}${contentToInject}${bootstrapMatch[3]}${html.slice(end)}` } // TODO: move this into regex-rewriting and have ast-rewriting handle this in its own way - const headMatch = html.match(headRe) + const headMatch = searchableHtml.match(headRe) if (headMatch) { return insertAfter(html, headMatch, htmlToInject) } - const bodyMatch = html.match(bodyRe) + const bodyMatch = searchableHtml.match(bodyRe) if (bodyMatch) { return insertBefore(html, bodyMatch, ` ${htmlToInject} `) } - const htmlMatch = html.match(htmlRe) + const htmlMatch = searchableHtml.match(htmlRe) if (htmlMatch) { return insertAfter(html, htmlMatch, ` ${htmlToInject} `) } // if only content, inject after doctype - if (doctypeRe.test(html)) { + if (doctypeRe.test(searchableHtml)) { return `${html} ${htmlToInject} ` } diff --git a/packages/proxy/test/unit/http/util/rewriter.spec.ts b/packages/proxy/test/unit/http/util/rewriter.spec.ts index f608e0b19bd..f2ceb060681 100644 --- a/packages/proxy/test/unit/http/util/rewriter.spec.ts +++ b/packages/proxy/test/unit/http/util/rewriter.spec.ts @@ -42,6 +42,122 @@ describe('http/util/rewriter', () => { expect(result).toContain('document.domain') }) + it('does not inject into a commented-out ', async () => { + // https://github.com/cypress-io/cypress/issues/33000 + const html = '\n\n' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + // The comment must survive untouched, with the injection placed in a + // real after the tag + expect(result).toContain('') + expect(result).toContain(' as the injection point', async () => { + const html = '' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + expect(result).toContain('') + expect(result).toContain(' ') + expect(result.indexOf('')) + }) + + it('ignores an unterminated comment when picking the injection point', async () => { + const html = 't' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + // The commented marker must survive untouched and the injection must + // land in the real head + expect(result).toContain('') + expect(result).toContain(' { + const html = 't' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + // The real head, including its attributes, must be the injection point + expect(result).toContain(' { + const html = 't' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + expect(result).toContain(' before the injection point', async () => { + const html = 't' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + expect(result).toContain(' as the injection point', async () => { + const html = '\n' + const opts = { + domainName: 'localhost', + wantsInjection: 'full', + shouldInjectDocumentDomain: true, + } as any + + const result = await rewriter.html(html, opts) + + expect(result).toContain('') + expect(result.indexOf('')) + }) + it('preserves existing attributes on developer-provided script tag', async () => { const html = '' const opts = {