Page MenuHomePhabricator

Moving a page over redirect loses the protection settings for that page
Closed, ResolvedPublic

Description

See https://de.wikipedia.org/w/index.php?title=Ernst_%26_Young_BKK&action=history

Data:

  • 2. March. 2016, 19:44:19: MBq protected page with move=only editors
  • 2. March. 2016, 19:44:51‎: MBq moved the page
  • 2. March. 2016, 19:44:51‎: Protection was moved, from: https://de.wikipedia.org/w/index.php?title=Spezial:Logbuch&page=Ernst+%26+Young+BKK&uselang=en: 19:44, 2 March 2016 MBq (A/B) (talk | contribs) moved protection settings from EY BKK to Ernst & Young BKK
  • 2. March. 2016, 19:49:24‎: A account which didn't has editor rights moved the page back, so the move protection don't helped anything.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 2 2016, 7:00 PM
Luke081515 updated the task description. (Show Details)Mar 2 2016, 7:01 PM
Luke081515 added a subscriber: MBq.
Luke081515 updated the task description. (Show Details)Mar 2 2016, 7:06 PM
csteipp triaged this task as Normal priority.Mar 8 2016, 10:41 PM
csteipp added a subscriber: csteipp.

There's something else odd with dewiki's custom protection level for 'editors', it doesn't show up on the protection form when using an account with Staff rights.

It does look like the move should have been prevented by the protection, and the protection failed. Still digging into possible reasons for that.

So the timeline is

  • Page starts out being protected against autoconfirmed users
  • 17:53, 2 March 2016 BKK EY (talk | contribs) moved page Ernst & Young BKK to EY BKK over redirect (siehe BKK PwC, Benutzer:GUMPi und https://de.wikipedia.org/w/index.php?title=BKK_PwC&diff=150899687&oldid=150899142)
  • 2016-03-02T18:44:19 MBq (talk | contribs) protected EY BKK [Move=Allow only editors] (expires 2016-04-02T17:44:19) (Edit-War) (hist)
  • 2016-03-02T18:44:51 MBq (talk | contribs) moved page EY BKK to Ernst & Young BKK over redirect (lemmaversion vor edit-war)
  • 2016-03-02T18:49:24 BKK EY (talk | contribs) moved page Ernst & Young BKK to EY BKK over redirect (BKK PwC)
  • 2016-03-02T18:57:13 MBq (talk | contribs) moved page EY BKK to Ernst & Young BKK over redirect (nochmal)
csteipp added a subscriber: Anomie.Mar 8 2016, 10:55 PM

@Anomie, bawolff and I tried to figure out what was going on here, and not seeing anything obvious. Any chance you see what might have gone wrong?

There's something else odd with dewiki's custom protection level for 'editors', it doesn't show up on the protection form when using an account with Staff rights.

Maybe try asking the stewards (or get @Jalexander to ask the stewards) to give staff the editeditorprotected right at https://meta.wikimedia.org/wiki/Special:GlobalGroupPermissions/staff ?

Anomie added a subscriber: aaron.Mar 10 2016, 4:46 PM

@Anomie, bawolff and I tried to figure out what was going on here, and not seeing anything obvious. Any chance you see what might have gone wrong?

When I try to reproduce locally, I see the following sequence of actions get logged:

2016-03-10 11:30:21 queries INFO: my_wiki: INSERT /* MovePage::move Anomie */ IGNORE INTO `page_restrictions` (pr_page,pr_type,pr_level,pr_cascade,pr_user,pr_expiry) SELECT  2585,pr_type,pr_level,pr_cascade,pr_user,pr_expiry FROM `page_restrictions`   WHERE pr_page = '2582'  
2016-03-10 11:30:21 queries INFO: my_wiki: SELECT /* MovePage::move Anomie */  pr_id  FROM `page_restrictions`   WHERE pr_page = '2585' 
[request finishes...]
2016-03-10 11:30:21 runJobs DEBUG: deleteLinks TestT128624 pageId=2584 (id=1864,timestamp=20160310163021) STARTING
2016-03-10 11:30:21 queries INFO: my_wiki: DELETE /* LinksDeletionUpdate::doUpdate 0:0:0:0:0:0:0:1 */ FROM `page_restrictions` WHERE pr_page = '2582'

In other words, what seems to be happening is that a deleteLinks job is being scheduled for the moved-over redirect, but doesn't take effect until long after the page being moved is already in place. And the job is apparently picking up the new page_id from the title instead of the page_id that was actually deleted. I can't say with certainty that this is what happened on dewiki, but it seems very likely.

Chances are the solution would be to have the deleteLinks job insist on using the passed pageId in preference to the page_id of the existing title. @aaron might be able to tell us more, since deferring jobs like this is his thing.

matmarex renamed this task from Lemma move protection did not moved to Moving a page over redirect loses the protection settings for that page.Jun 6 2016, 3:09 PM

"The page you requested was not found, or you do not have permission to view this page."

Code-Review: +1; Should fix the protection issue that this bug is concerned with. Haven't tested.

Other things that are still likely broken, and should probably be fixed with a public followup patch:

  • I note we might still lose non-log recentchanges entries that belong to the new page at the title rather than the old, deleted page since it deletes entries by both rc_namespace+rc_title and by rc_cur_id.
  • The 'CategoryAfterPageRemoved' hook function (inside the call to WikiPage::updateCategoryCounts()) will effectively be called on the new page at the title rather than the old. WikiPage::updateCategoryCounts() itself should be fine.

Code-Review: +1; Should fix the protection issue that this bug is concerned with. Haven't tested.

Other things that are still likely broken, and should probably be fixed with a public followup patch:

  • I note we might still lose non-log recentchanges entries that belong to the new page at the title rather than the old, deleted page since it deletes entries by both rc_namespace+rc_title and by rc_cur_id.
  • The 'CategoryAfterPageRemoved' hook function (inside the call to WikiPage::updateCategoryCounts()) will effectively be called on the new page at the title rather than the old. WikiPage::updateCategoryCounts() itself should be fine.

@Anomie Will you create separate tickets for those items?

aaron added a comment.Jun 13 2016, 6:49 PM

Code-Review: +1; Should fix the protection issue that this bug is concerned with. Haven't tested.

Other things that are still likely broken, and should probably be fixed with a public followup patch:

  • I note we might still lose non-log recentchanges entries that belong to the new page at the title rather than the old, deleted page since it deletes entries by both rc_namespace+rc_title and by rc_cur_id.
  • The 'CategoryAfterPageRemoved' hook function (inside the call to WikiPage::updateCategoryCounts()) will effectively be called on the new page at the title rather than the old. WikiPage::updateCategoryCounts() itself should be fine.

@Anomie Will you create separate tickets for those items?

I also made https://gerrit.wikimedia.org/r/#/c/294099/ and will do the category hook fix once this patch is merged.

@Anomie Will you create separate tickets for those items?

Looks like Aaron already fixed them both.

Restricted Application removed a subscriber: Poyekhali. · View Herald TranscriptJul 10 2016, 5:48 PM
demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 10 2016, 9:00 PM
demon changed Security from Software security bug to None.
demon added a subscriber: demon.

Unmarking as security as it was mostly a regression (didn't affect 1.26 and below) and was already fixed publicly in master. Backported to 1.27 so it gets caught in the next release. I think this is resolved now?

dpatrick closed this task as Resolved.Aug 10 2016, 9:52 PM
dpatrick claimed this task.

Change 304107 merged by jenkins-bot:
Use the specified page ID for LinksDeletionUpdate

https://gerrit.wikimedia.org/r/304107