Fix RevisionStorage::update()
Closed, ResolvedPublic
Actions

Description

This is broken (it NULLs the content if External Store is in use), and I do not believe it's used in the ordinary course of user actions. It is used once by the maintenance script that caused T95580: Flow data missing on Wikimedia production wikis.

I'm not sure we can technically remove it since it extends from ObjectStorage::update(), but we could have it throw an exception.

There is a check that's meant to prevent this, but processExternalStore is not affected. Also, calcUpdates/splitUpdates removes the content fields (because it only updates the columns that changed, and the content didn't), causing it to think it needs to be inserted into External Store.

Details

	Subject	Repo	Branch	Lines +/-
	Fix RevisionStorage->update	mediawiki/extensions/Flow	master	+461 -162

Customize query in gerrit

Revisions and Commits

rEFLW extension-Flow
	rEFLW8bee68296446 Fix RevisionStorage->update
	rEFLW959620379d75 Fix RevisionStorage->update
	rEFLW7b794e26f2c6 Fix RevisionStorage->update
	rEFLWbf59b5d7750e Fix RevisionStorage->update
	rEFLW5385a486b051 Fix RevisionStorage->update
	rEFLW4fffe538016c Fix RevisionStorage->update
rMEXT MediaWiki Extensions
	rMEXTb13c776987d2 Updated mediawiki/extensions Project: mediawiki/extensions/Flow…

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		EBernhardson	T95580 Flow data missing on Wikimedia production wikis
		Resolved		• Mattflaschen-WMF	T95869 Fix RevisionStorage::update()

Event Timeline

• Mattflaschen-WMF created this task.Apr 13 2015, 3:18 AM

• Mattflaschen-WMF raised the priority of this task from to Needs Triage.

• Mattflaschen-WMF updated the task description. (Show Details)

• Mattflaschen-WMF added a project: StructuredDiscussions.

• Mattflaschen-WMF added subscribers: • Mattflaschen-WMF, EBernhardson.

Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptApr 13 2015, 3:18 AM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

This will have to be fixed, the upcoming plans for a suppression bitfield will require it to exist for non-standard but possible user actions.

In T95869#1202144, @EBernhardson wrote:

This will have to be fixed, the upcoming plans for a suppression bitfield will require it to exist for non-standard but possible user actions.

Is this for adding the feature of suppressing past revisions while leaving the current one visible?

Yes, to make a past revisions un-viewable without just suppressing the whole thing we need to update that revision.

Any more details on what specifically is broken? Any example that gets treated incorrectly?

It NULLs out the content if External Store is in use (I guess unless you updated the content, but we would never want to update the content of an existing revision).

• Mattflaschen-WMF updated the task description. (Show Details)Apr 13 2015, 3:05 PM

I could think of one reason to update the content, if someone found an XSS in parsoid. We have a hook for keeping those from being output, but we would still want to update the historical revisions. We can certainly do that without going through the ObjectManager if its ever necessay though.

In T95869#1203030, @EBernhardson wrote:

I could think of one reason to update the content, if someone found an XSS in parsoid. We have a hook for keeping those from being output, but we would still want to update the historical revisions. We can certainly do that without going through the ObjectManager if its ever necessay though.

Good point. Ideally we'll be using RESTBase by then, though.

• Mattflaschen-WMF claimed this task.Apr 13 2015, 4:32 PM

• Mattflaschen-WMF triaged this task as Unbreak Now! priority.

• Mattflaschen-WMF added a project: Collaboration-Team-Sprint-U-2015-04-22.

• Mattflaschen-WMF mentioned this in T95580: Flow data missing on Wikimedia production wikis.Apr 13 2015, 4:35 PM

• Mattflaschen-WMF updated the task description. (Show Details)

• Mattflaschen-WMF added a parent task: T95580: Flow data missing on Wikimedia production wikis.Apr 13 2015, 4:38 PM

greg added a project: incident-20150410-flowdataloss.Apr 13 2015, 6:47 PM

• Mattflaschen-WMF mentioned this in T95974: Retest FlowUpdateRevisionContentLength.Apr 14 2015, 12:17 AM

Change 203998 had a related patch set uploaded (by Mattflaschen):
Fix RevisionStorage->update

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

gerritbot added a project: Patch-For-Review.Apr 15 2015, 2:31 AM