Side navigation
#7465 closed bug (fixed)
Opened November 10, 2010 10:37PM UTC
Closed January 09, 2011 05:24AM UTC
Last modified March 13, 2012 04:32PM UTC
Incorrect checking for cross-site XHR
| Reported by: | oryol | Owned by: | oryol |
|---|---|---|---|
| Priority: | low | Milestone: | 1.5 |
| Component: | ajax | Version: | 1.4.4rc |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
I already created http://bugs.jquery.com/ticket/6873 but it was incorrectly closed as duplicate of http://bugs.jquery.com/ticket/6908. But really these issues are completely different.
Let me explain with sort example:
<script src="http://code.jquery.com/jquery-1.4.3.min.js"></script>
<script>
var url = 'http://' + window.location.hostname;
$.get(url);
$.get(url + ':80');
</script>
1. Deploy such page to some web server (on 80 port)
2. Open page in a browser and see requests
3. Two requests will be made
Expected result: both requests should have X-Requested-With header.
Actual result: only one request (second in platform preview versions of IE9 and first in other browsers) will have this header.
Main problem in this line:
remote = parts && (parts[1] && parts[1] !== location.protocol || parts[2] !== location.host);
Default web port (80) can be presented in window.location.host or not (depends on the browser) also it can (or not)be presented in the URL for AJAX request. So condition parts[2] !== location.host is incorrect if parts[2] (url of request) or location.host (current url reported by browser) contains default port (:80) but not both.
Attachments (0)
Change History (8)
Changed November 10, 2010 10:41PM UTC by comment:1
Changed November 11, 2010 02:45PM UTC by comment:2
| component: | unfiled → ajax |
|---|---|
| owner: | → oryol |
| priority: | undecided → low |
| status: | new → pending |
Do you have any insight into what the correct check on that line might be?
Changed November 11, 2010 06:29PM UTC by comment:3
| _comment0: | Easy way (from my point of view) is to split this expression: \ 1. 'Normalize' hosts (if it doesn't ends with ':80' - add ':80'). For both: parts[2] and location.host. Also \ 2. Compare normalized hosts (if they aren't equal then this request is remote). \ \ Aslo I want to note that my research is based on the current version of jQuery (1.4.3). In latest version this expression is slightly changed: toLowerCase has been added (but it really doesn't related to this bug). → 1289519999062672 |
|---|---|
| status: | pending → new |
Easy way (from my point of view) is to split this expression:
1. 'Normalize' hosts (if it doesn't ends with ':80' - add ':80'). For both: parts[2] and location.host.
2. Compare normalized hosts (if they aren't equal then this request is remote).
Aslo I want to note that my research is based on the current version of jQuery (1.4.3). In latest version this expression is slightly changed: toLowerCase has been added (but it really doesn't related to this bug).
Changed November 11, 2010 10:40PM UTC by comment:4
| _comment0: | This seems like a real edge case. Does anyone else think it’s not worth fixing? I feel like it’s not worth fixing. \ \ Also, please don’t open new bugs if your old ones are closed. → 1289515260983422 |
|---|---|
| keywords: | → needsreview |
| version: | 1.4.3 → 1.4.4rc |
This seems like a real edge case. Does anyone else think it’s not worth fixing? I feel like it’s not worth fixing.
Also, please don’t open new bugs if your old ones are closed. Best to comment on the original if you think it has been closed improperly.
Changed November 11, 2010 11:59PM UTC by comment:5
Main problem that as result 'normal' URL (without explicitly specified port) don't work correctly in IE Platform Preview (I'm not sure - maybe this behavior will be changed in the release but anyway). On the server side we don't have X-Requested-With header so requests looks like 'not ajax' and handled in other way then it really should be handled.
About opening new bugs - previous was closed with incorrect reason (obviously it isn't duplicate of 6908). If it would be closed with some correct reason (even "won't fix" or some other that exists in this tracker) obviously I wouldn't like to open new bug (in worst case I can create some workaround). And as I didn't find any method to reopen my previous bug I have created a new one.
Changed December 27, 2010 10:37PM UTC by comment:6
| keywords: | needsreview → needsreview,ajaxrewrite |
|---|
Changed January 09, 2011 05:24AM UTC by comment:7
| resolution: | → fixed |
|---|---|
| status: | new → closed |
Fixes #7465. Reworked the regexp and associated test for cross-domain detection so that it now includes ports. Added cross-domain detection tests for protocol, hostname and port.
Changeset: afefb4f3d28f47c0a93cc9dfddfcbadb595a8efb
Changed January 30, 2011 12:59AM UTC by comment:8
| keywords: | needsreview,ajaxrewrite |
|---|
http://jsfiddle.net/Qf8RY/ - demo (see request headers of two requests)