Page MenuHomePhabricator
Create Task
Maniphest T144484

[Regression] revision delete bitfield get lost on page deletion
Closed, ResolvedPublic
Actions

Description

https://gerrit.wikimedia.org/r/#/c/305627/3 refactored the page delete, but in that case the revision delete bitfield is not correctly transfered to the archive table and get lost. This allows admins to delete pages and see revision deleted content or suppressed deleted content (oversight)

In sql strict mode this fails with
1366 Incorrect integer value: 'rev_deleted' for column 'ar_deleted' at row 1 (localhost)

but in non strict mode the text in the column is valid and gets converted to a 0, which means no revision delete

The patch set is already live on wmf/1.28.0-wmf.17, so this is created as security task. Please mark this as blocker for the today deployment. Thanks

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Fix deletion handling of rev_deletedmediawiki/coremaster+5 -3
Fix deletion handling of rev_deletedmediawiki/corewmf/1.28.0-wmf.17+3 -2
Customize query in gerrit

Related Objects

Event Timeline

Umherirrender created this task.Sep 1 2016, 11:37 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 1 2016, 11:38 AM
Umherirrender added projects: MediaWiki-Revision-deletion, MediaWiki-Page-deletion.Sep 1 2016, 11:52 AM
Umherirrender added a subscriber: aaron.
aaron added a comment.EditedSep 1 2016, 12:45 PM

fix_T144484.patch1 KBDownload

I just made a patch above to deal with that.

Thanks for the report!

aaron added a subscriber: Bawolff.Sep 1 2016, 12:46 PM
hashar added a parent task: T142117: MW-1.28.0-wmf.17 deployment blockers.Sep 1 2016, 1:06 PM
hashar subscribed.Sep 1 2016, 1:08 PM

@aaron poked me about it. I am not comfortable with that patch and I am really rusty with MW developement

Namely a select field is passed:

[ 'rev_deleted' => $bitfield ]

With $bitfield set above to either rev_deleted or an integer. So I am very confused about it.

aaron added a comment.Sep 1 2016, 1:19 PM

In the above patch, the syntax for the DB layer is [ alias => column]. If alias=column, then just column is used. So it's either:

  • rev_deleted
  • 15 AS rev_deleted

The later is for suppression.

I just tested round tripping this (delete/restore) and it looks fine.

hashar added a comment.Sep 1 2016, 1:22 PM

[alias => column] for column AS alias. Yeah I guess that is what confused me :]

hashar added a comment.EditedSep 1 2016, 1:29 PM

I have put patch on tin in /srv/patches/1.28.0-wmf.17/core/04-T144484.patch and applied it to 1.28.0-wmf.17.

synced after aaron reviewed the state on tin.

13:34:23 !log hashar@tin Synchronized php-1.28.0-wmf.17/includes/page/WikiPage.php: T144484 (duration: 00m 49s)
hashar assigned this task to aaron.Sep 1 2016, 1:56 PM

Aaron did the patch and it is deployed. Removing this task from 1.28.0-wmf.17 blockers.

hashar removed a parent task: T142117: MW-1.28.0-wmf.17 deployment blockers.Sep 1 2016, 1:56 PM
hashar mentioned this in T142117: MW-1.28.0-wmf.17 deployment blockers.
matmarex merged a task: T144513: Unable to delete a page.Sep 1 2016, 6:40 PM
matmarex added subscribers: Nikerabbit, • Mattflaschen-WMF.
Jdforrester-WMF subscribed.Sep 1 2016, 6:52 PM
• Mattflaschen-WMF unsubscribed.Sep 1 2016, 10:08 PM
Umherirrender added a comment.Sep 2 2016, 6:31 PM

Now public due to new test: https://integration.wikimedia.org/ci/job/mediawiki-phpunit-hhvm-trusty/5896/consoleFull

Can we get the fix in master? Thanks

aaron added a comment.Sep 2 2016, 7:04 PM

Master patch is https://gerrit.wikimedia.org/r/#/c/308220/

wmf17 (for anyone using out branch) is publicly at https://gerrit.wikimedia.org/r/#/c/308222/2

aaron changed the visibility from "Custom Policy" to "Public (No Login Required)".Sep 2 2016, 7:27 PM
Restricted Application added a subscriber: JEumerus. · View Herald TranscriptSep 2 2016, 7:27 PM
gerritbot subscribed.Sep 2 2016, 10:12 PM

Change 308220 merged by jenkins-bot:
Fix deletion handling of rev_deleted

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

gerritbot added a comment.Sep 2 2016, 10:27 PM

Change 308222 merged by jenkins-bot:
Fix deletion handling of rev_deleted

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

ReleaseTaggerBot added projects: MW-1.28-release-notes, MW-1.28-release (WMF-deploy-2016-08-30_(1.28.0-wmf.17)), MW-1.28-release (WMF-deploy-2016-09-06_(1.28.0-wmf.18)).Sep 2 2016, 11:00 PM
aaron closed this task as Resolved.Sep 3 2016, 11:00 AM
Nemo_bis mentioned this in T144686: Cannot delete any page: "A database query error has occurred".Sep 4 2016, 8:17 AM
• chasemp edited projects, added Security; removed MW-1.28-release (WMF-deploy-2016-09-06_(1.28.0-wmf.18)).Feb 10 2020, 10:53 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:13 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 · Credits