Bug Tracker

Modify

Ticket #8138 (closed bug: fixed)

Opened 3 years ago

Last modified 2 years ago

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
Blocking: Blocked by:

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

 http://stackoverflow.com/questions/1498788/reading-window-location-after-setting-document-domain-in-ie6

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

Change History

comment:1 Changed 3 years ago by jitter

  • Cc jaubourg added
  • Owner set to marantz
  • Status changed from new to pending
  • Component changed from unfiled to ajax
  • Priority changed from undecided to high

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

comment:2 Changed 3 years ago by jitter

#8136 is a duplicate of this ticket.

comment:3 follow-up: ↓ 4 Changed 3 years ago by marantz

  • Status changed from pending to 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.

comment:4 in reply to: ↑ 3 Changed 3 years ago by jaubourg

  • Owner changed from marantz to jaubourg
  • Status changed from new to assigned

Replying to 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.

comment:5 Changed 3 years ago by marantz

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).

comment:6 follow-up: ↓ 10 Changed 3 years ago by 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).

comment:7 Changed 3 years ago by jaubourg

  • Status changed from assigned to closed
  • Resolution set to fixed

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

comment:8 follow-up: ↓ 11 Changed 3 years ago by jaubourg

marantz, can you confirm git0 works in your environment?

 http://code.jquery.com/jquery-git.js

comment:9 Changed 3 years ago by jaubourg

  • Milestone changed from 1.next to 1.5.1

comment:10 in reply to: ↑ 6 Changed 3 years ago by marantz

Replying to 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.

comment:11 in reply to: ↑ 8 ; follow-up: ↓ 12 Changed 3 years ago by marantz

Replying to 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!

comment:12 in reply to: ↑ 11 Changed 3 years ago by marantz

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 marantz:

Replying to 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!

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.