[5.x]: asModelSuccess() / asModelFailure() duplicate list data when $data reuses the model name as its key (regression in 5.10.13)

[john-henry]

What happened?

Description

In 5.10.13, craft\web\Controller::asModelSuccess() and asModelFailure() changed from PHP's += union operator to ArrayHelper::merge() when combining the model's own serialisation with the caller-supplied $data.

When a caller passes $data under the same key as $modelName — a common pattern when the controller wants to return a richer serialisation than $model->toArray() produces — the two versions no longer replace one another. ArrayHelper::merge() recurses into them, and because Yii appends rather than overwrites when an integer key already exists (BaseArrayHelper::merge(), lines 131–136), every integer-indexed array inside gets concatenated instead of replaced.

The result is a JSON payload where each list entry appears twice: once from $model->toArray() and once from the caller's version. The two copies are not identical — the model's copy is missing any extra fields the caller requested — so consumers that assume a uniform shape break on the first copy.

Steps to reproduce

use craft\helpers\ArrayHelper;

// What the element serialises to, without any extra fields
$modelData = ['lineItems' => [['id' => 1, 'qty' => 2]]];

// What the controller supplies, under the SAME key as $modelName
$data = ['cart' => ['lineItems' => [['id' => 1, 'qty' => 2, 'snapshot' => ['sku' => 'ABC']]]]];

// 5.10.12 behaviour
$before = $data + array_filter(['modelName' => 'cart', 'cart' => $modelData]);

// 5.10.13 behaviour
$after = ArrayHelper::merge(array_filter(['modelName' => 'cart', 'cart' => $modelData]), $data);

count($before['cart']['lineItems']); // 1
count($after['cart']['lineItems']);  // 2

$after['cart']['lineItems'] is:

[
    { "id": 1, "qty": 2 },
    { "id": 1, "qty": 2, "snapshot": { "sku": "ABC" } }
]

Expected behavior

One entry per line item — the caller's version, as in 5.10.12.

Actual behavior

Two entries per line item. The first is the model's own serialisation and lacks the extra fields the caller asked for.

The change

src/web/Controller.php, asModelSuccess():

-        $data += array_filter([
+        $data = ArrayHelper::merge(array_filter([
             'modelName' => $modelName,
             'modelClass' => get_class($model),
             ($modelName ?? 'model') => $modelData,
-        ]);
+        ]), $data);

asModelFailure() was changed the same way:

-        $data += [
+        $data = ArrayHelper::merge([
             'modelName' => $modelName,
             $modelName => $model->toArray(),
             'errors' => $model->getErrors(),
-        ];
+        ], $data);

Under +=, a key already present in $data was left untouched, so the caller's serialisation won outright. Under ArrayHelper::merge() both are retained and merged element by element.

I appreciate the change likely exists so callers can add to the model payload rather than replace it wholesale. The problem is that it silently corrupts lists rather than deep-merging them, and there is no way for a caller to opt out short of not using asModelSuccess() at all.

Real-world impact

This surfaced as a total checkout outage on a Craft Commerce site.

craft\commerce\controllers\CartController calls:

return $this->asModelSuccess(
    $this->_cart,
    $message,
    'cart',
    [$this->_cartVariable => $this->cartArray($this->_cart)]
);

$this->_cartVariable defaults to 'cart' — the same value passed as $modelName. cartArray() serialises the order with lineItems.snapshot as an extra field; $model->toArray() does not include it.

So from 5.10.13 onwards, cart.lineItems in every AJAX cart response contains each line item twice, and the first copy has no snapshot key. Any front-end code iterating that list and reading snapshot throws on the first entry. Commerce itself is unchanged — 5.7.1 and 5.7.2 both behave this way once Craft is on 5.10.13.

Because asModelSuccess() is a shared base-controller method, anything following the same pattern is affected, not just Commerce.

Suggested fix

Either restore the previous precedence for keys the caller has already set, or use ReplaceArrayValue semantics for the model key so the caller's serialisation wins when both are present.

Additional info

• Craft version: 5.10.13.2 (regression introduced in 5.10.13; 5.10.12 is unaffected)
• PHP version: 8.3
• Database: MySQL 8.0
• Plugin that surfaced it: Craft Commerce 5.7.2 (also reproduces on 5.7.1)

Craft CMS version

5.10.13.2

PHP version

8.3

Operating system and version

No response

Database type and version

MySQL 8.0

Image driver and version

No response

Installed plugins and versions

[linear-code]

CMS-2362

[brandonkelly]

Thanks for reporting that! Craft 5.11.0 is out with a fix.