From 10ff074d19eabd481b59be071a0a81078f0ba0ca Mon Sep 17 00:00:00 2001 From: Jay Ohms Date: Thu, 28 Mar 2024 11:44:28 -0400 Subject: [PATCH 1/2] Detect cross-origin visit request attempts and propose them as new visits instead of displaying an error --- turbo/src/main/assets/js/turbo_bridge.js | 51 ++++++++++++++----- .../delegates/TurboWebFragmentDelegate.kt | 9 +++- .../dev/hotwire/turbo/session/TurboSession.kt | 45 +++++++++++++--- .../turbo/session/TurboSessionCallback.kt | 3 +- .../hotwire/turbo/session/TurboSessionTest.kt | 13 ++++- 5 files changed, 96 insertions(+), 25 deletions(-) diff --git a/turbo/src/main/assets/js/turbo_bridge.js b/turbo/src/main/assets/js/turbo_bridge.js index 2d210cdb..bd4f61af 100644 --- a/turbo/src/main/assets/js/turbo_bridge.js +++ b/turbo/src/main/assets/js/turbo_bridge.js @@ -98,18 +98,18 @@ // Adapter interface visitProposedToLocation(location, options) { - if (window.Turbo && Turbo.navigator.locationWithActionIsSamePage(location, options.action)) { - // Scroll to the anchor on the page - TurboSession.visitProposalScrollingToAnchor(location.toString(), JSON.stringify(options)) - Turbo.navigator.view.scrollToAnchorFromLocation(location) - } else if (window.Turbo && Turbo.navigator.location?.href === location.href) { - // Refresh the page without native proposal - TurboSession.visitProposalRefreshingPage(location.toString(), JSON.stringify(options)) - this.visitLocationWithOptionsAndRestorationIdentifier(location, JSON.stringify(options), Turbo.navigator.restorationIdentifier) - } else { - // Propose the visit - TurboSession.visitProposedToLocation(location.toString(), JSON.stringify(options)) - } + if (window.Turbo && Turbo.navigator.locationWithActionIsSamePage(location, options.action)) { + // Scroll to the anchor on the page + TurboSession.visitProposalScrollingToAnchor(location.toString(), JSON.stringify(options)) + Turbo.navigator.view.scrollToAnchorFromLocation(location) + } else if (window.Turbo && Turbo.navigator.location?.href === location.href) { + // Refresh the page without native proposal + TurboSession.visitProposalRefreshingPage(location.toString(), JSON.stringify(options)) + this.visitLocationWithOptionsAndRestorationIdentifier(location, JSON.stringify(options), Turbo.navigator.restorationIdentifier) + } else { + // Propose the visit + TurboSession.visitProposedToLocation(location.toString(), JSON.stringify(options)) + } } // Turbolinks 5 @@ -134,8 +134,18 @@ this.loadResponseForVisitWithIdentifier(visit.identifier) } - visitRequestFailedWithStatusCode(visit, statusCode) { - TurboSession.visitRequestFailedWithStatusCode(visit.identifier, visit.hasCachedSnapshot(), statusCode) + async visitRequestFailedWithStatusCode(visit, statusCode) { + // Turbo does not permit cross-origin fetch redirect attempts and + // they'll lead to a visit request failure. Attempt to see if the + // visit request failure was due to a cross-origin redirect. + const redirect = await this.fetchFailedRequestCrossOriginRedirect(visit, statusCode) + const location = visit.location.toString() + + if (redirect != null) { + TurboSession.visitProposedToCrossOriginRedirect(location, redirect.toString(), visit.identifier) + } else { + TurboSession.visitRequestFailedWithStatusCode(location, visit.identifier, visit.hasCachedSnapshot(), statusCode) + } } visitRequestFinished(visit) { @@ -168,6 +178,19 @@ // Private + async fetchFailedRequestCrossOriginRedirect(visit, statusCode) { + if (statusCode <= 0) { + try { + const response = await fetch(visit.location, { redirect: "follow" }) + if (response.url != null && response.url.origin != visit.location.origin) { + return response.url + } + } catch {} + } + + return null + } + afterNextRepaint(callback) { if (document.hidden) { callback() diff --git a/turbo/src/main/kotlin/dev/hotwire/turbo/delegates/TurboWebFragmentDelegate.kt b/turbo/src/main/kotlin/dev/hotwire/turbo/delegates/TurboWebFragmentDelegate.kt index f7e4e3bb..b60c8fa1 100644 --- a/turbo/src/main/kotlin/dev/hotwire/turbo/delegates/TurboWebFragmentDelegate.kt +++ b/turbo/src/main/kotlin/dev/hotwire/turbo/delegates/TurboWebFragmentDelegate.kt @@ -10,6 +10,7 @@ import androidx.lifecycle.findViewTreeLifecycleOwner import androidx.lifecycle.lifecycleScope import androidx.lifecycle.whenStateAtLeast import dev.hotwire.turbo.config.pullToRefreshEnabled +import dev.hotwire.turbo.errors.TurboVisitError import dev.hotwire.turbo.fragments.TurboWebFragmentCallback import dev.hotwire.turbo.nav.TurboNavDestination import dev.hotwire.turbo.nav.TurboNavigator @@ -21,7 +22,6 @@ import dev.hotwire.turbo.views.TurboView import dev.hotwire.turbo.views.TurboWebView import dev.hotwire.turbo.visit.TurboVisit import dev.hotwire.turbo.visit.TurboVisitAction -import dev.hotwire.turbo.errors.TurboVisitError import dev.hotwire.turbo.visit.TurboVisitOptions import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -233,6 +233,13 @@ internal class TurboWebFragmentDelegate( navigator.navigate(location, options) } + override fun visitProposedToCrossOriginRedirect(location: String) { + // Pop the current destination from the backstack since it + // resulted in a visit failure due to a cross-origin redirect. + navigator.navigateBack() + navigator.navigate(location, TurboVisitOptions()) + } + override fun visitNavDestination(): TurboNavDestination { return navDestination } diff --git a/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSession.kt b/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSession.kt index 82ad6649..42144997 100644 --- a/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSession.kt +++ b/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSession.kt @@ -11,7 +11,8 @@ import androidx.lifecycle.lifecycleScope import androidx.webkit.WebResourceErrorCompat import androidx.webkit.WebViewClientCompat import androidx.webkit.WebViewCompat -import androidx.webkit.WebViewFeature.* +import androidx.webkit.WebViewFeature.VISUAL_STATE_CALLBACK +import androidx.webkit.WebViewFeature.isFeatureSupported import dev.hotwire.turbo.config.TurboPathConfiguration import dev.hotwire.turbo.config.screenshotsEnabled import dev.hotwire.turbo.delegates.TurboFileChooserDelegate @@ -21,12 +22,14 @@ import dev.hotwire.turbo.errors.WebError import dev.hotwire.turbo.errors.WebSslError import dev.hotwire.turbo.http.* import dev.hotwire.turbo.nav.TurboNavDestination -import dev.hotwire.turbo.util.* +import dev.hotwire.turbo.util.isHttpGetRequest +import dev.hotwire.turbo.util.logEvent +import dev.hotwire.turbo.util.runOnUiThread +import dev.hotwire.turbo.util.toJson import dev.hotwire.turbo.views.TurboWebView import dev.hotwire.turbo.visit.TurboVisit import dev.hotwire.turbo.visit.TurboVisitAction import dev.hotwire.turbo.visit.TurboVisitOptions -import kotlinx.coroutines.* import java.util.* /** @@ -197,6 +200,33 @@ class TurboSession internal constructor( callback { it.visitProposedToLocation(location, options) } } + /** + * Called by Turbo bridge when a cross-origin redirect visit is proposed. + * + * Warning: This method is public so it can be used as a Javascript Interface. + * You should never call this directly as it could lead to unintended behavior. + * + * @param location The original visit location requested. + * @param redirectLocation The cross-origin redirect location. + * @param visitIdentifier A unique identifier for the visit. + */ + @JavascriptInterface + fun visitProposedToCrossOriginRedirect( + location: String, + redirectLocation: String, + visitIdentifier: String + ) { + logEvent("visitProposedToCrossOriginRedirect", + "location" to location, + "redirectLocation" to redirectLocation, + "visitIdentifier" to visitIdentifier + ) + + if (visitIdentifier == currentVisit?.identifier) { + callback { it.visitProposedToCrossOriginRedirect(redirectLocation) } + } + } + /** * Called by Turbo bridge when a new visit proposal will refresh the * current page. @@ -284,20 +314,19 @@ class TurboSession internal constructor( * @param statusCode The HTTP status code that caused the failure. */ @JavascriptInterface - fun visitRequestFailedWithStatusCode(visitIdentifier: String, visitHasCachedSnapshot: Boolean, statusCode: Int) { + fun visitRequestFailedWithStatusCode(location: String, visitIdentifier: String, visitHasCachedSnapshot: Boolean, statusCode: Int) { val visitError = HttpError.from(statusCode) logEvent( "visitRequestFailedWithStatusCode", + "location" to location, "visitIdentifier" to visitIdentifier, "visitHasCachedSnapshot" to visitHasCachedSnapshot, "error" to visitError ) - currentVisit?.let { visit -> - if (visitIdentifier == visit.identifier) { - callback { it.requestFailedWithError(visitHasCachedSnapshot, visitError) } - } + if (visitIdentifier == currentVisit?.identifier) { + callback { it.requestFailedWithError(visitHasCachedSnapshot, visitError) } } } diff --git a/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionCallback.kt b/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionCallback.kt index 0bbaa01e..cb8c905d 100644 --- a/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionCallback.kt +++ b/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionCallback.kt @@ -1,8 +1,8 @@ package dev.hotwire.turbo.session import android.webkit.HttpAuthHandler -import dev.hotwire.turbo.nav.TurboNavDestination import dev.hotwire.turbo.errors.TurboVisitError +import dev.hotwire.turbo.nav.TurboNavDestination import dev.hotwire.turbo.visit.TurboVisitOptions internal interface TurboSessionCallback { @@ -19,6 +19,7 @@ internal interface TurboSessionCallback { fun visitCompleted(completedOffline: Boolean) fun visitLocationStarted(location: String) fun visitProposedToLocation(location: String, options: TurboVisitOptions) + fun visitProposedToCrossOriginRedirect(location: String) fun visitNavDestination(): TurboNavDestination fun formSubmissionStarted(location: String) fun formSubmissionFinished(location: String) diff --git a/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionTest.kt b/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionTest.kt index 86ce02df..47221757 100644 --- a/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionTest.kt +++ b/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionTest.kt @@ -75,6 +75,17 @@ class TurboSessionTest { verify(callback).visitProposedToLocation(newLocation, options) } + @Test + fun visitProposedToCrossOriginRedirectFiresCallback() { + val location = "${visit.location}/page" + val redirectLocation = "https://example.com/page" + + session.currentVisit = visit + session.visitProposedToCrossOriginRedirect(location, redirectLocation, visit.identifier) + + verify(callback).visitProposedToCrossOriginRedirect(redirectLocation) + } + @Test fun visitStartedSavesCurrentVisitIdentifier() { val visitIdentifier = "12345" @@ -105,7 +116,7 @@ class TurboSessionTest { val visitIdentifier = "12345" session.currentVisit = visit.copy(identifier = visitIdentifier) - session.visitRequestFailedWithStatusCode(visitIdentifier, true, 500) + session.visitRequestFailedWithStatusCode(visit.location, visitIdentifier, true, 500) verify(callback).requestFailedWithError( visitHasCachedSnapshot = true, From 3f9ff5a8141e9f92057e2527992b67ff62e2aece Mon Sep 17 00:00:00 2001 From: Jay Ohms Date: Thu, 28 Mar 2024 12:21:25 -0400 Subject: [PATCH 2/2] Add comment clarifying status codes --- turbo/src/main/assets/js/turbo_bridge.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/turbo/src/main/assets/js/turbo_bridge.js b/turbo/src/main/assets/js/turbo_bridge.js index bd4f61af..ff439f61 100644 --- a/turbo/src/main/assets/js/turbo_bridge.js +++ b/turbo/src/main/assets/js/turbo_bridge.js @@ -179,6 +179,8 @@ // Private async fetchFailedRequestCrossOriginRedirect(visit, statusCode) { + // Non-HTTP status codes are sent by Turbo for network + // failures, including cross-origin fetch redirect attempts. if (statusCode <= 0) { try { const response = await fetch(visit.location, { redirect: "follow" })