From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; DigExt; Q312461) BuildID: 2002031104 Reproducible: Always Steps to Reproduce: Visit specified URL. Actual Results: Recieved an error that the URL redirection limit was exceeded. Expected Results: Should have browsed to the appropriate web page.

*** This bug has been marked as a duplicate of 128443 ***

This can't be a dupe of that bug because: a) The URL is different b) bug 128443 is in Tech Evangelism and even if this bugs is the same problem it would be not a dupe. Reporter: Do you accept all cookies ?

-> Networking:http and confirming with win2k and 0.9.9 (my current build is horked due to the gmake change)

"Enable all cookies" is selected in the Edit/Preferences/Privacy & Security/Cookies.

-> badami for investigation

1. In nsHttpResponseHead::ExpiresInPast(), assume that the entry has expired and then check if it actually is going to expire in the future. 2. In nsHttpChannel two changes : a. Some more debug logging b. Do not assume cache entry as valid if we got only READ access unless we are offline.

wrt comment 6, I need to mention that for this particular case we only need the fix in nsHttpResponseHead. However, I think that it is better to get both the fixes in.

vinay: as i said via email, if we get READ ONLY access to the cache then we should not go out to fetch the document from the network. this can only mean that we are offline already. so, there is no value in adding the first part of this patch (a debug assertion might be good, but otherwise i don't see the point). the changes to ExpiresInPast are going to cause regressions elsewhere. we use that function to implement nsIHttpChannel::isNoCacheResponse(). ExpiresInPast is only TRUE if the server sends an Expires header w/ a value less than the value of the Date header. we treat this case similarly to 'Pragma: no-cache' whereas a response sent w/o an Expires header should not be treated as 'Pragma: no-cache'. seems to me that your patch for ExpiresInPast works because it changes the behavior of nsHttpResponseHead::MustValidate() to always return TRUE when an Expires header is not present. this is bad because it'll prevent caching in a lot of cases. perhaps nsHttpResponseHead::MustValidateIfExpired could be modified to return TRUE for redirects. but that still seems like a hack. we need to better understand why we're hitting this condition. is there a '<=' that should be a '<'??

Comment on attachment 77028 [details] [diff] [review] version 1 r=gagan

Comment on attachment 77028 [details] [diff] [review] version 1 needs-work

Comment on attachment 77028 [details] [diff] [review] version 1 revoking r= per darin's comments. I am clearly getting way old for this stuff! :)

Summary of log / causes redirect to /all/public. We then have a series of redirects resulting in setting of a session cookie and then a redirect back to /all/public. We are then picking up the cached response. The Cache-Control directive should have been no-cache as opposed to private when we got the response for /all/public. We are not getting an expires header for the request to /all/public.

vinay: good work isolating the real problem here. your patch looks like a good work around for this broken server. unfortunately, i think your patch would hurt performance on sites that follow the RFC to the letter. some redirects are supposed to be cached... even requests that have Cookie headers that result in redirects. you could argue that since the first request didn't have cookies and the second one does that they are different. however, there is no provision for such in the RFC. perhaps if the website sent a "Vary: Cookie" response header, then we'd be just in landing something similar to your patch. of course, we currently don't handle the "Vary" header correctly, but that should change ;-) i'm torn between landing this patch and evangelizing the website. they are clearly violating RFC2616. 301 redirects are explicitly cacheable, and there's nothing in this response that would dictate otherwise: HTTP/1.1 301 Moved Permanently Server: Microsoft-IIS/4.0 Date: Tue, 02 Apr 2002 17:30:29 GMT Location: http://www.adtran.com/all/public Content-Length: 0 Content-Type: text/html Cache-Control: private the site should be using a 302, not a 301. no matter what we decide to do on our end, i want this bug to eventually end up in the hands of our evangelism team.

Darin I agree and understand. I think we should lean on getting this checked in. There are too many sites that have problems of this nature. No point tracking down these problems again on other sites. Just a waste of $$. Given the fact that it works with IE, it looks like they must have some such form a similar hack.

You get this error message on MSN too. 1. go to http://communities.msn.com/people 2. click a chat Result: you get the "Redirection limit ..." error It *is* expected that this site doesn't work (b/c they need to fix it) but that message was quite unexpected. Could it at least be reworded to something more user friendly if the error needs to stay?

FYI behavior in 9.4 on the MSN Chat site is an incessant "Transferring - Connecting" message in the bottom status bar. Call me an evangelista but I like that a user might be more prone to report the problem to the website's customer support, based on that behavior.

Susie What is wrong with this site ? Is there any info ? Seems to work with IE.

gagan, bbaetz: what are your thoughts on this issue? see comment #14. thx!

Susie: we added a limit on the number of times an URL could be redirected to another URL after mozilla 0.9.4. this was done to prevent needlessly spamming the internet and to let the user know that mozilla discovered that something is wrong. originally, i wanted the message to say that there is something wrong with the website, but in reality it is usually a combination of mozilla and the website that leads to infinite redirects. we've nailed a lot of the problems in mozilla that could lead to an infinite redirect, and what were seeing now are sites that don't follow the standards, but instead rely on IE's behavior to dictate expected behavior :-/ as is the case for chat.msn.com, iirc.

I think that no Vary header should imply: Vary: Cookie Optionally, only do this if we are actualy going to accept the cookie (based on cookie prefs), but that may not be possible from http. Do other browsers cache 301 responses permenantly, and what do thay do in this case?

Oh, and only check on the cookie's value, not expiration date, if possible. OTOH, this is a redirection, so there really is no data. Maybe lets just not cache 301s which set a cookie we didn't already have.

I am inclining towards bbaetz suggestion. IIRC that's similar to what we did for 4.x too.

Doesnt the patch do exactly what u folks are suggesting ? We check for a cookie and if it is a redirect. R u suggesting that in addition we check whther there was a cookie on the original request etc. I did not want to do that since we then start comparing the cookie value etc, existence of a new cookie etc. I think the fix in hand will suffice.

it'd be difficult to solve this problem by observing when cookies are set because we'd have to "be the cookie manager" to know when cookies that are received should be sent. i think vinay's patch is the best solution. it is effectively implementing a solution that assumes "Vary: Cookie" on any redirect response. since we don't really have much in the way of Vary header support right now, i think vinay's patch should be checked in as is.

Comment on attachment 77180 [details] [diff] [review] revalidate if redirect and cookie is present sr=darin

Comment on attachment 77180 [details] [diff] [review] revalidate if redirect and cookie is present r=bbaetz

Comment on attachment 77180 [details] [diff] [review] revalidate if redirect and cookie is present a=asa (on behalf of drivers) for checkin to the 1.0 trunk

fixed with checkin D:\mozilla\netwerk\protocol\http\src>cvs commit nsHttpChannel.cpp Checking in nsHttpChannel.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v <-- nsHttpChannel.cpp new revision: 1.108; previous revision: 1.107 done

In answer to Comment #18 MSN needs to do some server side work on Chat, for Gecko compatibility. If it didn't work in IE either that would be pretty funny!

Verified http://www.adtran.com on Win2000 and Linux. MSN chat works only on Win32, as designed.