Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce unvisitableExtensions to remove isHTML implementation #1230

Merged
merged 9 commits into from
Sep 12, 2024
12 changes: 11 additions & 1 deletion src/core/config/drive.js
Original file line number Diff line number Diff line change
@@ -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"
]
)
}
8 changes: 3 additions & 5 deletions src/core/url.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { config } from "./config"

export function expandURL(locatable) {
return new URL(locatable.toString(), document.baseURI)
}
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/tests/functional/form_submission_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
69 changes: 68 additions & 1 deletion src/tests/functional/visit_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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")])
Expand Down
14 changes: 14 additions & 0 deletions src/tests/server.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading