Page MenuHomePhabricator

Restbase redirects with cors not working on Android 4 native browser
Closed, ResolvedPublic

Description

When loading a restbase url that is a redirect, the request fails. Non-redirect urls seem to work fine.

Related task: T148368: Prototypes don't work on Safari

Info

  • Device: Galaxy Nexus
  • OS: Android 4.1.1
  • UA: Mozilla/5.0 (Linux; U; Android 4.1.1; en-us; Galaxy Nexus Build/JRO03C) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30

SSCCE

<!DOCTYPE html>
<html>
<head>
<meta name="description" content="mobile sections api call">
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
	<style>
	html, body, pre { width: 100%; height: 100%; }
	pre { overflow: auto; padding: 1em; }
	</style>
</head>
<body>
  <pre></pre>

	<script>
		var xhr = new XMLHttpRequest();
		xhr.addEventListener("load", onLoad);
		xhr.addEventListener("error", setError);
		xhr.open('GET', 'https://en.wikipedia.beta.wmflabs.org/api/rest_v1/page/mobile-sections/User%3APchelolo%2FRedirect%20Test?redirect=true', true);
		xhr.send(null);

		function onLoad() {
			setJSON(JSON.parse(this.responseText))
		}

		function setJSON (json) {
			document.querySelector('pre').innerHTML = JSON.stringify(json, null, 2)
		}
		function setError (e) {
			console.log(e)
			document.querySelector('pre').innerHTML = e.message + '\n' + e.stack
		}
	</script>
</body>
</html>

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 27 2016, 10:29 AM

I can't provide more information right now since the browser is terrible, and there are no devtools for it.

I suspect the issue will probably be the same as with the old safaris in the related task (them sending random headers on the options request before the redirect and the allow-headers not being permissive enough) but I haven't confirmed it.

Pchelolo claimed this task.Oct 27 2016, 2:57 PM
Pchelolo edited projects, added Services (doing); removed Services.

Hm.. Looks like I need to get a working android emulator after all. I'll do that and will investigate the issue.

So, I've run an android emulator and set up a proxy to capture the request:

	GET /api/rest_v1/page/mobile-sections/User%3APchelolo%2FRedirect%20Test?redirect=true HTTP/1.1
Host	en.wikipedia.beta.wmflabs.org
Connection	keep-alive
Referer	http://safari-restbase-cors.surge.sh/
Origin	http://safari-restbase-cors.surge.sh
X-Requested-With	com.android.browser
User-Agent	Mozilla/5.0 (Linux; U; Android 4.1.2; en-us; Android SDK built for x86 Build/MASTER) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30
Accept-Encoding	gzip,deflate
Accept-Language	en-US
Accept-Charset	utf-8, iso-8859-1, utf-16, *;q=0.7

So, out of the weird headers we have Accept-Charset and an X-Requested-With. I suppose the latter makes it do a preflight and we hit the same CORS redirect support bug. Lemme try to add these to the list of allowed headers and check if that helps.

Pchelolo added a comment.EditedOct 27 2016, 8:13 PM

I seems like a bug in the browser CORS support. I've tried to add these 2 additional headers in our Access-Control-Allow-Headers list, but that didn't change anything. Surprisingly, intercepting the network activity shows that there's NO preflight request being made, however the browser blocks redirects for some reason as if the request was preflighted.

I don't really know if we can do something on our side, @Jhernandez.. The only idea I have is to implement a fallback for redirect pages, that, in case normal request failed, would fetch the page with ?redirect=false, inspect the Location header and redirect manually. How much do we care about this particular browser? Android 4 is quite old, isn't it?

Also, you should drop the ?redirect=true part. Following the redirect is the default behaviour and by explicitly adding it you're fragmenting the caches.

Responses inline

I seems like a bug in the browser CORS support. I've tried to add these 2 additional headers in our Access-Control-Allow-Headers list, but that didn't change anything. Surprisingly, intercepting the network activity shows that there's NO preflight request being made, however the browser blocks redirects for some reason as if the request was preflighted.

That's very very weird

I don't really know if we can do something on our side, @Jhernandez.. The only idea I have is to implement a fallback for redirect pages, that, in case normal request failed, would fetch the page with ?redirect=false, inspect the Location header and redirect manually. How much do we care about this particular browser? Android 4 is quite old, isn't it?

Manually handling redirects can definitely be an option I guess, but it is more error prone and probably slower since it has to go to the client to handle the logic manually (in the web case to JS). It can be doable though.

According to the analytics dashboard android 4 has 29% of the visits on mobile web:

2015-08-23 Android 4 29.2%

Split by browser, the android browser on Android 4 seems to have a 9%

2015-07-05 Android 4 9.1%

Those are significant chunks of our visits and if we were to use restbase for mobile web we would need to support it for sure.

Right now the issue is for prototypes only since we don't query restbase from production sites right now, but that should hopefully change in the near future (using the summary endpoint for hovercards on desktop, for example).

Also, you should drop the ?redirect=true part. Following the redirect is the default behaviour and by explicitly adding it you're fragmenting the caches.

Sure, done


I've done some investigation myself and modified the sscce to support proxying the ajax request via a cors proxy of mine (based on cors-anywhere to see if the issue was with the headers and the preflight or not.

The result is that this url:

Which queries via ajax this url:

Works just fine in the device I mentioned in the description.

Which makes me think that we could fix the restbase config if we wanted to to make it work (even if it is painful to figure it out).


As I mentioned before, right now this is just for some user research prototypes, so I can proxy my requests to restbase through my proxy and get the job done for the user testing phase, but it may be worth fixing this looking forward to broader restbase usage on our main web properties.

BTW @Pchelolo thanks for looking into this, I know how painful and annoying this stuff is 🍪

@Jhernandez Hm, interesting, the CORS proxy of yours have some more interesting headers that might be related like X-CORS-Redirect-1 that I've never heard about. Let me try to hack up RESTBase to expose all of those headers and figure out what we need to add to RB.

Aha, I get what's happening here. The CORS proxy is actually following the redirect for you and returning the content of the redirect target with 200 code and adds a custom header X-CORS-Redirect-1, that's why it doesn't hit the bug with redirects...

What we could do in RESTBase is check the Origin header, if it's set and if it's different from the current domain, we would also follow the redirect inside RESTBase and return the content, but we would have to disable Varnish caching for responses like this, because we cannot purge the returned content (since it's been accessed my the redirect source URI while the content is normally referenced by the redirect target URI). This would allow the prototypes with CORS work, and in production it will be requested non cross-domain, so lack of caching wouldn't be a problem)

@GWicke what do you think about this plan?

GWicke added a subscriber: BBlack.EditedOct 28 2016, 6:46 PM

@Pchelolo, this sounds a bit like the "follow redirects in Varnish" idea we discussed a while ago. By doing this in Varnish, this solution could apply more generally to all CORS redirects, and we might even be able to preserve caching.

The other issue we found with CORS redirects is the inability to figure out the final URL from fetch or XHR responses (security limitation). To resolve this issue, we could provide a Content-Location header (possibly even in all responses, redirected or not), and then allow access to that from CORS requests.

@BBlack, what are your thoughts on following CORS redirects on behalf of clients in the Varnish layer?

Restricted Application added a project: Operations. · View Herald TranscriptOct 28 2016, 7:35 PM
ema moved this task from Triage to Caching on the Traffic board.Oct 31 2016, 10:52 AM

@Pchelolo and I discussed this a bit in person, and decided to go with a RESTBase-only solution for now. Concretely, we will:

  1. Set Vary: Origin on all redirect responses from the REST API`.
  2. When processing a CORS request that would result in a redirect response, follow that redirect inside RESTBase, and return the final content with content-location headers (now set on all resources, and *no-cache headers*. We expect the volume of CORS redirects to be relatively modest, so not caching those should not be a significant performance issue.

If we later want to avoid the need to disable caching & generalize CORS redirects to work for other entry points (like MediaWiki) too, then it would probably make sense to add the redirect-following logic in Varnish itself.

Created a PR to fix it, after review will test it in labs, hopefully it's gonna fix everything: https://github.com/wikimedia/restbase/pull/702

Pchelolo closed this task as Resolved.Nov 10 2016, 7:20 PM

Merged and deployed. Resolving. Hopefully we've got all of the CORS edge cases now.