Hi,

I had a number of users report that differing user names were appearing on various peoples screens. It seemed obvious to me that, since they were behind a proxy, that the pages were being cached. But I didn't expect this to be happening as, as I understood it, auth users weren't cached. Also, some threads a year or so ago put in the http headers to ensure external proxies didn't cache pages.

However, with a bit of wget work, I soon realised that if, in admin >> settings >> cache settings, of the page caching is disabled then the vital headers that stop external proxy caching do not get sent.

My quick fix to ensure that auth users always have the "no cache" headers sent out are in my patch. However, I doubt I have implemented in the correct way but it got me "out of a hole" quickly.

So my issue is "not send external no cache headers if page caching is switched off" Is this normal?

best regards,
--AjK

CommentFileSizeAuthor
#11 bootstrap.inc_r1.96.2.2_patch014.55 KBAjK
#7 bootstrap.inc_1.100_patch044.64 KBAjK
#5 bootstrap.inc_1.100_patch034.64 KBAjK
#1 bootstrap.inc_1.100_patch021.4 KBAjK
bootstrap.inc_1.100_patch011004 bytesAjK

Comments

AjK’s picture

AjK commented
Status: Active » Needs review
StatusFileSize
new bootstrap.inc_1.100_patch021.4 KB

OK, I believe this patch is the correct approach (ie having spent time looking at the problem as opposed to the knee jerk reaction patch above).

best regards,
--AjK

killes@www.drop.org’s picture

killes@www.drop.org commented

I think the two if clauses could be merged into one.

dries’s picture

dries commented
Status: Needs review » Needs work

With the proposed change, we'll issue an extra (redundant) database query. page_get_cache() goes to the database ...

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Needs work » Needs review

Dries: Nope, it doesn't if the user is logged in. Thought the same initially.

AjK’s picture

AjK commented
StatusFileSize
new bootstrap.inc_1.100_patch034.64 KB

Killes,

Patch but merging the two if()s

regards
--AjK

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

I do think it might add another query in some cases. If we flip around the conditions on the if statement, the variable_get() may evaluate to FALSE and short circuit to skip the page_get_cache() call. I think this would be better since variable_get() is relatively lightweight.

AjK’s picture

AjK commented
StatusFileSize
new bootstrap.inc_1.100_patch044.64 KB

page_get_cache() does in fact check $user->uid first in it's own test so I assumed that it being first would cause the additional possibility of a query.

However, since it does no harm to swap the two conditions and you guys know better, here's a patch with the two swapped as suggested.

best regards
--AjK

AjK’s picture

AjK commented
Status: Needs work » Needs review
drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Fixed

Committed to HEAD.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Fixed » Active

Does this need to be fixed in 4.7 as well? If yes, I need a patch, cvs cvs one wouldn't apply.

AjK’s picture

AjK commented
Status: Active » Needs review
StatusFileSize
new bootstrap.inc_r1.96.2.2_patch014.55 KB

As requested, patch for the DRUPAL-4-7 branch.

best regards,
--AjK

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: x.y.z » 4.7.2
Status: Needs review » Reviewed & tested by the community

Looks like the same code to me.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Reviewed & tested by the community » Fixed

applied

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)