Closed ghost closed 3 years ago
I can reproduce this in iOS 14.4.2, but this happens not just in the app, but also in iOS Safari -- something in the page is causing the web-page-loading process to crash.
If I clear the cache/cookies for tuxed.net (Settings > Safari > Advanced > Website Data > tuxed.net), then open vpn.tuxed.net in iOS Safari, it gives me a list of IdPs to choose from. If I choose an IdP (say FrKoIdP), then close the tab, open a new tab in iOS Safari, and go to vpn.tuxed.net, then the page crashes, like this:
So, I don't think this can be fixed in the app. It looks like a bug in Safari, and it should probably be worked around in the website.
Thanks for looking into this!
So, I don't think this can be fixed in the app. It looks like a bug in Safari, and it should probably be worked around in the website.
Do you know what is causing this crash? Could it be an infinite redirect loop that is aborted at some point? Do you have any idea how to debug this to figure out what the problem is and/or how to work around it?
My guess is it's not related to redirects -- it's something in the page (HTML/CSS/JS) that's causing the crash.
As I understand, the server gives different pages based on whether there are any previously used IdPs or not (I guess based on a cookie). If there are no previously used IdPs, it returns a simple list with a search box on top (say Version A). If there are previously used IdPs, it returns a list with the previously used IdPs and then an Other node that can be expanded to a search box + other IdPs (say Version B).
We know that there's a crash only if there are cookies, so I'm guessing Version B causes a crash but Version A doesn't. One way to confirm that could be to make the server return Version A even if there were cookies. If that's confirmed, then I guess we could see which part of the differences between Version A and Version B causes the crash.
A full factory reset (iPhone 6S, iOS 14.7.1) without restoring anything from backup, not even providing Apple account info does not help.
Tailing the vpn.tuxed.net server log shows that there are no requests after starting Safari (after swiping it away from the task switcher) so it crashes all by itself without sending any network requests.
Disabling JavaScript in Safari stops it from crashing!
This is a very interesting problem!
This is all the JS that is used: https://vpn.tuxed.net/php-saml-sp/js/search.js
Steps to reproduce:
At this point the page/Safari crashes.
Good find that the page doesn't crash if JavaScript is disabled. Maybe we can keep commenting out stuff from the (admittedly very minimal) JS code and try loading the page with JS enabled to see which line cause the crash?
Can we not have Apple fix this somehow? It is a (possibly exploitable?) bug in Safari...
I think we need to first identify which part of the JS is causing the crash, then check if it's a known issue -- maybe there's some info in forums / openradar / webkit bugzilla. From there, we might get to know a fix timeline, and maybe a workaround. If it's an unknown issue, we might have to file a bug with a testcase with Apple or WebKit or both (but unless it's a security issue, I'd say we'll be lucky if it gets fixed in the next major iOS release).
I mean, I don't work for Apple, I think I spent enough time on this issue already. Apple engineers can take it from here. I can create a bug report somewhere of course with the steps to reproduce (as above) if that helps.
I did the comment-out-and-check thing and found out that the line where it crashes is: searchBoxInput.focus();
.
Googling for it led me to this comment on HN, and from there to webkit bug 225259. That bug is fixed in changeset 276901 whose commit message indicates it's related to autofocus. I tried to remove the autofocus in our HTML and could see that that avoids the crash too.
So, as a workaround, we could remove either the autofocus on the previously used IdP (bad), or conditionally don't focus the search box under the "Other" (not so bad) for iOS only maybe?. The bug indicates it's macOS too, but not sure which version -- I don't see the crash in macOS 10.15.7 and on macOS 11.4.
I don't know when this webkit fix will ship with iOS Safari.
Wow @roop amazing work debugging this!
Perhaps on macOS you don't see this bug because the window is big enough. What if you reduce the macOS Safari window size to mobile size?
The production server nl.eduvpn.org also uses php-saml-sp with this autofocus thing. The good news is this is not triggered because the php-saml-sp WAYF is never shown to users of the iOS app. Only when the user uses Safari to manually download a configuration file on nl.eduvpn.org this is triggered. As it is already fixed and will eventually be part of a released Safari we're good I guess.
Thanks @fkooman 😀.
Reducing the macOS Safari window size doesn't trigger the crash. Using responsive design mode to make the window iPhone-sized doesn't trigger the crash either.
Only when the user uses Safari to manually download a configuration file on nl.eduvpn.org this is triggered. As it is already fixed and will eventually be part of a released Safari we're good I guess.
So any user on iOS 14 (which is most iOS users) trying to download a config would hit this bug. And if they hit this bug, they wouldn't really know how to make it work. So if downloading a config from an iOS device (or managing the connection like revoking or something) is something that we want to support, don't you think it's better this is addressed in the server?
Not fixed in 14.8. If it is still broken in 15 I'll implement a work-around.
Seems to have been fixed in iOS 15, can't get Safari to crash.
I have to all the time clear Safari's cache to be able to authorize vpn.tuxed.net with the new Let's Connect! app. After clearing the cache once, it works great, the next time authorization is required it fails.
iOS: 14.7.1 App: Let's Connect! 2.2.3 (1504)