Closed Bug 559618 Opened 11 years ago Closed 11 years ago

Firefox 3.6.3 XMLHttpRequest object reports having "No expiration time" in the server's response when valid expires and cache-control headers exist

Categories

(Core :: Networking: Cache, defect)

x86
Windows Vista
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+
status1.9.2 --- .7-fixed

People

(Reporter: jessie_cavada, Assigned: bzbarsky)

References

Details

(Keywords: regression, verified1.9.2)

Attachments

(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 (.NET CLR 3.5.30729)
Build Identifier: Firefox 3.6.3

Firefox 3.6.3 reports having "No expiration time" in the server's response when valid expires and cache-control headers exist. Below is the actual response. This is a critical problem when the expiration is in the past which is intended to instruct the browser to not cache the response. The cached response causes Firefox to display invalid data while also causing loss of data in applications where the invalid data is used to pre-fill fields which when sent back to the server, invalid data replaces valid data. The problem also occurs in Firefox 3.6.2. The problem does NOT occur in Firefox 3.5.8 and presumably earlier.

HTTP/1.1 200 OK
Cache-Control: private, max-age=-1000
Content-Type: text/plain
Content-Encoding: gzip
Expires: Wed, 14 Apr 2010 23:44:10 GMT
Vary: Accept-Encoding
Date: Thu, 15 Apr 2010 16:24:09 GMT


Reproducible: Always

Steps to Reproduce:
1. Using Firefox 3.6.3 and it's XMLHttpRequest object
2. Send a "GET" request using the XMLHttpRequest object where the response contains a valid "Expires" and "Cache-Control" header. The "Expires" header value should be a date in the past. The "Cache-Control" header should be a negative number.
3. The Firefox 3.6.3 "about:cache" should report having an "expires" date/time for this request but will instead have "No expiration time". Additionally request for the same URL will not be sent to the server but instead rendered from Firefox's cache. 
Actual Results:  
1. Request sent to server.
2. Firefox interpreted the response as having "No expiration time"
3. Additional request to the same URL answered from Firefox's cache.

Expected Results:  
1. Request sent to server.
2. Response contains a valid "Expires" and "Cache-Control" header which should be interpreted by Firefox as valid and in the past and so the results should be that Firefox should not cache the server's response.
3. Additional request to the same URL should be sent to the server.

Interpreted the response as having a valid "Expires" and "Cache-Control" header which should be interpreted by Firefox as valid and in the past and so the results should be that Firefox should not cache the server's response.

Firefox version 3.5.8 interprets the response correctly and so does NOT have the problem described in this bug report which I think is compelling proof that that Firefox's caching behavior changed after version 3.5.8.
The request shown in the image was made using the XMLHttpRequest object with a “GET” method.
Jesse, can you please point me to a URL that shows the problem?  I haven't been able to reproduce it over here so far.
And just to be sure, you're testing in safe mode or equivalent?
(In reply to comment #2)
> Jesse, can you please point me to a URL that shows the problem?  I haven't been
> able to reproduce it over here so far.

Link to page that demonstrates the bug described here.
http://www.quimby.com/dev/firefox3.6.3Cachebug.asp
(In reply to comment #3)
> And just to be sure, you're testing in safe mode or equivalent?

Problem exists in both safe mode and non-safe-mode.
It seem reasonable that this may also a privacy issue since the response is stored in the browser’s cache which can be viewed using “about:cache”
Ah, I see what's going on here, and failed to reproduce initially only due to my own failure to follow directions....  The issue is the bogus max-age value (the HTTP spec does not allow negative max-age; see RFC 2616 section 3.3.2 and section 14.9, which defines max-age as using delta-seconds as the syntax).  We end up parsing it as an integer then casting to unsigned integer, which makes a negative max-age into a very large positive number.  Large enough that we treat it as effectively infinite (though in practice it's only 127 years or so).

The behavior was changed in bug 203271, so ccing Bjarne.  Before that we used to ignore max-age in nsHttpResponseHead::ExpiresInPast, and used the expires value, which is in fact in the past here.

It's not clear to me whether it's better to treat negative max-age as not set or as 0... Jason, biesi, any thoughts?  We should certainly not be treating it as a large positive integer!

Jesse, you can deal with this in the meantime by using max-age=0, which is what you're supposed to use for "do not treat cache as valid".

Note that it WILL still be stored in the cache if you do that; none of the headers you're sending say to not cache but merely to not treat the cached content as valid without revalidating with the server.  There's nothing you can send that would prevent the data from being accessed via about:cache (or view-source, say, which uses the same underlying code).  So comment 6 isn't really related to this bug.
Blocks: 203271
Status: UNCONFIRMED → NEW
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status1.9.2: --- → ?
Ever confirmed: true
For ignoring of whole max-age=xxx when xxx is invalid.

RFC 2616 says max-age superceeds Expires: header. Does the "max-age" in this context mean existence of max-age=<valid_value>? Existence of max-age=? Or existence of max-age?

For max-age=<negative_value>, max-age=<invalid_value>, max-age without =.

Following is a part of Age Calculations section of RFC 2616. Negative delta is treated as ZERO when "now minus date_value".
As keyword of max-age is correctly specified in Cache-Control: header by server, server apparently knows about max-age. I think interpretation of "delta-seconds=1*DIGIT" as "delta-seconds=1*DIGIT(default:0)" in max-age processing is adequate error handling of incorrect setup or mistake at server side.
> http://tools.ietf.org/html/rfc2616#section-13.2.3
> 13.2.3 Age Calculations
>(snip)
>   We use the term "age_value" to denote the value of the Age header, in
>   a form appropriate for arithmetic operations.
>   A response's age can be calculated in two entirely independent ways:
>      1. now minus date_value, if the local clock is reasonably well
>         synchronized to the origin server's clock. If the result is
>         negative, the result is replaced by zero.
>      2. age_value, if all of the caches along the response path
>         implement HTTP/1.1.
> Does the "max-age" in this context mean existence of max-age=<valid_value>?
> Existence of max-age=? Or existence of max-age?

Unspecified.  The spec assumes no one will ever send invalid header values; one of the problems with that spec.  Probably a good thing to raise with httpbis too.
Hmm. I guess for negative values it would make more sense to treat them as zero instead of outright ignoring them, but what about max-age=foo?


What do other browsers do?
FYI.
Off-topic for this bug but next is a Weblog page of developer of Fiddler about IE's behaviour on valid max-age>2^31.
> http://blogs.msdn.com/ieinternals/archive/2010/01/26/Use-Max-Age-values-less-than-MaxInt.aspx
> http://msdn.microsoft.com/en-us/library/Bb250446.aspx
> http://www.fiddler2.com/Fiddler/help/
> http://insidehttp.blogspot.com/
According to the weblog, IE doesn't handle max-age>2^31 well, but Firefox handles it well(he says Fx uses 64bits value).
As he say "cause unnecessary revalidations", IE(and Opera/Safari, as he says "same mistake") seems to use ZERO when overflow occurs dues to max-age>2^31.
For next bug summary of this bug;
> Firefox 3.6.3 XMLHttpRequest object reports having "No expiration time"
> in the server's response when valid expires and cache-control headers exist

It should be following; 
> Firefox 3.6.3 XMLHttpRequest object reports having "No expiration time" in the server's response 
> when valid expires and INVALID value(negative value) of max-age in cache-control headers exist
Cache-Control: no-cache, no-store was returned to IE8.
I looks that your application does do browser sniffing and returns different header. (html structure, text in html looks same, but css, script etc. looks different).

(Off-topic)

To Jesse(bug opener):

Is your application developement system well-formed?
Is your application codes well-coded?
Why invalid Content-Type: max-age=-1000 can easily occur in your application?
I saw some pages which states that max-age=one year is set if max-age greater than one yer is requested by function call of an application developement system like ASP, JSP, .NET, PHP. If ordinal function call for expiration like max-age is used, negative value can't happen. Do you use function call which directly generates HTTP header?
Is max-age=-1000 intenstional setting for other than IE or Firefox only?
If so, what is your purpose to return invalid max-age=-1000 to other than IE or Firefox only?
Most of comment 15 doesn't belong in this bug.  If you really feel the need to ask those things, please use private mail instead of adding to the noise in the bug?

If someone has the time to test IE/Chrome/etc, I'd love an answer to Christian's question from comment 10.
(In reply to comment #16)
> Most of comment 15 doesn't belong in this bug.  If you really feel the need to
> ask those things, please use private mail instead of adding to the noise in the bug?

Roger. Sorry for noise.
But, { (all of comment 15) - (the most of comment 15 which you call "noise") } is my answer for IE8 case to next question of comment #10 by Christian :Biesinger, using site provided by bug opener, using my hands and my time.
> What do other browsers do?
I quickly reportd next in order that other people won't waste his valuable time by testing with site provided by bug opener using IE(8).
> application provided by bug opener returns Cache-Control:no-cache,no-store to IE8.
In such case, test with test case by tester himself is required. But, site I can use PHP doesn't permit use of function call which directly generates HTTP header such as header("Content-Type: ..."). Boris, can you provide a test case at a site which always returns invalid max-age=-1000 and valid Expires: header to any browser?
I'll try to get that set up today, yeah.
Clearing blocking flags because I can't really tell what the decision is here - why do we think this blocks earlier branches?
blocking1.9.2: ? → ---
status1.9.2: ? → ---
(In reply to comment #16)
> Most of comment 15 doesn't belong in this bug.  If you really feel the need to
> ask those things, please use private mail instead of adding to the noise in the
> bug?
> 
> If someone has the time to test IE/Chrome/etc, I'd love an answer to
> Christian's question from comment 10.

Thank you for your analysis of the problem. I now understand the source of the problem is my application’s “Cache-Control: max-age” header invalid value.  I’ve changed my application to “Cache-Control: max-age=0” and consider that a permanent solution. For what it’s worth, my reason for making max-age=-1000 was ensure that the response would not be cached like using an “Expired” header with value equal to a date in the past. I was not aware that a negative max-age value was not valid. I also understand the good reason why Firefox changed the behavior of using “Control: max-age” before “Expires” header. However, Firefox should consider the implications of such a change which in this case would cause all applications having an invalid value for “Control: max-age” to break. Further, determining that an application is affected is difficult due to no hard-error being reported. With that said there could be many applications affected and know way quantify. I’d say that a negative “Control: max-age=-xxx” should be treated as zero and a non-numeric “Control: max-age=xxx” should be treated as not set should mitigate most if not all problems.

Thanks again for your time, and expertise!
WADA, I put up four testcases:

<http://landfill.mozilla.org/ryl/testMaxAge.cgi> sends negative max-age and Expires in the past.
<http://landfill.mozilla.org/ryl/testMaxAgeExpiresFuture.cgi> sends negative max-age and Expires in the future (by 1 day).
<http://landfill.mozilla.org/ryl/testExpiresFuture.cgi> sends expires in the future (by 1 day) and no max-age header.
<http://landfill.mozilla.org/ryl/testMaxAgeNonnumericExpiresFuture.cgi> sends expires in the future (by 1 day) and max-age=abc.

I just tried Opera, Safari, Chrome (all on Mac), and they all seem to reload all four testcases every time (so not caching them?  Did I mess up the headers somehow?).  IE8 on Windows caches testExpiresFuture but not any of the other tests.

So it looks like IE8 treats negative and nonnumeric max-age values as 0 or something.  WADA, can you confirm?

> why do we think this blocks earlier branches?

Because it can cause content on 3.6 to be cached forever when it shouldn't be and wasn't in 3.5.  So while I'm not completely sure it should block, I definitely think we should backport the fix once we have one.
status1.9.2: --- → ?
(In reply to comment #21)
> I put up four testcases:

Following is "Caching" report by Fiddler2 for IE8.
  max-age: This resource will expire immediately. [-1000 sec]
    => it sounds that IE8 use max-age=0
  max-age directive malformed: abc
    => it sounds that IE8 ignores max-age.

> <http://landfill.mozilla.org/ryl/testMaxAge.cgi> sends negative max-age and
> Expires in the past.

HTTP/1.0 Expires Header is present: Sun Apr 18 22:25:40 PDT 2010
HTTP/1.1 Cache-Control Header is present: private, max-age=-1000
	private: This response MUST NOT be cached by a shared cache.
	max-age: This resource will expire immediately. [-1000 sec]

> <http://landfill.mozilla.org/ryl/testMaxAgeExpiresFuture.cgi> sends negative
> max-age and Expires in the future (by 1 day).

HTTP/1.0 Expires Header is present: Tue Apr 20 22:31:09 PDT 2010
HTTP/1.1 Cache-Control Header is present: private, max-age=-1000
	private: This response MUST NOT be cached by a shared cache.
	max-age: This resource will expire immediately. [-1000 sec]

> <http://landfill.mozilla.org/ryl/testExpiresFuture.cgi> sends expires in the
> future (by 1 day) and no max-age header.

HTTP/1.0 Expires Header is present: Tue Apr 20 22:38:57 PDT 2010

> <http://landfill.mozilla.org/ryl/testMaxAgeNonnumericExpiresFuture.cgi> sends
> expires in the future (by 1 day) and max-age=abc.

HTTP/1.0 Expires Header is present: Tue Apr 20 22:39:40 PDT 2010
HTTP/1.1 Cache-Control Header is present: private, max-age=abc
	private: This response MUST NOT be cached by a shared cache.
	max-age directive malformed: abc

FYI.
IE's cache settings and behaviour. Very similar to Fx's one, but it looks slightly different.
> http://msdn.microsoft.com/en-us/library/bb250442.aspx#Table5
Behaviour of IE8. Checked using Fiddler2.
(a) Ctrl+R at http://www.google.com/intl/ja/googlegroups/privacy3.html
    IE8 sent If-Modified-Since: Thu, 25 Mar 2010 09:42:43 GMT as expected,
    and 304 was returned as expected.
(b) Ctrl+R at four test pages.
    IE8 didn't send If-Modified-Since: in any case, and 200 was returned.
It looks IE8 uses max-age=0 also for max-age=abc as you say.
Ctrl+R of IE8 seems different from Mozilla's one. I should have been aware of it upon check of testExpiresFuture.cgi(Expires only) case in which HTTP GET shouldn't be sent when standard cache setting of IE8.

Following is check result with re-visiting(add a space and remove the space at URL bar, Enter).
> testMaxAge.cgi :sends negative max-age and Expires in the past
  HTTP GET, without If-Modified-Since. 200
> testMaxAgeExpiresFuture.cgi : sends negative max-age and Expires in the future (by 1 day).
  HTTP GET, without If-Modified-Since. 200
> testExpiresFuture.cgi : sends expires in the future (by 1 day) and no max-age header.
  No HTTP GET.
> testMaxAgeNonnumericExpiresFuture.cgi : sends expires in the future (by 1 day) and max-age=abc.
  HTTP GET, without If-Modified-Since. 200

It looks that IE8 uses max-age=0 for both max-age=-1000 and max-age=abc.
I guess the alternatives for max-age=foo are (1) treat it as 0, and (2) ignore it and work as if it isn't present? (Sounds like there is consensus about treating negative values as 0.) FWIW, I think alternative (2) makes more sense...

However, FWIW refer to RFC 2616 section 14.22 5th paragraph about the Expires-header

> HTTP/1.1 clients and caches MUST treat other invalid date formats,
> especially including the value "0", as in the past (i.e., "already expired").

I.e. invalid values for the Expires-header must be treated as 0. There is no similar statement about max-age though. :(

(In reply to comment #22)
> HTTP/1.0 Expires Header is present: Sun Apr 18 22:25:40 PDT 2010
> HTTP/1.1 Cache-Control Header is present: private, max-age=-1000
>     private: This response MUST NOT be cached by a shared cache.
>     max-age: This resource will expire immediately. [-1000 sec]

If "Cache-Control: private" is set it explains the unconditional GETs from comment #24 and blows up the experiment IMO. (I can confirm that "private" is sent, btw, by logs from my debug-build.)

What we need is "testMaxAgeNonnumericExpiresFuture.cgi" without "Cache-Control: private". If max-age=abc is treated as 0 we should see a subsequent conditional request, but if it's ignored it should be read from cache.

In order to verify that max-age=-1000 is treated as 0 we could use "testExpiresFuture.cgi" (without "Cache-Control: private") and expect a subsequent conditional request.
I'll change all the tests to not send "private" later today and retest.
I made that change to the tests in comment 21.  The IE results are still the same.
I also checked that IE treats max-age=500abc as 500.
Strictly speaking, an If-Modified-Since (IMS) request should not be sent since the cached response has no Last-Modified header. I overlooked this in my previous comment - sorry! This means that we cannot expect IMS in the second request for any of the tests above AFAICS.

If we want to see requests with If-Modified-Since, a Last-Modified header should be added to the responses. The last-modified time shouldn't be important - a week in the past or so is probably ok. (Not quite sure why we want to see this type of requests, though, but it seemed important when reading comment #24.)

(In reply to comment #27)
> I made that change to the tests in comment 21.  The IE results are still the
> same.

Which indicates that IE treats negative max-age as 0 and probably uses atoi() (like we do) to parse the value, giving 0 for bogus values (as indicated in the end of comment #24). In both cases, the mere presence of max-age seems to override the Expires-header. No?

(In reply to comment #28)
> I also checked that IE treats max-age=500abc as 500.

atoi() ?

So, should we add a simple check for negative values in GetMaxAgeValue() before casting to uint?
> since the cached response has no Last-Modified header.

Hrm.  The HTTP spec says to use Date to compute Last-Modified if not specified explicitly.  But I added a Last-Modified for a week in the past to the tests.  In any case, we don't care about IMS requests (since they will always get a 200 response).  We just care whether a request is made at all, or whether the cache is used.

> Which indicates that IE treats negative max-age as 0 and probably uses atoi()

Yes.

> So, should we add a simple check for negative values in GetMaxAgeValue()

That would make us have the bug described at the end of comment 11.

In any case, I have a patch for this bug; I'm just running it on tryserver to make sure that Windows has the right standard library stuff.
And in fact it does not.  In particular, no strtoll on MSVC.  Testing another patch now that uses _strtoi64 as needed....
Actually, wait.  MSVC does have strtol.  That should be dandy.
And in fact it seems that atoi is supposed to do clamping on the high end.  And does on Mac, at least.  I have no idea why the bug of comment 11 shows up at all.
Attached patch FixSplinter Review
Attachment #440683 - Flags: review?(jduell.mcbugs)
(In reply to comment #34)
> Fix

>     const char *p = PL_strcasestr(val, "max-age=");
>     if (!p)
>         return NS_ERROR_NOT_AVAILABLE;
> -    *result = (PRUint32) atoi(p + 8);
> +    int maxAgeValue = atoi(p + 8);
> +    if (maxAgeValue < 0)
> +        maxAgeValue = 0;
> +    *result = PRUint32(maxAgeValue);

Fx's interpretation of "existence of max-age directive" looks "existence of string of max-age=" insted of "existence of string of max-age".

Does "atoi(p + 8);" mean max-age=8 is used if max-age=0 or maxage= is specified?

Reason why Fx is torelant with 2^32>max-age value>2~^31 looks use of PRUint32. IE8 probably uses 32bits signed integer, and, may execute value range check before store data in 32bits signed integer or may utilize overflow/underflow status code, exception etc. of arithmatic instruction like "Add".

For atoi() if max-age= or max-age=-abc123xyz etc.

(A C++ Reference, C Library)
> http://www.cplusplus.com/reference/clibrary/cstdlib/atoi/
> int atoi ( const char * str );
> Convert string to integer
>(snip)
> If the first sequence of non-whitespace characters in str is not a valid integral number, or if no such sequence exists because either str is empty or it contains only whitespace characters, no conversion is performed.

(A C Guide)
> http://www.acm.uiuc.edu/webmonkeys/book/c_guide/
> http://www.acm.uiuc.edu/webmonkeys/book/c_guide/2.13.html#atoi
> 2.13.2.2 atoi
>(snip)
> On success the converted number is returned.
> If the number cannot be converted, then 0 is returned. 

What does "no conversion is performed" mean in C++ world?
Correct spec is "0 is retuned" as above C Guide says?
Note: If parseInt of JavaScript, NaN is returned in such case.
> Fx's interpretation of "existence of max-age directive" looks "existence of
> string of max-age=" insted of "existence of string of max-age".

Yes.  And is otherwise sorta dodgy.  But I wasn't going to try to get involved in that morass.

> Does "atoi(p + 8);" mean max-age=8 is used if max-age=0 or maxage= is
> specified?

No, it means "start parsing the number 8 characters after the character pointed to by p".  'p' is pointing to the 'm' of "max-age"; 8 chars later is the first char after the '='.

> Reason why Fx is torelant with 2^32>max-age value>2~^31 looks use of PRUint32.

No, just the use of atoi.  atoi will treat any number >= 2^31 as 2^31-1.

> What does "no conversion is performed" mean in C++ world?

Nothing in particular.  The function is defined to return 0 in all those conditions is all.

parseInt in JS has nothing to do with atoi in C.
Assignee: nobody → bzbarsky
Attachment #440683 - Flags: review?(jduell.mcbugs) → review+
Is there any SPEC or DESIGN of Mozilla for "RFC violation of max-age=+nnnX... by server" where nnn is valid digits, X is null or non-digit, ... is any string?

I think treatment as "max-age=nnn" by atoi() use is practically right thing. However SPEC is SPEC, DESIGN is DESIGN. I believe any code should base on SPEC or DESIGN. As RFC never defines about RFC violation or tolerance with RFC violation, reason why a software chose an action for RFC violation should be clearly stated. Can I understand current spec is next? (never same as MS's answer of "it's spec, because it's as coded" to personal user for bug of office products or products for personal user)
  - We chose atoi() because behavior based on atoi() is practically correct
    for max-age handling, if negative value is considered ZERO.
    And, by atoi() use, it can be tolerant with max-age>=2^31 without any
    excess logic, without any problem in real world.
  - Although definition of "max-age directive" is not string of max-age=,
    and although "existence of max-age directive" in RFC sounds "existence of
    name of max-age", max-age directive always requests specified value.
    Next is funny from view point of software design if it's design,
    unless "=" is defined as mandatory string.
      max-age= => max-age directive is specified. max-age=0
      max-age  => ignored. max-age directive is not specified.
    However, it's practically acceptable spec, because max-age directive
    requests specified value, and it won't produce any problem in real world.

> > Reason why Fx is torelant with 2^32>max-age value>2~^31 looks use of PRUint32.
> No, just the use of atoi. atoi will treat any number >= 2^31 as 2^31-1.

Aha.
IE's code is perhaps written by alien :-)
Opera traced IE's behaviour well, including bugs of IE in many cases for compatibility wit IE, to avoid user's confusion. I guess it's reason why "same fault" of Opera.
> Is there any SPEC or DESIGN of Mozilla for "RFC violation of max-age=+nnnX...
> by server"

Not really.  ;)

Your two bullet points look correct, though.
Pushed http://hg.mozilla.org/mozilla-central/rev/5d310c069d6d
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Comment on attachment 440683 [details] [diff] [review]
Fix

We should backport this very safe regression fix to 1.9.2 as well.
Attachment #440683 - Flags: approval1.9.2.5?
Attachment #440683 - Flags: approval1.9.2.4?
Attachment #440683 - Flags: approval1.9.2.6?
Attachment #440683 - Flags: approval1.9.2.5?
Attachment #440683 - Flags: approval1.9.2.4?
Keywords: regression
Attachment #440683 - Flags: approval1.9.2.6?
Attachment #440683 - Flags: approval1.9.2.6?
Attachment #440683 - Flags: approval1.9.2.6? → approval1.9.2.6+
Comment on attachment 440683 [details] [diff] [review]
Fix

Approved for 1.9.2.6, a=dveditz for release-drivers
Verified for 1.9.2 using Boris' testcases and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7 (.NET CLR 3.5.30729).
Keywords: verified1.9.2
blocking2.0: ? → final+
You need to log in before you can comment on or make changes to this bug.