Page MenuHomePhabricator

Wikibase Snak hashes (and thus "mainsnak", "references" and "qualifiers" hashes) depend on legacy PHP serialization
Closed, ResolvedPublic

Description

As can be seen in the Wikibase JSON documentation, we output hashes for snaks ("mainsnak") and collections of snaks ("references" and "qualifiers"). These hashes depend on the PHP serialization of these objects, for example:

abstract class SnakObject implements Snak {
	public function getHash() {
		return sha1( serialize( $this ) );
	}
}

Once we introduce the new PHP __serialize/ __unserialize methods (and PHP 7.4+ is used), this serialization will change. Especially when running in a mixed pre-PHP 7.4 and PHP 7.4+ environment, this can cause unexpected behavior (as hashes will change depending on the PHP version used).

In order to solve this, we should introduce legacy serialization methods that will output the old serialization regardless of the presence of the __serialize/__unserialize methods and the PHP version used.

Long-term we should consider moving away from using any form of PHP serialization for these hashes and introduce a fully controlled mean of defining these (previously discussed in T167759), but that's beyond the scope of this task.

Related Objects

Event Timeline

Change 773521 had a related patch set uploaded (by Hoo man; author: Hoo man):

[mediawiki/extensions/Wikibase@master] DataModel\Entity: Add __serialize/__unserialize for EntityIds

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

As mentioned by @Lucas_Werkmeister_WMDE in the Gerrit change, we should probably not merge this before production migrated away from PHP 7.2.

Related: T243590

As mentioned by @Lucas_Werkmeister_WMDE in the Gerrit change, we should probably not merge this before production migrated away from PHP 7.2.

Related: T243590

There’s now a dedicated task for moving CI back to a PHP 7.4 that emits the new serialization: T318918

Change 773521 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] DataModel\Entity: Add __serialize/__unserialize for EntityIds

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

I'm removing the in-Phab dependency on T318918 to make the graph more manageable.

Change 880550 had a related patch set uploaded (by Addshore; author: Hoo man):

[mediawiki/extensions/Wikibase@REL1_39] DataModel\Entity: Add __serialize/__unserialize for EntityIds

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

Change 880550 abandoned by Addshore:

[mediawiki/extensions/Wikibase@REL1_39] DataModel\Entity: Add __serialize/__unserialize for EntityIds

Reason:

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