hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.74k stars 430 forks source link

Chrome bug: turbo errors using Basic Auth (attempted cross-origin replaceState) #536

Open mweitzel opened 2 years ago

mweitzel commented 2 years ago

To reproduce, on any app using turbo with Chrome: visit a page using basic auth foo:bar@localhost:3000. This doesn't happen on all browsers, or when using the basic-auth prompt box. But when visiting a uri with user:info@ prefixed, it errs on chrome.

The following error occurs:

Uncaught DOMException: Failed to execute 'replaceState' on 'History': A history state object with URL 'http://localhost:3000/' cannot be created in a document with origin 'http://localhost:3000' and URL 'http://foo:bar@localhost:3000/'.
    at History.update (http://foo:bar@localhost:3000/assets/source.js:23054)
    at History.replace (http://foo:bar@localhost:3000/assets/source.js:23050)
    at History.start (http://foo:bar@localhost:3000/assets/source.js:23063)
    at Session.start (http://foo:bar@localhost:3000/assets/source.js:23623)
    at start (http://foo:bar@localhost:3000/assets/source.js:23864)
    at http://foo:bar@localhost:3000/assets/source.js:24399
    at http://foo:bar@localhost:3000/assets/source.js:23292
update @ source.js:23054
replace @ source.js:23050
start @ source.js:23036
start @ source.js:23623
start @ source.js:23864
(anonymous) @ source.js:24399
(anonymous) @ source.js:26591

Uncaught TypeError: Cannot read properties of undefined (reading 'href')
    at Session.notifyApplicationAfterPageLoad (source.js:23805)
    at Session.pageBecameInteractive (source.js:23742)
    at PageObserver.pageIsInteractive (source.js:23288)
    at PageObserver.pageIsComplete (source.js:23292)
    at HTMLDocument.PageObserver.interpretReadyState (source.js:23260)
KjellMorgenstern commented 2 years ago

It looks like this bug in chrome is triggered: https://bugs.chromium.org/p/chromium/issues/detail?id=675884

rails (7.0.3.1) turbo-rails (1.1.1)

KjellMorgenstern commented 2 years ago

One way to work around this could be to catch the error thrown by chrome when turbo calls replaceState:

const onerrorBackup = window.onerror;

window.onerror = function(msg, url, line, col, error) {
    if (error.constructor.name === "DOMException" && error.message.startsWith("Failed to execute 'replaceState'")) {
        // This is a chrome bug. See https://github.com/hotwired/turbo/issues/536
        // Solution is to navigate to the same page again but without the basic auth credentials.
        window.location.href = window.location.href + " ";
    } else {
        onerrorBackup && onerrorBackup(msg, url, line, col, error);
    }
};

This works fine locally, or when I manually call the staging deployment, but unfortunately it is not a solution in the CI. I am not sure why, maybe the CI overrides window.onerror to detect errors.

marcoroth commented 2 years ago

Would it make sense that we strip out the credentials before pushing them onto the history object? I don't see why we shouldn't do that.

KjellMorgenstern commented 2 years ago

Do you mean a call like window.location.href = window.location.href + " "; before pushing to the history? We can't strip out credentials directly, since, I think, neither in Rails nor in the DOM API this information is available. The username/password gets leaked from Chrome via the thrown error.

But catching that error directly around the history.pushState call might be a much better solution then catching it with window.onerror. It would have to be integrated to turbo, though.

marcoroth commented 2 years ago

I was more thinking something along the lines of:

const urlStringToPushToTheHistory = "https://myuser:mypassword@example.com/some/path"

const url = new URL(urlStringToPushToTheHistory)
// => URL {href: "https://myuser:mypassword@example.com/some/path", origin: "http://example.com", protocol: "https:", username: "myuser", password: "mypassword", …} = $1

url.username = ""
url.password = ""

// => URL {href: "https://example.com/some/path", origin: "http://example.com", protocol: "https:", username: "", password: "", …} = $2

url.toString()
// https://example.com/some/path

window.history.pushState({}, '', url.toString())
KjellMorgenstern commented 2 years ago

Are you sure it is not the other way round? The URL to be pushed doesn't have a authentification in the first place, and chrome mistakenly demands one.

marcoroth commented 2 years ago

Ha, maybe - yeah! But then it might be possible to something similar in reverse

KjellMorgenstern commented 2 years ago

But the reverse would be to add the authentication everywhere in the history, right? That sounds a bit odd, and I think we can not access this, at least not from javascript. From all my tests, it looks like chrome does not even include it in the request url, or maybe it is filtered by Cloudflare, nginx or others, and the javascript on client side does not have access as well.

How about catching the error?

In history.ts,

 update(method: HistoryMethod, location: URL, restorationIdentifier = uuid()) {
    const state = { turbo: { restorationIdentifier } }
    method.call(history, state, "", location.href)
    this.location = location
    this.restorationIdentifier = restorationIdentifier
  }

could be replaced with

  update(method: HistoryMethod, location: URL, restorationIdentifier = uuid()) {
    const state = { turbo: { restorationIdentifier } }
    try {
      method.call(history, state, "", location.href)
    } catch (err) {
      if (err.constructor.name === "DOMException" && 
        document.handleChrome675884) { // configure turbo, opt in to catch the error
        // This is a chrome bug. See https://github.com/hotwired/turbo/issues/536
        // Solution is to navigate to the same page again but without the basic auth credentials.
        window.location.href = window.location.href + " "
      } else {
        throw err
      }
    }

    this.location = location
    this.restorationIdentifier = restorationIdentifier
  }

Untested code (yet), I don't really know how to set up turbo-rails to use a custom version of turbo.