Page MenuHomePhabricator
Create Task
Maniphest T30354

Edit tab is shown as "view source" for blocked users, which breaks squid caching
Closed, ResolvedPublic
Actions

Description

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.

Version: 1.18.x
Severity: normal
URL: http://de.wikipedia.org/w/index.php?oldid=87148073#Seitenschutz_-_oder_doch_nicht.3F

Attached:

bug28354.patch1010 BDownload

Details

Reference
bz28354

Related Objects

Event Timeline

• bzimport raised the priority of this task from to High.Nov 21 2014, 11:28 PM
• bzimport added a project: MediaWiki-User-Interface.
• bzimport set Reference to bz28354.
P.Copp created this task.Apr 1 2011, 12:18 AM
P.Copp added a comment.Apr 1 2011, 12:20 AM

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.

Attached:

bug28354.test.patch2 KBDownload

MarkAHershberger added a comment.Apr 26 2011, 3:38 AM

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?

Catrope added a comment.Apr 26 2011, 3:30 PM

(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.

P.Copp added a comment.May 3 2011, 1:20 PM

Fixed in r87326.

Note there's also bug5106 for the issue of showing the "view source" tab to blocked users.

• brion added a comment.Jun 7 2011, 6:27 PM

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?

matmarex mentioned this in T7106: Changing label text of "Edit page" tab when blocked.Sep 10 2017, 7:44 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL