331825 - Unable to do conditional gets via XMLHttpRequest with 1.5.0.1, works in 1.0.7

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect)

Description

[Yusuf Goolamabbas]

Hi, I was trying to write a simple ajaxy application to learn Mochikit. The URL above calls some simple JS functions which get a static file periodically and print the HTTP status response code. 

I use the If-Modified-Since and If-None-Match header fields to simulate a conditional get. The expectation is that the the alert window will show 200 the first time and then 304 subsequently. This behaviour is demonstrated with Internet Explorer 6.0/Windows and Firefox 1.0.7 on MacOSX. However if I use Firefox 1.5.0.1 on Windows 2000/XP and MacOSX I am only shown '200' in the alert box on the first time and then on subsequent call outs.

Comment 1

[Yusuf Goolamabbas]

If I change the pref network.http.use-cache from true to false. Firefox behaves as expected. I have changed the component to networking. 

Comment 2

[Yusuf Goolamabbas]

sorry for bugspam, really meant networking:cache

Comment 3

[Yusuf Goolamabbas]

I've also filed a corresponding bug in WebKit's bugzilla as well as via Apple's Radar

http://bugzilla.opendarwin.org/show_bug.cgi?id=8210

Comment 4

[Boris Zbarsky [:bzbarsky]]

> I have changed the component to networking.

You want to change the assignee when you do that if you expect someone to notice....

Comment 5

[Darin Fisher]

So, what is the intended behavior of XMLHttpRequest?  As I understand it, both the W3C and WhatWG have made efforts to standardize XMLHttpRequest.  Is there a spec for what should happen if these request headers are set?

Comment 6

[Hixie (not reading bugmail)]

anne: please ensure your spec covers this. If it already does, please provide a link to the relevant section. Thanks!

Comment 7

[Anne (:annevk)]

Look for HTTP cache in http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/publish/XMLHttpRequest.html#dfn-send

Please send comments to [e-mail].

Comment 8

[Darin Fisher]

OK, well that certainly supports this bug.

-> Core: Networking: HTTP

Comment 9

[Daniel Veditz [:dveditz]]

Would consider a fix for this regression in 1.8.0.5, but it's not going to write itself assigned to [e-mail] -- anyone actively working on this?

Comment 10

[Daniel Veditz [:dveditz]]

Would need a tested trunk and 1.8 branch patch before we can consider it for the 1.8.0 branch. Not blocking, but if you get such a patch you can ask for approval on it and we'll consider it.

Comment 11

[Mike Schroepfer]

Darin - any thoughts on what the right fix is here?

Comment 12

[Anne (:annevk)]

Draft is now located at http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/Overview.html?content-type=text/html;%20charset=utf-8#dfn-send ... This hasn't changed though.

Comment 13

[Darin Fisher]

I can reproduce the behavior difference  FF1.0 and FF1.5.  I'm investigating to see what changed.

Comment 14

[Darin Fisher]

OK, this bug appears to be caused by the fact that we no longer send 'Pragma: no-cache' and 'Cache-control: no-cache' with each XMLHttpRequest GET.  I didn't realize that Firefox 1.0 did that, but it seems like a good thing that Firefox 1.5 does not.  The 304 is still happening, however, it is just happening transparently to the application.  Necko reports a 304 as a 200 since it is able to re-use its cached copy.  Perhaps it should realize that the consumer is initiating the conditional request and skip trying to use its own cache.

Comment 15

[Darin Fisher]

Here's the old code where the local cache was unconditionally bypassed:
http://lxr.mozilla.org/aviary101branch/source/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp#1543

The change was made in bug 268844.  The fact that conditional queries ever worked in Firefox 1.0 is simply an unintended side-effect of our support for multipart queries (which was added in bug 237319 just before we branched for Firefox 1.0).

I think this bug should be fixed inside Necko's HTTP implementation.

Comment 16

[Darin Fisher]

Attachment: v1 patch

Comment 17

[Darin Fisher]

Attachment: corresponding regression test

Comment 18

[Darin Fisher]

Attachment: v1.1 patch

Here's a slightly better variant on the same patch that biesi suggested.

Comment 19

[Darin Fisher]

fixed-on-trunk

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What effects (if any) might this have on stuff other than XMLHttpRequest?

Comment 21

[Darin Fisher]

fixed1.8.1

Comment 22

[Darin Fisher]

Comment on attachment 226714 [details] [diff] [review]
v1.1 patch

It might be worth back-porting this fix to the 1.8.0 branch.

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 226714 [details] [diff] [review]
v1.1 patch

approved for 1.8.0 branch, a=dveditz for drivers

Comment 24

[Darin Fisher]

Grr.. the patch did not apply cleanly to the 1.8.0 branch :-(

Comment 25

[Darin Fisher]

fixed1.8.0.7

Comment 26

[alice nodelman [:alice] [:anode]]

using testcase described in comment #0

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060825 Firefox/1.5.0.7pre

verified 1.5.0.7

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/20060825 BonEcho/2.0b2

verified 1.8.1b2