Side navigation
#8138 closed bug (fixed)
Opened February 02, 2011 10:11AM UTC
Closed February 02, 2011 07:53PM UTC
Last modified December 11, 2014 10:57PM UTC
Make$.ajax() call more error proof
Reported by: | marantz | Owned by: | jaubourg |
---|---|---|---|
Priority: | high | Milestone: | 1.5.1 |
Component: | ajax | Version: | 1.5 |
Keywords: | Cc: | jaubourg | |
Blocked by: | Blocking: |
Description
$.ajax() appear to be seriously flawed after the 1.4.3 rewrite
Please find this line in jQuery 1.5:
protocol = loc.protocol || "http:",
'document.location.protocol' is not always safe to read, so it's better to completely error protect this line or an unnecessary (access denied) exception will be thrown in some rare cases.
The bug appear to hit expecially IE6-7-8 but not in all machines, and at random.
Docs says that after you set document.domain = "foo.com" the protocol property become write-only and any attempt to access it may fail
If "http:" is a valid default for protocol, i think the exception should not be here in initialization, but at least after the call is really made. So i belive this simple fix (error protect this line of cone) will be accepted and integrated in next jQuery release.
Best regards,
Paolo Marani
Attachments (0)
Change History (15)
Changed February 02, 2011 11:54AM UTC by comment:1
cc: | → jaubourg |
---|---|
component: | unfiled → ajax |
owner: | → marantz |
priority: | undecided → high |
status: | new → pending |
Changed February 02, 2011 03:24PM UTC by comment:3
status: | pending → new |
---|
Sorry, i was not able to use jsFiddle to make a testcase, my application is rather complex with $.ajax calls inside iframes. But i can debug using IE8 developer toolbar, and i ensure you that window.location object is readonly, and location.protocol is writeonly, thus $.ajax call will fail because this restrictions. FF and CHROME works just fine, and window.location.protocol can be readed without throwing the "access denied" error. The problem is in IE7-8 only, and occurs at random under high stress. I still think that the best way to prevent this kind of errors is to try catch all read to windows.location properties, expecially in the "loc.protocol" line position inside jQuery core, as already pointed out.
Changed February 02, 2011 04:32PM UTC by comment:4
owner: | marantz → jaubourg |
---|---|
status: | new → assigned |
Replying to [comment:3 marantz]:
Sorry, i was not able to use jsFiddle to make a testcase, my application is rather complex with $.ajax calls inside iframes. But i can debug using IE8 developer toolbar, and i ensure you that window.location object is readonly, and location.protocol is writeonly, thus $.ajax call will fail because this restrictions. FF and CHROME works just fine, and window.location.protocol can be readed without throwing the "access denied" error. The problem is in IE7-8 only, and occurs at random under high stress. I still think that the best way to prevent this kind of errors is to try catch all read to windows.location properties, expecially in the "loc.protocol" line position inside jQuery core, as already pointed out.
Silencing the exception is not enough, we still need to get the protocol and all the other location properties in order to determine if a request is cross-domain or not. Do you know of any documentation regarding this specifity of IE and ways to figure out the page's protocol in this case?
We want this fixed but we can't fix it like you propose because it will just move the problem elsewhere.
Changed February 02, 2011 04:53PM UTC by comment:5
Well, i have googled around but i've found only few articles (like the one pointed in first post) that state location is "obviously" supposed to be readonly and all properties become writeonly, when document.domain does not match. By the way, i'm absolutely certain that the url is not x-site, because in that case it would break 100% time. Also, i have tried to compare current $.ajax() implementation with the previous 1.4.2 (that ALWAYS WORKS), but they completely differs, and nowhere (at least i has not been able to found where) the .protocol property is accessed directly.
One customer (Using XP and IE8) reported back that setting all IE security settings to default have cured the occurrence of this random crash so far. So i think have something to do with arcane security settings that may be applied to IE into "intranet sites" area. All url are intranet.
We will try to temporary use the developer version of 1.5 with a try catch added, just to know if the crash is postponed elsewhere or not. At the last resort i will revert back to 1.4.2 that DO work rocksolid without any hiccup (but i would like to stay on 1.5 for other minor advantages).
Thank you for your great support. I still think that in case of an internal "access denied" error inside $.ajax it would be better to forward the error to external error callback handler appropriately, and not internally crash (so i can at least implement a retry alghoritm before giving up).
Changed February 02, 2011 05:40PM UTC by comment:6
Just a quick question, do you set document.domain before or after loading jQuery? Because if it was possible for you to load it before, then we could store location.protocol at load time and not have to access it once document.domain is set.
That is:
<script src="....jQuery.1.5..."></script> <script> document.domain = "...."; </script>
Rather than:
<script> document.domain = "...."; </script> <script src="....jQuery.1.5..."></script>
As to potential problems with your proposed patch, well, you'll need to at least set the protocol variable to your environment protocol ("http:" or "https:" or else you requests will all be considered cross-domain).
Changed February 02, 2011 07:53PM UTC by comment:7
resolution: | → fixed |
---|---|
status: | assigned → closed |
Fixes #8138. Access to document.location is made only once at load time and if it fails (throwing an exception in IE when document.domain is already set), we use the href of an A element instead.
Changeset: e3cc440934fcb03bfeb10fb6281615409ad6f483
Changed February 02, 2011 07:57PM UTC by comment:8
marantz, can you confirm git0 works in your environment?
Changed February 02, 2011 07:57PM UTC by comment:9
milestone: | 1.next → 1.5.1 |
---|
Changed February 03, 2011 09:09AM UTC by comment:10
Replying to [comment:6 jaubourg]:
Just a quick question, do you set document.domain before or after loading jQuery? Because if it was possible for you to load it before, then we could store location.protocol at load time and not have to access it once document.domain is set. That is:> <script src="....jQuery.1.5..."></script> > <script> > document.domain = "...."; > </script> >Rather than:> <script> > document.domain = "...."; > </script> > <script src="....jQuery.1.5..."></script> >As to potential problems with your proposed patch, well, you'll need to at least set the protocol variable to your environment protocol ("http:" or "https:" or else you requests will all be considered cross-domain).
No, in my code there are no explicit set of document.domain, mainly because is supposed there are no cross-site scripting at all.
Changed February 03, 2011 09:13AM UTC by comment:11
Replying to [comment:8 jaubourg]:
marantz, can you confirm git0 works in your environment? http://code.jquery.com/jquery-git.js
I am performing some tests using jit0 and so far it's running as expected, a conclusive response after few days of use (of course), but so far im very satisfied, both for the superb jQuery quality and also for your fantastic support. Great job!
Changed February 04, 2011 10:17AM UTC by comment:12
I can conform that no more "access denied" error have been reported after applying the patch, so i consider the case closed.
Great job guys!
Replying to [comment:11 marantz]:
Replying to [comment:8 jaubourg]: > marantz, can you confirm git0 works in your environment? > > http://code.jquery.com/jquery-git.js I am performing some tests using jit0 and so far it's running as expected, a conclusive response after few days of use (of course), but so far im very satisfied, both for the superb jQuery quality and also for your fantastic support. Great job!
Changed July 12, 2014 08:41PM UTC by comment:13
Ajax: Remove workaround for IE6/7
Closes gh-1597
Ref #8138
Changeset: e5190982c40d7ac8ab9bdb2e7e4334f0e123ef66
Changed December 11, 2014 10:50PM UTC by comment:14
Ajax: Remove workaround for IE6/7
Closes gh-1597
Ref #8138
Changeset: d72e18c813aff4b101db78f58d397f9c117df0fe
Changed December 11, 2014 10:57PM UTC by comment:15
Ajax: Remove workaround for IE6/7
(cherry-picked from e5190982c40d7ac8ab9bdb2e7e4334f0e123ef66)
Closes gh-1597
Ref #8138
Changeset: ad6e422c85367844a917997dadaf193f54ad13f1
Thanks for taking the time to contribute to the jQuery project by writing a bug report.
Please submit a reduced test case, which reproduces the issue you are experiencing, on http://jsfiddle.net. So that we can investigate this issue further.
Also include a link to the "documentation" you mention.
How to report bugs