Page MenuHomePhabricator

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

Description 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

Event Timeline

I just made a patch above to deal with that.

Thanks for the report!

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

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.

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

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)

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

aaron changed the visibility from "Custom Policy" to "Public (No Login Required)".Sep 2 2016, 7:27 PM

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

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