craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

[4.x]: Custom fields have double-encoded values if the content column type is `Schema::TYPE_JSON` #13916

Closed khalwat closed 1 year ago

khalwat commented 1 year ago

What happened?

Description

I have a custom Field that adds a Schema::TYPE_JSON content column type. Upon saving the field, the values in the field are double-encoded.

Craft does this in the saveContent() method in Content service: https://github.com/craftcms/cms/blob/develop/src/services/Content.php#L79

        foreach ($serializedFieldValues as $fieldUid => $value) {
            $field = $fields[$fieldUid];
            $type = $field->getContentColumnType();

            if (is_array($type)) {
                foreach (array_keys($type) as $i => $key) {
                    $column = ElementHelper::fieldColumnFromField($field, $i !== 0 ? $key : null);
                    $values[$column] = Db::prepareValueForDb($value[$key] ?? null);
                }
            } else {
                $column = ElementHelper::fieldColumnFromField($field);
                $values[$column] = Db::prepareValueForDb($value);
            }
        }

...and then in Db::prepareValueForDb() it does this: https://github.com/craftcms/cms/blob/develop/src/helpers/Db.php#L128

        // If this isn’t a JSON column and the value is an object or array, JSON-encode it
        if (
            !in_array($columnType, [Schema::TYPE_JSON, YiiPgqslSchema::TYPE_JSONB]) &&
            (is_object($value) || is_array($value))
        ) {
            return Json::encode($value);
        }

The problem is it isn't passing in the $columnType when calling Db::prepareValueForDb() so it always encodes the value before passing it to Yii2... which then also encodes the value because it's a JSON column.

So you end up with stored values that are double-encoded as JSON.

Here's the Field in question: https://github.com/lsst-epo/canto-dam-assets/blob/develop-v4/src/fields/CantoDamAsset.php

Steps to reproduce

  1. Have a custom Field that has a content column type of Schema::TYPE_JSON
  2. Save an object or array into that field's value

Expected behavior

The value will only be encoded once as JSON

Actual behavior

The value is actually doubly encoded as JSON

Craft CMS version

4.5.5

PHP version

n/a

Operating system and version

n/a

Database type and version

MySQL 8.0.32

Image driver and version

n/a

Installed plugins and versions

- n/a

khalwat commented 1 year ago

This PR fixes this issue: https://github.com/craftcms/cms/pull/13920

brandonkelly commented 1 year ago

Fixed for 4.6 via #13920 / d02708c52b1f570b37cc31fc8c4b58329e9a9fcd.

brandonkelly commented 10 months ago

Craft 4.6.0 is out now with that change. Use with caution, though!

khalwat commented 10 months ago

What's the caution, MariaDB issues?

brandonkelly commented 10 months ago

Right