114508 - The syntax of the Last-Modified header of a HTTP response is not validated before storing the cache entry, and a malformed date will be used verbatim in future requests' If-Modified-Since

Status: NEW

(Core :: Networking: HTTP, defect, P5)

Description

[aaronr]

I don't know if this will be a bug or not, but I think that it is 
imporant that you decide whether this is a behavior that you want to 
emulate or not. 

The "If-Modified-Since" header entry is treated differently in Mozilla 
than Netscape 4.x.  In Netscape 4.x, if the "Last-Modified" header 
entry is of an unexpected format, it wouldn't put a "If-Modified-Since" 
entry in the request header.  In Mozilla, it will blindly use the value 
that was in the "Last-Modified" entry.  This is done in 
nsHttpChannel::CheckCache().  We ran into a problem in OS2 where the 
web server would set Last-Modified in milliseconds since it was 
generated by Java.  But it didn't like receiving milliseconds in 
"If-Modified-Since" since it was looking for a string here, not a 
value.  Yes, this is a bug in the web server, but it illustrates the 
problems that MIGHT exist out there.  BTW, IE 5.5 translates 
milliseconds into a proper date format to pass back in as 
"If-Modified-Since".

If you want to emulate the 4.x behavior, you just need to add a check 
after the call to PeekHeader(nsHttp::Last_Modified) inside 
nsHttpChannel::CheckCache().  Pass the value returned by this 
PeekHeader to PR_ParseTimeString(val, TRUE, &validTime).  If it doesn't 
return PR_FAILURE, then it is a format that we understand.  Otherwise 
set val=nsnull and pass that into SetHeader(nsHttp::If_Modified_Since).

Again, I don't know if this is the kind of thing you want to do or not, 
but it really needs to be considered as a "quirk" if nothing else.

--Aaron
[e-mail]

Comment 1

[Darin Fisher]

i'm all for supporting quirks provided they're handled as error conditions...
ie,  IMO we should minimize the cost of checking for quirks.

and... sounds like you have a patch in mind...
   ... how about whipping one together?  :-)

Comment 2

[aaronr]

Attachment: suggested "fix" to emulate net4.x behavior

This doesn't have the "error condition only" type of checking that you were
hoping for, but maybe that is something that we can add if the quick checking
in PR_ParseTimeString isn't quick enough.

Comment 3

[Darin Fisher]

i think this patch is reasonable, and i'd like to enable it XP.  i was even
thinking that a PeekDateHeader method might be nice.  this method would do the
checking as you have done.  i suspect there might be other places in the code
that might want such a function.  at any rate, thanks for the suggested patch :)

Comment 4

[Darin Fisher]

-> mozilla1.0, though it may miss.

Comment 5

[Darin Fisher]

-> will get to this after moz 1.0

Comment 6

[Darin Fisher]

mass futuring of untargeted bugs

Comment 7

[Darin Fisher]

-> default owner

Comment 8

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258