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.

Sign up for GitHub

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

Jump to bottom

Deal with antiCORS proxies. #1

Closed
olaf-k opened this issue Dec 5, 2013 · 9 comments
Closed

Deal with antiCORS proxies. #1

olaf-k opened this issue Dec 5, 2013 · 9 comments

Comments

@olaf-k
Copy link

olaf-k commented Dec 5, 2013

Some proxies filter CORS request that need preflight which obviously breaks the load of AT.
See ariatemplates-plugins/calendarwidget#4 as reported by @piuccio.
@jakub-g
Copy link
Collaborator

jakub-g commented Dec 5, 2013

Looking at https://developer.mozilla.org/en/docs/HTTP/Access_control_CORS#Preflighted_requests it seems to me that the reason is that we set X-Requested-With: XMLHttpRequest in aria.core.IO. To be checked if things will work without that header.

@olaf-k
Copy link
Author

olaf-k commented Dec 5, 2013

I'm not sure, from what I understand the Rakuten proxy redirects the queries, thus impacting the Origin header, which is not accepted when making a CORS request. But I haven't got time to investigate yet (this will be a pain to reproduce actually) so I may be wrong.

@jakub-g
Copy link
Collaborator

jakub-g commented Dec 5, 2013

What I meant is, there's a preflight query required at all (also for me when I checked in Fiddler), because of AFAIU the presence of X-Requested-With request header in the XHR (which is injected in aria.core.IO). If we set a proper flag in that class to not inject this header, perhaps we can eliminate the preflight request, which will make the CDN faster apart from everything -- because there will be less HTTP requests needed -- which would be good (as long as this won't break something else).

Anyway on top of that, there might be indeed some other issues with the Rakuten proxy, maybe they're injecting some more custom headers (which is typical for proxies) which are doing harm. I'm not a CORS expert unfortunately :) Would be good if @piuccio pasted here the request headers he gets in Fiddler when requesting the multiparts for the first time (with empty cache etc).

@piuccio
Copy link

piuccio commented Dec 6, 2013

Request

OPTIONS http://cdn.ariatemplates.com/aria/pack-utils-1aa654f1d08e85570db15acf603bff8a.js HTTP/1.1
Host: cdn.ariatemplates.com
Connection: keep-alive
Cache-Control: no-cache
Access-Control-Request-Method: GET
Pragma: no-cache
Origin: http://www.ariatemplates.com
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.41 Safari/537.36
Access-Control-Request-Headers: x-requested-with
Accept: */*
Referer: http://www.ariatemplates.com/plugins/calendarwidget/
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8,it;q=0.6

Response

HTTP/1.1 302 Found
Location: http://urlf-auth.office.rakuten.co.jp/?cfru=...
Cache-Control: no-cache
Pragma: no-cache
Content-Type: text/html; charset=utf-8
Connection: close
Content-Length: 660

<HTML><HEAD>
<TITLE>Redirect</TITLE>

@olaf-k
Copy link
Author

olaf-k commented Dec 6, 2013

Yup, I already saw in the logs that we don't get OPTIONS requests from your IP, only GET.

@olaf-k
Copy link
Author

olaf-k commented Dec 10, 2013

The good news is that setting aria.core.IO.useXHRHeader=false as suggested by @jakub-g gets rid of OPTION requests.
The bad news is that you need to reset it if your app requires x-requested-with : XMLHttpRequest

@piuccio
Copy link

piuccio commented Dec 11, 2013

Apparently in angular they prefer not using this non-standard header.
And it kind of make sense.

x-requested-with is basically needed only when the same entry point must return two different results whether it's AJAX or not.
That seems like a poor design decision.

@jakub-g
Copy link
Collaborator

jakub-g commented Dec 11, 2013

Agreed with @piuccio - we may even think about removing this header from the fwk by default in the new version of AT, after making sure if it doesn't break the Big Apps using AT.

If it's not needed by default, then we're just wasting bandwidth, and small things add up [1]. We shouldn't be like IE7 ;) [2]

[1] http://chrishateswriting.com/post/68794699432/small-things-add-up
[2] http://blog.keithcirkel.co.uk/the-ups-and-downs-of-the-http-header/#acceptthebiggie

@olaf-k
Copy link
Author

olaf-k commented Dec 11, 2013

Fixed in commit f0f1934

@olaf-k olaf-k closed this as completed Dec 11, 2013
@jakub-g jakub-g mentioned this issue Jan 2, 2014
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@piuccio @olaf-k @jakub-g