Last modified: 2011-06-07 18:27:30 UTC
Created attachment 8355 [details] Proposed fix: Skip user block checks for Title::quickUserCan() In 1.16, unprotected pages showed an "edit" tab to the user, even if he was blocked from editing. In 1.17 this behavior has accidentally changed to show a "view source" tab for blocked users. This is problematic because the page is then cached in squid and shown to other, non-blocked users with the wrong tab. The change has been caused by r65504, when Title::getUserPermissionsErrorsInternal (which is called for quickUserCan) was refactored to also check for user blocks. The attached patch fixes this by skipping the block checks for Title::quickUserCan. Note that the patch also removes an unnecessary check for "$short && count($errors)", this is handled by getUserPermissionsErrorsInternal() already.
Created attachment 8356 [details] Test addition for TitlePermissionTest.php A small unit test to check that quickUserCan() ignores blocks. I had to make some unrelated tweaks to Title.php to make the test work again, as it is currently broken on trunk.
Comment from triage: "should be fixed if it is breaking caching, but the right thing may be to disable general caching if we find that you (user) are blocked?" Could you apply your fix so we can test it?
(In reply to comment #2) > Comment from triage: "should be fixed if it is breaking caching, but the right > thing may be to disable general caching if we find that you (user) are > blocked?" > And how would we do that? The caching happens on the Squid level and only looks at login cookies to let stuff through.
Fixed in r87326. Note there's also bug5106 for the issue of showing the "view source" tab to blocked users.
Please see code review comment on r87326; the $short parameter on Title::checkUserBlock is no longer used. Is this deliberate? Does that need to be fixed, or removed, or changed?
