From b8b66625ff8611f589e6e9148459ba84c4e80b31 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Thu, 12 Sep 2024 17:38:44 +0300 Subject: [PATCH] Introduce `unvisitableExtensions ` to remove `isHTML` implementation (#1230) - Fixes #608 - Closes #519 --- src/core/config/drive.js | 12 +++- src/core/url.js | 8 +-- src/tests/functional/form_submission_tests.js | 2 +- src/tests/functional/visit_tests.js | 69 ++++++++++++++++++- src/tests/server.mjs | 14 ++++ 5 files changed, 97 insertions(+), 8 deletions(-) diff --git a/src/core/config/drive.js b/src/core/config/drive.js index 80f3c3c69..e5710d589 100644 --- a/src/core/config/drive.js +++ b/src/core/config/drive.js @@ -1,4 +1,14 @@ export const drive = { enabled: true, - progressBarDelay: 500 + progressBarDelay: 500, + unvisitableExtensions: new Set( + [ + ".7z", ".aac", ".apk", ".avi", ".bmp", ".bz2", ".css", ".csv", ".deb", ".dmg", ".doc", + ".docx", ".exe", ".gif", ".gz", ".heic", ".heif", ".ico", ".iso", ".jpeg", ".jpg", + ".js", ".json", ".m4a", ".mkv", ".mov", ".mp3", ".mp4", ".mpeg", ".mpg", ".msi", + ".ogg", ".ogv", ".pdf", ".pkg", ".png", ".ppt", ".pptx", ".rar", ".rtf", + ".svg", ".tar", ".tif", ".tiff", ".txt", ".wav", ".webm", ".webp", ".wma", ".wmv", + ".xls", ".xlsx", ".xml", ".zip" + ] + ) } diff --git a/src/core/url.js b/src/core/url.js index dcd50cf26..ec7955fad 100644 --- a/src/core/url.js +++ b/src/core/url.js @@ -1,3 +1,5 @@ +import { config } from "./config" + export function expandURL(locatable) { return new URL(locatable.toString(), document.baseURI) } @@ -22,17 +24,13 @@ export function getExtension(url) { return (getLastPathComponent(url).match(/\.[^.]*$/) || [])[0] || "" } -export function isHTML(url) { - return !!getExtension(url).match(/^(?:|\.(?:htm|html|xhtml|php))$/) -} - export function isPrefixedBy(baseURL, url) { const prefix = getPrefix(url) return baseURL.href === expandURL(prefix).href || baseURL.href.startsWith(prefix) } export function locationIsVisitable(location, rootLocation) { - return isPrefixedBy(location, rootLocation) && isHTML(location) + return isPrefixedBy(location, rootLocation) && !config.drive.unvisitableExtensions.has(getExtension(location)) } export function getRequestURL(url) { diff --git a/src/tests/functional/form_submission_tests.js b/src/tests/functional/form_submission_tests.js index 7723f4b88..83d769cd5 100644 --- a/src/tests/functional/form_submission_tests.js +++ b/src/tests/functional/form_submission_tests.js @@ -1206,7 +1206,7 @@ test("following a link with [data-turbo-method] and [data-turbo=true] set when h test("following a link with [data-turbo-method] and [data-turbo=true] set when Turbo.session.drive = false", async ({ page }) => { - await page.evaluate(() => (window.Turbo.config.drive = false)) + await page.evaluate(() => (window.Turbo.config.drive.enabled = false)) const link = await page.locator("#turbo-method-post-to-targeted-frame") await link.evaluate((link) => link.setAttribute("data-turbo", "true")) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index c53a2600c..165aef74b 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -53,7 +53,10 @@ test("skip programmatically visiting a cross-origin location falls back to windo assert.equal(await visitAction(page), "load") }) -test("visiting a location served with a non-HTML content type", async ({ page }) => { +test("visiting a location served with a known non-HTML content type", async ({ page }) => { + const requestedUrls = [] + page.on('request', (req) => { requestedUrls.push([req.resourceType(), req.url()]) }) + const urlBeforeVisit = page.url() await visitLocation(page, "/src/tests/fixtures/svg.svg") await nextBeat() @@ -62,11 +65,75 @@ test("visiting a location served with a non-HTML content type", async ({ page }) const contentType = await contentTypeOfURL(url) assert.equal(contentType, "image/svg+xml") + assert.deepEqual(requestedUrls, [ + ["document", "http://localhost:9000/src/tests/fixtures/svg.svg"] + ]) + const urlAfterVisit = page.url() assert.notEqual(urlBeforeVisit, urlAfterVisit) assert.equal(await visitAction(page), "load") }) +test("visiting a location served with an unknown non-HTML content type", async ({ page }) => { + const requestedUrls = [] + page.on('request', (req) => { requestedUrls.push([req.resourceType(), req.url()]) }) + + const urlBeforeVisit = page.url() + await visitLocation(page, "/__turbo/file.unknown_svg") + await nextBeat() + + // Because the file extension is not a known extension, Turbo will request it first to + // determine the content type and only then refresh the full page to the provided location + assert.deepEqual(requestedUrls, [ + ["fetch", "http://localhost:9000/__turbo/file.unknown_svg"], + ["document", "http://localhost:9000/__turbo/file.unknown_svg"] + ]) + + const urlAfterVisit = page.url() + assert.notEqual(urlBeforeVisit, urlAfterVisit) + assert.equal(await visitAction(page), "load") +}) + +test("visiting a location served with an unknown non-HTML content type added to the unvisitableExtensions set", async ({ page }) => { + const requestedUrls = [] + page.on('request', (req) => { requestedUrls.push([req.resourceType(), req.url()]) }) + + page.evaluate(() => { + window.Turbo.config.drive.unvisitableExtensions.add(".unknown_svg") + }) + + const urlBeforeVisit = page.url() + await visitLocation(page, "/__turbo/file.unknown_svg") + await nextBeat() + +assert.deepEqual(requestedUrls, [ + ["document", "http://localhost:9000/__turbo/file.unknown_svg"] +]) + + const urlAfterVisit = page.url() + assert.notEqual(urlBeforeVisit, urlAfterVisit) + assert.equal(await visitAction(page), "load") +}) + +test("visiting a location with a non-HTML extension", async ({ page }) => { + await visitLocation(page, "/__turbo/file.unknown_html") + await nextBeat() + + assert.equal(await visitAction(page), "advance") +}) + +test("refreshing a location with a non-HTML extension", async ({ page }) => { + await page.goto("/__turbo/file.unknown_html") + const urlBeforeVisit = page.url() + + await visitLocation(page, "/__turbo/file.unknown_html") + await nextBeat() + + const urlAfterVisit = page.url() + assert.equal(urlBeforeVisit, urlAfterVisit) + assert.equal(await visitAction(page), "advance") +}) + test("canceling a turbo:click event falls back to built-in browser navigation", async ({ page }) => { await cancelNextEvent(page, "turbo:click") await Promise.all([page.waitForNavigation(), page.click("#same-origin-link")]) diff --git a/src/tests/server.mjs b/src/tests/server.mjs index 76cdaf464..1978ca601 100644 --- a/src/tests/server.mjs +++ b/src/tests/server.mjs @@ -175,6 +175,20 @@ router.get("/messages", (request, response) => { streamResponses.add(response) }) +router.get("/file.unknown_svg", (request, response) => { + response.set({ + "Content-Type": "image/svg+xml" + }) + response.sendFile(path.join(__dirname, "../../src/tests/fixtures/svg.svg")) +}) + +router.get("/file.unknown_html", (request, response) => { + response.set({ + "Content-Type": "text/html" + }) + response.sendFile(path.join(__dirname, "../../src/tests/fixtures/visit.html")) +}) + function receiveMessage(content, id, target) { const data = renderSSEData(renderMessage(content, id, target)) for (const response of streamResponses) {