serialized values becomes strings

Path in #2 works for me too. Thank you Stefan.

I am still using the patch from #2, but run into an issue with Computed Field → and Metatag → module resulting in the following error (with parts of backtrace) as metatag module expects a serialized array in MetatagManager::getFieldTags():

TypeError: unserialize(): Argument #1 ($data) must be of type string, array given in /app/docroot/modules/contrib/metatag/src/MetatagManager.php on line 414 #0 /app/docroot/modules/contrib/metatag/src/MetatagManager.php(414): unserialize(Array)
#1 /app/docroot/modules/contrib/metatag/src/MetatagManager.php(169): Drupal\metatag\MetatagManager->getFieldTags(Object(Drupal\node\Entity\Node), 'field_meta_tags')
#2 /app/docroot/modules/contrib/metatag/src/MetatagManager.php(179): Drupal\metatag\MetatagManager->tagsFromEntity(Object(Drupal\node\Entity\Node))
#3 /app/docroot/modules/contrib/metatag/src/Plugin/Field/FieldType/MetatagNormalizedFieldItemList.php(41): Drupal\metatag\MetatagManager->tagsFromEntityWithDefaults(Object(Drupal\node\Entity\Node))
#4 /app/docroot/modules/contrib/metatag/src/TypedData/ComputedItemListTrait.php(32): Drupal\metatag\Plugin\Field\FieldType\MetatagNormalizedFieldItemList->computeValue()
#5 /app/docroot/core/lib/Drupal/Core/TypedData/ComputedItemListTrait.php(43): Drupal\metatag\Plugin\Field\FieldType\MetatagNormalizedFieldItemList->ensureComputedValue()
#6 /app/docroot/core/lib/Drupal/Core/Entity/ContentEntityBase.php(694): Drupal\metatag\Plugin\Field\FieldType\MetatagNormalizedFieldItemList->getValue()
#7 /app/docroot/modules/contrib/computed_field/src/ComputedFieldHelpers.php(55): Drupal\Core\Entity\ContentEntityBase->toArray()
#8 /app/docroot/modules/contrib/computed_field/src/Plugin/Field/FieldType/ComputedFieldItemTrait.php(58): Drupal\computed_field\ComputedFieldHelpers->executeCode('field_title_com...', Object(Drupal\node\Entity\Node), 0)
#9 /app/docroot/modules/contrib/computed_field/src/Plugin/Field/FieldType/ComputedFieldStronglyTypedItemTrait.php(19): Drupal\computed_field\Plugin\Field\FieldType\ComputedStringItemBase->executeCode()
#10 /app/docroot/modules/contrib/computed_field/src/Plugin/Field/FieldType/ComputedFieldItemTrait.php(22): Drupal\computed_field\Plugin\Field\FieldType\ComputedStringItem->getRawResult()
#11 /app/docroot/core/lib/Drupal/Core/Field/FieldItemList.php(93): Drupal\computed_field\Plugin\Field\FieldType\ComputedStringItemBase->isEmpty()
#12 [internal function]: Drupal\Core\Field\FieldItemList->Drupal\Core\Field\{closure}(Object(Drupal\computed_field\Plugin\Field\FieldType\ComputedStringItem))
#13 /app/docroot/core/lib/Drupal/Core/TypedData/Plugin/DataType/ItemList.php(275): call_user_func(Object(Closure), Object(Drupal\computed_field\Plugin\Field\FieldType\ComputedStringItem))
#14 [internal function]: Drupal\Core\TypedData\Plugin\DataType\ItemList->Drupal\Core\TypedData\Plugin\DataType\{closure}(Object(Drupal\computed_field\Plugin\Field\FieldType\ComputedStringItem))
#15 /app/docroot/core/lib/Drupal/Core/TypedData/Plugin/DataType/ItemList.php(281): array_filter(Array, Object(Closure))
#16 /app/docroot/core/lib/Drupal/Core/Field/FieldItemList.php(94): Drupal\Core\TypedData\Plugin\DataType\ItemList->filter(Object(Closure))
#17 /app/docroot/core/lib/Drupal/Core/Entity/ContentEntityBase.php(1459): Drupal\Core\Field\FieldItemList->filterEmptyItems()
#18 /app/docroot/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/ChangedItem.php(49): Drupal\Core\Entity\ContentEntityBase->hasTranslationChanges()
#19 /app/docroot/core/lib/Drupal/Core/Field/FieldItemList.php(233): Drupal\Core\Field\Plugin\Field\FieldType\ChangedItem->preSave()
#20 /app/docroot/core/lib/Drupal/Core/Field/FieldItemList.php(191): Drupal\Core\Field\FieldItemList->delegateMethod('preSave')
#21 /app/docroot/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php(938): Drupal\Core\Field\FieldItemList->preSave()
#22 /app/docroot/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php(888): Drupal\Core\Entity\ContentEntityStorageBase->invokeFieldMethod('preSave', Object(Drupal\node\Entity\Node))
#23 /app/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(563): Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('presave', Object(Drupal\node\Entity\Node))
#24 /app/docroot/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php(753): Drupal\Core\Entity\EntityStorageBase->doPreSave(Object(Drupal\node\Entity\Node))
#25 /app/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(517): Drupal\Core\Entity\ContentEntityStorageBase->doPreSave(Object(Drupal\node\Entity\Node))
#26 /app/docroot/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php(804): Drupal\Core\Entity\EntityStorageBase->save(Object(Drupal\node\Entity\Node))
#27 /app/docroot/core/lib/Drupal/Core/Entity/EntityBase.php(339): Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object(Drupal\node\Entity\Node))
#28 /app/docroot/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php(240): Drupal\Core\Entity\EntityBase->save()
#29 /app/docroot/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php(179): Drupal\migrate\Plugin\migrate\destination\EntityContentBase->save(Object(Drupal\node\Entity\Node), Array)

As said, I still use this patch, but had to serialize the metatag value like:

  field_meta_tags/0/value:
    - plugin: single_value
      source: field_meta_tags/0/value
    - plugin: callback
      callable: serialize

It seems to me this might be a bug in core's ContentEntity source class as well as in this module?

I'm a bit iffy about the fix, but I can't think of anything better :/

Also, we should probably pass allowed_classes = TRUE to unserialize() for security.

I think it is worth opening a feature request for the core entity system: add a way to indicate that a string is already serialized, so it should be stored as is. If that feature gets added, then we can easily update the Migrate API to add support for it. I have not checked myself: I am trusting you that there is not already such an option.

Until that happens, the current approach seems like a reasonable work-around.

From #8:

Also, we should probably pass allowed_classes = TRUE to unserialize() for security.

From https://www.php.net/unserialize:

Omitting this option is the same as defining it as true: PHP will attempt to instantiate objects of any class.

Did you mean to say FALSE instead of TRUE?

> Did you mean to say FALSE instead of TRUE?

Yes, oops, I meant FALSE.

> I think it is worth opening a feature request for the core entity system: add a way to indicate that a string is already serialized, so it should be stored as is.

Yup, let's do that.

@benjifisher and @joachim was there an issue filed with core that I can follow?

I see https://www.drupal.org/project/drupal/issues/2871217 🐛 Avoid error when $options is NULL in buildUrl() Needs work , has tagged this issue, but what they're addressing there is actually a bit different. (Here we're dealing with serialized data that has actually been stored incorrectly -- the error we're seeing is valid; #2871217 🐛 Avoid error when $options is NULL in buildUrl() Needs work is dealing with cases where $options is NULL.)