Page MenuHomePhabricator
Create Task
Maniphest T328503

Custom content types whose getNativeData() method returns a non-string value can break XML dump generation
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to replicate the issue (include links if applicable):

  • Define a custom Content and ContentHandler; make the custom Content subclass return something from getNativeData that's not a string.
  • Create a page using the custom content model.
  • Try to export the page or generate a wiki-wide XML dump via dumpBackup.php

What happens?:

TypeError: strlen(): Argument #1 ($string) must be of type string, array given
Backtrace:
from includes/export/XmlDumpWriter.php(569)
#0 includes/export/XmlDumpWriter.php(569): strlen(array)
#1 includes/export/XmlDumpWriter.php(503): XmlDumpWriter->writeText(Fandom\InteractiveMaps\Content\InteractiveMapContent, array, string)
#2 includes/export/XmlDumpWriter.php(389): XmlDumpWriter->writeSlot(MediaWiki\Revision\SlotRecord, integer)
#3 includes/export/WikiExporter.php(555): XmlDumpWriter->writeRevision(stdClass, array)
#4 includes/export/WikiExporter.php(493): WikiExporter->outputPageStreamBatch(Wikimedia\Rdbms\MysqliResultWrapper, stdClass)
#5 includes/export/WikiExporter.php(315): WikiExporter->dumpPages(string, boolean)
#6 includes/export/WikiExporter.php(175): WikiExporter->dumpFrom(string)
#7 maintenance/includes/BackupDumper.php(363): WikiExporter->allPages()
#8 maintenance/dumpBackup.php(84): BackupDumper->dump(integer, integer)
#9 maintenance/includes/MaintenanceRunner.php(309): DumpBackup->execute()
#10 maintenance/doMaintenance.php(85): MediaWiki\Maintenance\MaintenanceRunner->run()
#11 maintenance/dumpBackup.php(144): require_once(string)
#12 {main}

What should have happened instead?:
The export / dump should have generated successfully.

Software version : MW 1.37

Details

Related Changes in Gerrit:
SubjectAuthorRepoBranchLines +/-
Fix XML dumps for content types with non-string getNativeData()Reedymediawiki/coreREL1_35+1 -1
Fix XML dumps for content types with non-string getNativeData()Reedymediawiki/coreREL1_39+117 -18
Fix XML dumps for content types with non-string getNativeData()Reedymediawiki/coreREL1_38+118 -18
Fix XML dumps for content types with non-string getNativeData()TK-999mediawiki/coremaster+117 -18
Customize query in gerrit

Related Objects

Event Timeline

TK-999 created this task.Feb 1 2023, 12:11 AM
gerritbot added a comment.Feb 1 2023, 12:20 AM

Change 885444 had a related patch set uploaded (by TK-999; author: TK-999):

[mediawiki/core@master] Fix XML dumps for content types with non-string getNativeData()

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

gerritbot added a project: Patch-For-Review.Feb 1 2023, 12:20 AM
Reedy subscribed.Feb 1 2023, 12:23 AM

Are you running with modifications? Line 571 is the strlen() - https://github.com/wikimedia/mediawiki/blame/REL1_37/includes/export/XmlDumpWriter.php#L571

Obviously 1.37 is unsupported, but clearly can still happen on master as the code is basically identical...

TK-999 added a comment.Feb 1 2023, 12:24 AM
In T328503#8576741, @Reedy wrote:

Are you running with modifications? Line 571 is the strlen() - https://github.com/wikimedia/mediawiki/blame/REL1_37/includes/export/XmlDumpWriter.php#L571

Obviously 1.37 is unsupported, but clearly can still happen on master as the code is basically identical...

My bad, the stacktrace is from 1.39 - https://github.com/wikimedia/mediawiki/blame/REL1_39/includes/export/XmlDumpWriter.php#L569. The issue reproduces on both 1.37 and 1.39. We're in the middle of deploying 1.39.

Reedy added projects: MW-1.35-release, MW-1.38-release, MW-1.39-release.EditedFeb 1 2023, 12:29 AM

As the code is from rMWfdc3e9f9524d: Add support for xml dump schema 0.11, I suspect it's applicable all the way back to 1.34 :)

TK-999 added a comment.Feb 1 2023, 12:31 AM
In T328503#8576744, @Reedy wrote:

As the code is from rMWfdc3e9f9524d: Add support for xml dump schema 0.11, I suspect it's applicable all the way back to 1.34 :)

Yup. It does require a custom content type though, which is probably why it went unnoticed.

Ottomata added a subscriber: Milimetric.Feb 1 2023, 1:24 PM
daniel subscribed.EditedFeb 1 2023, 2:27 PM

getNativeData was deprecated in 1.33. It's still used in XmlDumpWriter though:

		if ( $content instanceof TextContent ) {
			// HACK: For text based models, bypass the serialization step. This allows extensions (like Flow)
			// that use incompatible combinations of serialization format and content model.
			$data = $content->getNativeData();
		} else {
			$data = $content->serialize( $contentFormat );
		}

It seems to me like this should be fixed in Flow. getNativeData is broken by design, nothing should use it. Import/export should rely on serialize().

Anyway, if you have a Content object that extends TextContent but does not return a string from getNativeData(), this will explode.

TK-999 added a comment.Feb 1 2023, 3:18 PM
In T328503#8578510, @daniel wrote:

getNativeData was deprecated in 1.33. It's still used in XmlDumpWriter though:

		if ( $content instanceof TextContent ) {
			// HACK: For text based models, bypass the serialization step. This allows extensions (like Flow)
			// that use incompatible combinations of serialization format and content model.
			$data = $content->getNativeData();
		} else {
			$data = $content->serialize( $contentFormat );
		}

It seems to me like this should be fixed in Flow. getNativeData is broken by design, nothing should use it. Import/export should rely on serialize().

Anyway, if you have a Content object that extends TextContent but does not return a string from getNativeData(), this will explode.

Yeah, the fix just changes this to getText.

gerritbot added a comment.Feb 1 2023, 5:12 PM

Change 885444 merged by jenkins-bot:

[mediawiki/core@master] Fix XML dumps for content types with non-string getNativeData()

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

gerritbot added a comment.Feb 1 2023, 5:23 PM

Change 885775 had a related patch set uploaded (by Reedy; author: TK-999):

[mediawiki/core@REL1_39] Fix XML dumps for content types with non-string getNativeData()

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

gerritbot added a comment.Feb 1 2023, 5:24 PM

Change 885776 had a related patch set uploaded (by Reedy; author: TK-999):

[mediawiki/core@REL1_38] Fix XML dumps for content types with non-string getNativeData()

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

gerritbot added a comment.Feb 1 2023, 5:25 PM

Change 885777 had a related patch set uploaded (by Reedy; author: TK-999):

[mediawiki/core@REL1_35] Fix XML dumps for content types with non-string getNativeData()

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

gerritbot added a comment.Feb 1 2023, 5:57 PM

Change 885776 merged by jenkins-bot:

[mediawiki/core@REL1_38] Fix XML dumps for content types with non-string getNativeData()

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

gerritbot added a comment.Feb 1 2023, 5:57 PM

Change 885775 merged by jenkins-bot:

[mediawiki/core@REL1_39] Fix XML dumps for content types with non-string getNativeData()

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

ReleaseTaggerBot added projects: MW-1.39-notes, MW-1.40-notes (1.40.0-wmf.22; 2023-02-06), MW-1.38-notes.Feb 1 2023, 6:00 PM
gerritbot added a comment.Feb 1 2023, 7:43 PM

Change 885777 merged by jenkins-bot:

[mediawiki/core@REL1_35] Fix XML dumps for content types with non-string getNativeData()

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

ReleaseTaggerBot added a project: MW-1.35-notes.Feb 1 2023, 8:00 PM
Maintenance_bot removed a project: Patch-For-Review.Feb 1 2023, 8:30 PM
Reedy closed this task as Resolved.Feb 2 2023, 12:40 AM
Reedy assigned this task to TK-999.
ArielGlenn moved this task from Backlog to Done on the Dumps-Generation board.Jun 22 2023, 4:58 AM
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