From 970f2517e69a2b9168178236e60930ec6bb1dba4 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Mon, 25 Mar 2024 17:15:22 +0100 Subject: [PATCH 1/7] Introduce `treatAsNonHtml` to remove `isHTML` implementation --- src/core/index.js | 3 +- src/core/url.js | 13 +++-- src/tests/functional/visit_tests.js | 75 ++++++++++++++++++++++++++++- src/tests/server.mjs | 14 ++++++ 4 files changed, 98 insertions(+), 7 deletions(-) diff --git a/src/core/index.js b/src/core/index.js index a4a4f2d23..49c630cf3 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -4,10 +4,11 @@ import { PageSnapshot } from "./drive/page_snapshot" import { FrameRenderer } from "./frames/frame_renderer" import { FormSubmission } from "./drive/form_submission" import { fetch, recentRequests } from "../http/fetch" +import { treatAsNonHtml } from "./url" const session = new Session(recentRequests) const { cache, navigator } = session -export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch } +export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch, treatAsNonHtml } /** * Starts the main session. diff --git a/src/core/url.js b/src/core/url.js index dcd50cf26..8488653b6 100644 --- a/src/core/url.js +++ b/src/core/url.js @@ -22,17 +22,20 @@ 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 const treatAsNonHtml = new Set( + [ + ".css", ".csv", ".gif", ".jpeg", ".jpg", ".json", ".png", + ".pdf", ".svg", ".txt", ".xls", ".xlsx", ".zip" + ] +) + export function locationIsVisitable(location, rootLocation) { - return isPrefixedBy(location, rootLocation) && isHTML(location) + return isPrefixedBy(location, rootLocation) && !treatAsNonHtml.has(getExtension(location)) } export function getRequestURL(url) { diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index 782871747..a22a21bc5 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,81 @@ 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 treatAsNonHtml set", async ({ page }) => { + const requestedUrls = [] + page.on('request', (req) => { requestedUrls.push([req.resourceType(), req.url()]) }) + + page.evaluate(() => { + window.Turbo.treatAsNonHtml.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 10eab3abb..e5fc90990 100644 --- a/src/tests/server.mjs +++ b/src/tests/server.mjs @@ -168,6 +168,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) { From 09b1e36b1f8e9ee92e2e2f4e1b279cf413509edb Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Wed, 7 Aug 2024 14:49:33 +0200 Subject: [PATCH 2/7] Rename `treatAsNonHtml` to `unvisitableExtensions` and expand list --- src/core/index.js | 4 ++-- src/core/url.js | 9 ++++++--- src/tests/functional/visit_tests.js | 18 +++++++++--------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/core/index.js b/src/core/index.js index 49c630cf3..44f33c5e5 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -4,11 +4,11 @@ import { PageSnapshot } from "./drive/page_snapshot" import { FrameRenderer } from "./frames/frame_renderer" import { FormSubmission } from "./drive/form_submission" import { fetch, recentRequests } from "../http/fetch" -import { treatAsNonHtml } from "./url" +import { unvisitableExtensions } from "./url" const session = new Session(recentRequests) const { cache, navigator } = session -export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch, treatAsNonHtml } +export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch, unvisitableExtensions } /** * Starts the main session. diff --git a/src/core/url.js b/src/core/url.js index 8488653b6..d67ffec43 100644 --- a/src/core/url.js +++ b/src/core/url.js @@ -27,15 +27,18 @@ export function isPrefixedBy(baseURL, url) { return baseURL.href === expandURL(prefix).href || baseURL.href.startsWith(prefix) } -export const treatAsNonHtml = new Set( +export const unvisitableExtensions = new Set( [ ".css", ".csv", ".gif", ".jpeg", ".jpg", ".json", ".png", - ".pdf", ".svg", ".txt", ".xls", ".xlsx", ".zip" + ".pdf", ".svg", ".txt", ".xls", ".xlsx", ".zip", + ".tar", ".gz", ".bz2", ".rar", ".7z", ".dmg", ".exe", ".msi", ".pkg", ".deb", + ".iso", ".bmp", ".mp4", ".mov", ".avi", ".mkv", ".wmv", ".heic", ".heif", ".mp3", + ".wav", ".ogg", ".aac", ".wma", ".webm", ".ogv", ".mpg", ".mpeg" ] ) export function locationIsVisitable(location, rootLocation) { - return isPrefixedBy(location, rootLocation) && !treatAsNonHtml.has(getExtension(location)) + return isPrefixedBy(location, rootLocation) && !unvisitableExtensions.has(getExtension(location)) } export function getRequestURL(url) { diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index a22a21bc5..ec3dec8b9 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -67,8 +67,8 @@ test("visiting a location served with a known non-HTML content type", async ({ p assert.deepEqual( requestedUrls, [ - ['document', 'http://localhost:9000/src/tests/fixtures/svg.svg'] - ] + ['document', 'http://localhost:9000/src/tests/fixtures/svg.svg'] + ] ) const urlAfterVisit = page.url() @@ -88,9 +88,9 @@ test("visiting a location served with an unknown non-HTML content type", async ( // 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'] - ] + ['fetch', 'http://localhost:9000/__turbo/file.unknown_svg'], + ['document', 'http://localhost:9000/__turbo/file.unknown_svg'] + ] ) const urlAfterVisit = page.url() @@ -98,12 +98,12 @@ test("visiting a location served with an unknown non-HTML content type", async ( assert.equal(await visitAction(page), "load") }) -test("visiting a location served with an unknown non-HTML content type added to the treatAsNonHtml set", async ({ page }) => { +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.treatAsNonHtml.add(".unknown_svg") + window.Turbo.unvisitableExtensions.add(".unknown_svg") }) const urlBeforeVisit = page.url() @@ -112,8 +112,8 @@ test("visiting a location served with an unknown non-HTML content type added to assert.deepEqual( requestedUrls, [ - ['document', 'http://localhost:9000/__turbo/file.unknown_svg'] - ] + ['document', 'http://localhost:9000/__turbo/file.unknown_svg'] + ] ) const urlAfterVisit = page.url() From cdd500c4e5b8aef673bc31f5122ced05ca8fb349 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Wed, 7 Aug 2024 14:58:54 +0200 Subject: [PATCH 3/7] Sort extensions in Set and add some more commonly used extensions to our default set. --- src/core/url.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/url.js b/src/core/url.js index d67ffec43..12a958ffc 100644 --- a/src/core/url.js +++ b/src/core/url.js @@ -29,11 +29,12 @@ export function isPrefixedBy(baseURL, url) { export const unvisitableExtensions = new Set( [ - ".css", ".csv", ".gif", ".jpeg", ".jpg", ".json", ".png", - ".pdf", ".svg", ".txt", ".xls", ".xlsx", ".zip", - ".tar", ".gz", ".bz2", ".rar", ".7z", ".dmg", ".exe", ".msi", ".pkg", ".deb", - ".iso", ".bmp", ".mp4", ".mov", ".avi", ".mkv", ".wmv", ".heic", ".heif", ".mp3", - ".wav", ".ogg", ".aac", ".wma", ".webm", ".ogv", ".mpg", ".mpeg" + ".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" ] ) From f7b486796d6c4b3b61af949c8b04fd9940efdd36 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Mon, 2 Sep 2024 12:05:05 +0200 Subject: [PATCH 4/7] Move unvisitableExtensions config to config --- src/core/config/drive.js | 12 +++++++++++- src/core/index.js | 3 +-- src/core/url.js | 15 +++------------ src/tests/functional/visit_tests.js | 2 +- 4 files changed, 16 insertions(+), 16 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/index.js b/src/core/index.js index 5a8fe6cc4..17f804bcf 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -4,11 +4,10 @@ import { PageSnapshot } from "./drive/page_snapshot" import { FrameRenderer } from "./frames/frame_renderer" import { fetch, recentRequests } from "../http/fetch" import { config } from "./config" -import { unvisitableExtensions } from "./url" const session = new Session(recentRequests) const { cache, navigator } = session -export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch, config, unvisitableExtensions } +export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch, config } /** * Starts the main session. diff --git a/src/core/url.js b/src/core/url.js index 12a958ffc..544a3d0a4 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) } @@ -27,19 +29,8 @@ export function isPrefixedBy(baseURL, url) { return baseURL.href === expandURL(prefix).href || baseURL.href.startsWith(prefix) } -export const 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" - ] -) - export function locationIsVisitable(location, rootLocation) { - return isPrefixedBy(location, rootLocation) && !unvisitableExtensions.has(getExtension(location)) + return isPrefixedBy(location, rootLocation) && !config.drive.unvisitableExtensions.has(getExtension(location)) } export function getRequestURL(url) { diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index a6e0acdd7..a0c7df911 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -103,7 +103,7 @@ test("visiting a location served with an unknown non-HTML content type added to page.on('request', (req) => { requestedUrls.push([req.resourceType(), req.url()]) }) page.evaluate(() => { - window.Turbo.unvisitableExtensions.add(".unknown_svg") + window.Turbo.config.drive.unvisitableExtensions.add(".unknown_svg") }) const urlBeforeVisit = page.url() From 28911ad8ec8f56d241e6413f7cc529f8273bb712 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Mon, 2 Sep 2024 12:10:13 +0200 Subject: [PATCH 5/7] Remove semicolon --- src/core/url.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/url.js b/src/core/url.js index 544a3d0a4..ec7955fad 100644 --- a/src/core/url.js +++ b/src/core/url.js @@ -1,4 +1,4 @@ -import { config } from "./config"; +import { config } from "./config" export function expandURL(locatable) { return new URL(locatable.toString(), document.baseURI) From 10adafb3db05250475a77a1528657b04cba09206 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Mon, 2 Sep 2024 13:22:49 +0200 Subject: [PATCH 6/7] Fix config error in test --- src/tests/functional/form_submission_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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")) From 30cead8f8cf239b7edd4ba307f169c38f81e1197 Mon Sep 17 00:00:00 2001 From: Edwin Vlieg Date: Thu, 12 Sep 2024 16:24:08 +0200 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Jeffrey Hardy --- src/tests/functional/visit_tests.js | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index a0c7df911..165aef74b 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -65,11 +65,9 @@ test("visiting a location served with a known non-HTML content type", async ({ p const contentType = await contentTypeOfURL(url) assert.equal(contentType, "image/svg+xml") - assert.deepEqual( - requestedUrls, [ - ['document', 'http://localhost:9000/src/tests/fixtures/svg.svg'] - ] - ) + assert.deepEqual(requestedUrls, [ + ["document", "http://localhost:9000/src/tests/fixtures/svg.svg"] + ]) const urlAfterVisit = page.url() assert.notEqual(urlBeforeVisit, urlAfterVisit) @@ -86,12 +84,10 @@ test("visiting a location served with an unknown non-HTML content type", async ( // 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'] - ] - ) + 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) @@ -110,11 +106,9 @@ test("visiting a location served with an unknown non-HTML content type added to await visitLocation(page, "/__turbo/file.unknown_svg") await nextBeat() - assert.deepEqual( - requestedUrls, [ - ['document', 'http://localhost:9000/__turbo/file.unknown_svg'] - ] - ) +assert.deepEqual(requestedUrls, [ + ["document", "http://localhost:9000/__turbo/file.unknown_svg"] +]) const urlAfterVisit = page.url() assert.notEqual(urlBeforeVisit, urlAfterVisit)