From 0937eae8e4621b665469495c03cfc7e34b98c5df Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 12:41:46 -0400 Subject: [PATCH 1/4] Skip encoding detection when the relationship config is already UTF-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base64-encoded fieldtype config was always run through mb_convert_encoding() with the full mb_list_encodings() detection list. That's ~9.6us per request, and it can misdetect perfectly valid UTF-8 as another encoding and corrupt it — a config containing only emoji came back as Cyrillic mojibake. Checking mb_check_encoding() first skips the detection entirely for valid UTF-8, which is the normal case, and leaves the existing conversion in place as the fallback for genuinely non-UTF-8 input. --- .../RelationshipFieldtypeController.php | 15 ++-- .../RelationshipFieldtypeControllerTest.php | 74 +++++++++++++++++++ 2 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 tests/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeControllerTest.php diff --git a/src/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeController.php b/src/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeController.php index 899feef822b..a138e037fee 100644 --- a/src/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeController.php +++ b/src/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeController.php @@ -56,12 +56,15 @@ private function getConfig($request) // The json may include unicode characters, so we'll try to convert it to UTF-8. // See https://github.com/statamic/cms/issues/566 - $utf8 = mb_convert_encoding($json, 'UTF-8', mb_list_encodings()); - - // In PHP 8.1 there's a bug where encoding will return null. It's fixed in 8.1.2. - // In this case, we'll fall back to the original JSON, but without the encoding. - // Issue #566 may still occur, but it's better than failing completely. - $json = empty($utf8) ? $json : $utf8; + // Fast path: skip encoding detection when already valid UTF-8. + if (! mb_check_encoding($json, 'UTF-8')) { + $utf8 = mb_convert_encoding($json, 'UTF-8', mb_list_encodings()); + + // In PHP 8.1 there's a bug where encoding will return null. It's fixed in 8.1.2. + // In this case, we'll fall back to the original JSON, but without the encoding. + // Issue #566 may still occur, but it's better than failing completely. + $json = empty($utf8) ? $json : $utf8; + } return json_decode($json, true); } diff --git a/tests/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeControllerTest.php b/tests/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeControllerTest.php new file mode 100644 index 00000000000..178050352ca --- /dev/null +++ b/tests/Http/Controllers/CP/Fieldtypes/RelationshipFieldtypeControllerTest.php @@ -0,0 +1,74 @@ +save(); + EntryFactory::id('123')->collection('blog')->slug('one')->data(['title' => 'One'])->create(); + } + + private function request($config, $selections = ['123']) + { + return $this + ->actingAs(Facades\User::make()->makeSuper()) + ->post(cp_route('relationship.data'), [ + // JSON.stringify() in the browser leaves multibyte characters as-is, + // so don't let PHP escape them to \uXXXX sequences here. + 'config' => base64_encode(json_encode($config, JSON_UNESCAPED_UNICODE)), + 'selections' => $selections, + ]); + } + + #[Test] + public function it_gets_item_data_for_the_configured_fieldtype() + { + $this->request(['type' => 'entries', 'collections' => ['blog']]) + ->assertOk() + ->assertJsonPath('data.0.title', 'One'); + } + + #[Test] + public function it_doesnt_mangle_multibyte_characters_in_the_config() + { + // The base64-encoded JSON config used to be run through mb_convert_encoding() + // with a detection list, which could misdetect perfectly valid UTF-8 as another + // encoding and corrupt it. Emoji were mangled into Cyrillic. See #566. + (new class extends Relationship + { + public static function handle() + { + return 'config_echo'; + } + + protected function toItemArray($id) + { + return ['id' => $id, 'title' => $this->config('display')]; + } + + public function getIndexItems($request) + { + return collect(); + } + })::register(); + + $display = '😀👍'; + + $this->request(['type' => 'config_echo', 'display' => $display]) + ->assertOk() + ->assertJsonPath('data.0.title', $display); + } +} From d99e9a5babd41699298760ece1a3831bdadfe29a Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 12:41:52 -0400 Subject: [PATCH 2/4] Avoid resolving fields twice in Fields::newInstance() setItems() resolves every field into a Field object, and the very next call in the chain, setFields(), immediately threw that work away. Assigning the items directly skips the wasted resolution. --- src/Fields/Fields.php | 8 ++++++-- tests/Fields/FieldsTest.php | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/Fields/Fields.php b/src/Fields/Fields.php index f6707b50866..aab43494431 100644 --- a/src/Fields/Fields.php +++ b/src/Fields/Fields.php @@ -118,10 +118,14 @@ public function only(...$keys): self public function newInstance() { - return (new static) + // Assign items directly — setItems() would re-resolve every field, then + // setFields() would immediately discard that work. + $instance = new static; + $instance->items = $this->items; + + return $instance ->setParent($this->parent) ->setParentField($this->parentField, $this->parentIndex) - ->setItems($this->items) ->setFields($this->fields) ->setFilled($this->filled); } diff --git a/tests/Fields/FieldsTest.php b/tests/Fields/FieldsTest.php index 1d7f9532d77..448ce1ddb3f 100644 --- a/tests/Fields/FieldsTest.php +++ b/tests/Fields/FieldsTest.php @@ -371,6 +371,45 @@ public function it_gets_only_specific_fields() $this->assertEquals(['one', 'two'], $fields->only(['one', 'two'])->all()->keys()->all()); } + #[Test] + public function it_carries_items_over_to_a_new_instance_without_resolving_them_again() + { + $items = [ + ['handle' => 'one', 'field' => ['display' => 'First']], + ['handle' => 'two', 'field' => ['display' => 'Second']], + ]; + + $fields = new Fields($items); + $instance = $fields->newInstance(); + + $this->assertNotSame($fields, $instance); + $this->assertEquals($items, $instance->items()->all()); + + // The already resolved fields get carried over as-is, rather than being + // resolved a second time only for setFields() to throw them away. + $this->assertSame($fields->get('one'), $instance->get('one')); + $this->assertSame($fields->get('two'), $instance->get('two')); + } + + #[Test] + public function replacing_the_items_on_a_new_instance_doesnt_affect_the_original() + { + $fields = new Fields([ + ['handle' => 'one', 'field' => ['display' => 'First']], + ]); + + $instance = $fields->newInstance(); + + $instance->setItems([ + ['handle' => 'two', 'field' => ['display' => 'Second']], + ]); + + $this->assertEquals(['one'], $fields->items()->pluck('handle')->all()); + $this->assertEquals(['two'], $instance->items()->pluck('handle')->all()); + $this->assertTrue($fields->has('one')); + $this->assertFalse($fields->has('two')); + } + #[Test] public function converts_to_array_suitable_for_rendering_fields_in_publish_component() { From 2535d30de99905d11e5ee5b2a77d5e462a7c6c1b Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 12:41:59 -0400 Subject: [PATCH 3/4] Memoize relationship item lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getItemData() looked each id up twice — once in authorizeItemData() to check the user can view it, and again in toItemArray() to build the response. Caching the found item on the instance halves the lookups. The cache is reset in __clone(), since the fieldtype repository hands out clones of a single instance per handle. Without it the first consumer's lookups get baked into the shared instance and inherited by every unrelated field of the same type. Authorization is unaffected — the cache holds the item, not a decision, so authorizeViewable() still runs on every call. --- src/Fieldtypes/Entries.php | 9 +++++++-- src/Fieldtypes/Relationship.php | 8 ++++++++ src/Fieldtypes/Terms.php | 28 +++++++++++++++++++--------- src/Fieldtypes/Users.php | 9 +++++++-- tests/Fieldtypes/EntriesTest.php | 18 ++++++++++++++++++ 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/Fieldtypes/Entries.php b/src/Fieldtypes/Entries.php index 402540e0a9d..8b070789cac 100644 --- a/src/Fieldtypes/Entries.php +++ b/src/Fieldtypes/Entries.php @@ -379,18 +379,23 @@ protected function getCreatables() protected function authorizeItemData($id): bool { - return $this->authorizeViewable(Entry::find($id)); + return $this->authorizeViewable($this->findEntry($id)); } protected function toItemArray($id) { - if (! $entry = Entry::find($id)) { + if (! $entry = $this->findEntry($id)) { return $this->invalidItemArray($id); } return (new EntryResource($entry, $this))->resolve()['data']; } + protected function findEntry($id) + { + return $this->itemCache[$id] ??= Entry::find($id); + } + protected function collect($value) { return new \Statamic\Entries\EntryCollection($value); diff --git a/src/Fieldtypes/Relationship.php b/src/Fieldtypes/Relationship.php index d7cf287998c..16d93057144 100644 --- a/src/Fieldtypes/Relationship.php +++ b/src/Fieldtypes/Relationship.php @@ -31,6 +31,14 @@ abstract class Relationship extends Fieldtype '_' => '_', // forces an object in js ]; protected $formStackSize; + protected array $itemCache = []; + + public function __clone() + { + // The fieldtype repository hands out clones of a single instance per handle, + // so without this the cache would be inherited by unrelated fields. + $this->itemCache = []; + } protected function configFieldItems(): array { diff --git a/src/Fieldtypes/Terms.php b/src/Fieldtypes/Terms.php index 1501d4c29c2..7e608fd8ca0 100644 --- a/src/Fieldtypes/Terms.php +++ b/src/Fieldtypes/Terms.php @@ -418,20 +418,14 @@ protected function getCreatables() protected function authorizeItemData($id): bool { - if ($this->usingSingleTaxonomy() && ! Str::contains($id, '::')) { - $id = "{$this->taxonomies()[0]}::{$id}"; - } - - return $this->authorizeViewable(Term::find($id)); + return $this->authorizeViewable($this->findTerm($id)); } protected function toItemArray($id) { - if ($this->usingSingleTaxonomy() && ! Str::contains($id, '::')) { - $id = "{$this->taxonomies()[0]}::{$id}"; - } + $id = $this->normalizeTermId($id); - if (! $term = Term::find($id)) { + if (! $term = $this->findTerm($id)) { return $this->invalidItemArray($id); } @@ -457,6 +451,22 @@ protected function toItemArray($id) ]; } + protected function normalizeTermId($id): string + { + if ($this->usingSingleTaxonomy() && ! Str::contains($id, '::')) { + return "{$this->taxonomies()[0]}::{$id}"; + } + + return $id; + } + + protected function findTerm($id) + { + $id = $this->normalizeTermId($id); + + return $this->itemCache[$id] ??= Term::find($id); + } + protected function getColumns() { $columns = [Column::make('title')]; diff --git a/src/Fieldtypes/Users.php b/src/Fieldtypes/Users.php index 298771dc765..35597809570 100644 --- a/src/Fieldtypes/Users.php +++ b/src/Fieldtypes/Users.php @@ -104,12 +104,12 @@ public function preProcess($data) protected function authorizeItemData($id): bool { - return $this->authorizeViewable(User::find($id)); + return $this->authorizeViewable($this->findUser($id)); } protected function toItemArray($id, $site = null) { - if ($user = User::find($id)) { + if ($user = $this->findUser($id)) { $canViewUsers = $this->canViewUser($user); return [ @@ -123,6 +123,11 @@ protected function toItemArray($id, $site = null) return $this->invalidItemArray($id); } + protected function findUser($id) + { + return $this->itemCache[$id] ??= User::find($id); + } + public function getIndexItems($request) { // Don't reveal existence to a user who can't view the listing; return an empty diff --git a/tests/Fieldtypes/EntriesTest.php b/tests/Fieldtypes/EntriesTest.php index 7416ca21d04..752d5bcab2d 100644 --- a/tests/Fieldtypes/EntriesTest.php +++ b/tests/Fieldtypes/EntriesTest.php @@ -376,6 +376,24 @@ public function it_doesnt_localize_when_select_across_sites_setting_is_enabled() $this->assertEquals(['one', 'two', 'three', 'four'], $augmented->get()->map->slug()->all()); } + #[Test] + public function it_doesnt_inherit_the_item_cache_from_another_fieldtype_instance() + { + $this->actingAs(Facades\User::make()->makeSuper()); + + $field = new Field('test', ['type' => 'entries', 'collections' => ['blog']]); + + // The fieldtype repository hands out clones of a single instance per handle, + // so one field's lookups must not leak into another's. + $first = $field->fieldtype(); + $this->assertFalse($first->getItemData(['123'])->first()['invalid'] ?? false); + + Facades\Entry::find('123')->delete(); + + $second = $field->fieldtype(); + $this->assertTrue($second->getItemData(['123'])->first()['invalid']); + } + public function fieldtype($config = [], $parent = null) { $field = new Field('test', array_merge([ From 72a5d1c0cf932e29a2b85c1ed2c16d1d754b9df0 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 13:24:09 -0400 Subject: [PATCH 4/4] Clone the items collection in Fields::newInstance() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assigning the collection directly left both instances sharing one collection object, where setItems() would have given the new instance its own. Nothing in core mutates it in place, but addons can subclass Fields and reach it through items(), and a clone costs one allocation against the resolveFields() call we're skipping. Note this is shallow, matching what setItems() did — items can contain validation rule objects, and those stay shared either way. --- src/Fields/Fields.php | 7 ++++--- tests/Fields/FieldsTest.php | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Fields/Fields.php b/src/Fields/Fields.php index aab43494431..e1531f92b89 100644 --- a/src/Fields/Fields.php +++ b/src/Fields/Fields.php @@ -118,10 +118,11 @@ public function only(...$keys): self public function newInstance() { - // Assign items directly — setItems() would re-resolve every field, then - // setFields() would immediately discard that work. + // Assign the items directly — setItems() would re-resolve every field, then + // setFields() would immediately discard that work. Clone so the two instances + // don't share one collection, which is what setItems() would have given us. $instance = new static; - $instance->items = $this->items; + $instance->items = clone $this->items; return $instance ->setParent($this->parent) diff --git a/tests/Fields/FieldsTest.php b/tests/Fields/FieldsTest.php index 448ce1ddb3f..711b27acb3e 100644 --- a/tests/Fields/FieldsTest.php +++ b/tests/Fields/FieldsTest.php @@ -410,6 +410,24 @@ public function replacing_the_items_on_a_new_instance_doesnt_affect_the_original $this->assertFalse($fields->has('two')); } + #[Test] + public function mutating_the_items_on_a_new_instance_doesnt_affect_the_original() + { + $fields = new Fields([ + ['handle' => 'one', 'field' => ['display' => 'First']], + ]); + + $instance = $fields->newInstance(); + + $this->assertNotSame($fields->items(), $instance->items()); + + $instance->items()->push(['handle' => 'two', 'field' => ['display' => 'Second']]); + $instance->items()->put(0, ['handle' => 'clobbered', 'field' => ['display' => 'Clobbered']]); + + $this->assertEquals(['one'], $fields->items()->pluck('handle')->all()); + $this->assertEquals(['clobbered', 'two'], $instance->items()->pluck('handle')->all()); + } + #[Test] public function converts_to_array_suitable_for_rendering_fields_in_publish_component() {