Merge branch '5.x' into 5.5

craftcms · Sep 6, 2024 · eda79c2 · eda79c2
2 parents 9d4d7b6 + 94b14ce
commit eda79c2
Show file tree

Hide file tree

Showing 18 changed files with 91 additions and 76 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,8 +2,15 @@
 
 ## Unreleased
 
+- Added `craft\services\Security::isSystemDir()`.
 - Fixed a bug where `craft\helpers\StringHelper::lines()` was returning an array of `Stringy\Stringy` objects, rather than strings.
 - Fixed styling issues with Template field layout UI elements’ selector labels.
+- Fixed a validation error that could occur when saving a relational field, if the “Maintain hierarchy” setting had been enabled but was no longer applicable. ([#15666](https://github.com/craftcms/cms/issues/15666))
+- Fixed a PHP error that occurred when running PHP 8.2 or 8.3.
+- Fixed a bug where disabled entries became enabled when edited within Live Preview. ([#15670](https://github.com/craftcms/cms/issues/15670))
+- Fixed a bug where new nested entries could get incremented slugs even if there were no elements with conflicting URIs. ([#15672](https://github.com/craftcms/cms/issues/15672))
+- Fixed a bug where Addresses fields weren’t always returning data in GraphQL.
+- Fixed an information disclosure vulnerability.
 
 ## 5.4.1 - 2024-09-04
 

diff --git a/src/controllers/ElementsController.php b/src/controllers/ElementsController.php
@@ -114,7 +114,7 @@ public function beforeAction($action): bool
         $this->_draftId = $this->_param('draftId');
         $this->_revisionId = $this->_param('revisionId');
         $this->_siteId = $this->_param('siteId');
-        $this->_enabled = $this->_param('enabled', true);
+        $this->_enabled = $this->_param('enabled', $this->_param('setEnabled', true) ? true : null);
         $this->_enabledForSite = $this->_param('enabledForSite');
         $this->_slug = $this->_param('slug');
         $this->_fresh = (bool)$this->_param('fresh');

diff --git a/src/fieldlayoutelements/Template.php b/src/fieldlayoutelements/Template.php
@@ -130,8 +130,6 @@ private function _error(string $error, string $errorClass): string
             'class' => $errorClass,
         ]);
 
-        return Html::tag('div', $content, [
-            'class' => 'pane',
-        ]);
+        return Html::tag('div', $content);
     }
 }
diff --git a/src/fields/BaseRelationField.php b/src/fields/BaseRelationField.php
@@ -408,7 +408,7 @@ public function validateSources(string $attribute): void
         $inputSources = $this->getInputSources();
 
         if ($inputSources === null) {
-            $this->addError($attribute, Craft::t('app', 'A source is required when relating ancestors.'));
+            $this->maintainHierarchy = false;
             return;
         }
 
@@ -423,19 +423,14 @@ public function validateSources(string $attribute): void
         );
 
         if (count($elementSources) > 1) {
-            $this->addError($attribute, Craft::t('app', 'Only one source is allowed when relating ancestors.'));
+            $this->maintainHierarchy = false;
+            return;
         }
 
         foreach ($elementSources as $elementSource) {
             if (!isset($elementSource['structureId'])) {
-                $this->addError(
-                    $attribute,
-                    Craft::t(
-                        'app',
-                        '{source} is not a structured source. Only structured sources may be used when relating ancestors.',
-                        ['source' => $elementSource['label']]
-                    )
-                );
+                $this->maintainHierarchy = false;
+                return;
             }
         }
     }

diff --git a/src/fs/Local.php b/src/fs/Local.php
@@ -130,21 +130,8 @@ protected function defineRules(): array
      */
     public function validatePath(string $attribute, ?array $params, InlineValidator $validator): void
     {
-        // Make sure it’s not within any of the system directories
-        $path = FileHelper::absolutePath($this->getRootPath(), '/');
-
-        $systemDirs = Craft::$app->getPath()->getSystemPaths();
-
-        foreach ($systemDirs as $dir) {
-            $dir = FileHelper::absolutePath($dir, '/');
-            if (str_starts_with("$path/", "$dir/")) {
-                $validator->addError($this, $attribute, Craft::t('app', 'Local filesystems cannot be located within system directories.'));
-                break;
-            }
-            if (str_starts_with("$dir/", "$path/")) {
-                $validator->addError($this, $attribute, Craft::t('app', 'Local filesystems cannot be located above system directories.'));
-                break;
-            }
+        if (Craft::$app->getSecurity()->isSystemDir($this->getRootPath())) {
+            $validator->addError($this, $attribute, Craft::t('app', 'Local filesystems cannot be located within or above system directories.'));
         }
     }
 

diff --git a/src/gql/resolvers/elements/Address.php b/src/gql/resolvers/elements/Address.php
@@ -10,9 +10,7 @@
 use craft\elements\Address as AddressElement;
 use craft\elements\db\AddressQuery;
 use craft\elements\db\ElementQuery;
-use craft\elements\ElementCollection;
 use craft\gql\base\ElementResolver;
-use craft\helpers\Gql as GqlHelper;
 use yii\base\UnknownMethodException;
 
 /**
@@ -52,10 +50,6 @@ public static function prepareQuery(mixed $source, array $arguments, ?string $fi
             }
         }
 
-        if (!GqlHelper::canQueryUsers()) {
-            return ElementCollection::empty();
-        }
-
         return $query;
     }
 }
diff --git a/src/helpers/ElementHelper.php b/src/helpers/ElementHelper.php
@@ -240,6 +240,7 @@ private static function _renderUriFormat(string $uriFormat, ElementInterface $el
     private static function _isUniqueUri(string $testUri, ElementInterface $element): bool
     {
         $query = (new Query())
+            ->select(['elements.id', 'elements.type'])
             ->from(['elements_sites' => Table::ELEMENTS_SITES])
             ->innerJoin(['elements' => Table::ELEMENTS], '[[elements.id]] = [[elements_sites.elementId]]')
             ->where([
@@ -268,7 +269,21 @@ private static function _isUniqueUri(string $testUri, ElementInterface $element)
             ]);
         }
 
-        return (int)$query->count() === 0;
+        $info = $query->all();
+
+        if (empty($info)) {
+            return true;
+        }
+
+        // Make sure the element(s) isn't owned by a draft/revision
+        foreach ($info as $row) {
+            $conflictingElement = Craft::$app->getElements()->getElementById($row['id'], $row['type'], $element->siteId);
+            if ($conflictingElement && !static::isDraftOrRevision($conflictingElement)) {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     /**

diff --git a/src/helpers/Html.php b/src/helpers/Html.php
@@ -1018,11 +1018,22 @@ public static function dataUrl(string $file, ?string $mimeType = null): string
             throw new InvalidArgumentException("Invalid file path: $file");
         }
 
+        $file = FileHelper::absolutePath(Craft::getAlias($file), '/');
+
+        if (Craft::$app->getSecurity()->isSystemDir(dirname($file))) {
+            throw new InvalidArgumentException(sprintf('%s cannot be passed a path within or above system directories.', __METHOD__));
+        }
+
+        $ext = pathinfo($file, PATHINFO_EXTENSION);
+        if (strtolower($ext) === 'php') {
+            throw new InvalidArgumentException(sprintf('%s cannot be passed a path to a PHP file.', __METHOD__));
+        }
+
         if ($mimeType === null) {
             try {
                 $mimeType = FileHelper::getMimeType($file);
             } catch (Throwable $e) {
-                Craft::warning("Unable to determine the MIME type for $file: " . $e->getMessage());
+                Craft::warning("Unable to determine the MIME type for $file: " . $e->getMessage(), __METHOD__);
                 Craft::$app->getErrorHandler()->logException($e);
             }
         }

diff --git a/src/services/Fields.php b/src/services/Fields.php
@@ -1497,9 +1497,9 @@ public function getTableData(
                     'icon' => Cp::iconSvg($field::icon()),
                 ],
                 'usages' => isset($usages[$field->id])
-                    ? mb_ucfirst(Craft::t('app', '{count, number} {count, plural, =1{layout} other{layouts}}', [
+                    ? Craft::t('app', '{count, number} {count, plural, =1{layout} other{layouts}}', [
                         'count' => count($usages[$field->id]),
-                    ]))
+                    ])
                     : null,
             ];
         }

diff --git a/src/services/Security.php b/src/services/Security.php
@@ -8,6 +8,7 @@
 namespace craft\services;
 
 use Craft;
+use craft\helpers\FileHelper;
 use yii\base\Exception;
 use yii\base\InvalidArgumentException;
 use yii\base\InvalidConfigException;
@@ -196,4 +197,25 @@ public function redactIfSensitive(string $key, mixed $value): mixed
 
         return $value;
     }
+
+    /**
+     * Returns whether the given file path is located within or above any system directories.
+     *
+     * @param string $path
+     * @return bool
+     * @since 5.4.2
+     */
+    public function isSystemDir(string $path): bool
+    {
+        $path = FileHelper::absolutePath($path, '/');
+
+        foreach (Craft::$app->getPath()->getSystemPaths() as $dir) {
+            $dir = FileHelper::absolutePath($dir, '/');
+            if (str_starts_with("$path/", "$dir/") || str_starts_with("$dir/", "$path/")) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
diff --git a/src/translations/en/app.php b/src/translations/en/app.php
@@ -18,7 +18,6 @@
     'A folder with the name “{folderName}” already exists in the volume.' => 'A folder with the name “{folderName}” already exists in the volume.',
     'A license key is required.' => 'A license key is required.',
     'A server error occurred.' => 'A server error occurred.',
-    'A source is required when relating ancestors.' => 'A source is required when relating ancestors.',
     'A subpath is required for this filesystem.' => 'A subpath is required for this filesystem.',
     'A template name cannot contain NUL bytes.' => 'A template name cannot contain NUL bytes.',
     'Abandoned' => 'Abandoned',
@@ -902,8 +901,7 @@
     'Loading' => 'Loading',
     'Local Folder' => 'Local Folder',
     'Local copies of remote images, generated thumbnails' => 'Local copies of remote images, generated thumbnails',
-    'Local filesystems cannot be located above system directories.' => 'Local filesystems cannot be located above system directories.',
-    'Local filesystems cannot be located within system directories.' => 'Local filesystems cannot be located within system directories.',
+    'Local filesystems cannot be located within or above system directories.' => 'Local filesystems cannot be located within or above system directories.',
     'Locality' => 'Locality',
     'Localizing relations' => 'Localizing relations',
     'Location' => 'Location',
@@ -1096,7 +1094,6 @@
     'One update available!' => 'One update available!',
     'Online' => 'Online',
     'Only allow {type} to be selected if they match the following rules:' => 'Only allow {type} to be selected if they match the following rules:',
-    'Only one source is allowed when relating ancestors.' => 'Only one source is allowed when relating ancestors.',
     'Only save entries to the site they were created in' => 'Only save entries to the site they were created in',
     'Only show for users who match the following rules:' => 'Only show for users who match the following rules:',
     'Only show when editing {type} that match the following rules:' => 'Only show when editing {type} that match the following rules:',
@@ -2142,7 +2139,6 @@
     '{num, plural, =1{Only one author is} other{Up to {num, number} authors are}} allowed.' => '{num, plural, =1{Only one author is} other{Up to {num, number} authors are}} allowed.',
     '{pct} width' => '{pct} width',
     '{plugin} requires Craft CMS {edition} edition.' => '{plugin} requires Craft CMS {edition} edition.',
-    '{source} is not a structured source. Only structured sources may be used when relating ancestors.' => '{source} is not a structured source. Only structured sources may be used when relating ancestors.',
     '{step, number} of {total, number}' => '{step, number} of {total, number}',
     '{title} ({site})' => '{title} ({site})',
     '{total, number} {total, plural, =1{error} other{errors}} found in {num, number} {num, plural, =1{tab} other{tabs}}.' => '{total, number} {total, plural, =1{error} other{errors}} found in {num, number} {num, plural, =1{tab} other{tabs}}.',

diff --git a/src/web/assets/cp/dist/cp.js b/src/web/assets/cp/dist/cp.js
diff --git a/src/web/assets/cp/dist/cp.js.map b/src/web/assets/cp/dist/cp.js.map
diff --git a/src/web/assets/cp/src/js/ElementEditor.js b/src/web/assets/cp/src/js/ElementEditor.js
@@ -1648,6 +1648,10 @@ Craft.ElementEditor = Garnish.Base.extend(
         );
       }
 
+      for (const [name, value] of Object.entries(this.settings.saveParams)) {
+        params.push(`${this.namespaceInputName(name)}=${value}`);
+      }
+
       return asArray ? params : params.join('&');
     },
 
@@ -2296,6 +2300,7 @@ Craft.ElementEditor = Garnish.Base.extend(
       revisionId: null,
       siteId: null,
       siteStatuses: [],
+      saveParams: {},
       siteToken: null,
       visibleLayoutElements: {},
       updatedTimestamp: null,

diff --git a/src/web/assets/cp/src/js/Preview.js b/src/web/assets/cp/src/js/Preview.js
@@ -389,6 +389,9 @@ Craft.Preview = Garnish.Base.extend(
               this.$saveBtn.removeClass('loading');
             },
             autosaveDrafts: true,
+            saveParams: {
+              setEnabled: 0,
+            },
           },
           this.$editorContainer.data('elementEditorSettings')
         )

diff --git a/src/web/twig/Extension.php b/src/web/twig/Extension.php
@@ -1444,11 +1444,16 @@ public function collectFunction(mixed $var): Collection
      */
     public function dataUrlFunction(Asset|string $file, ?string $mimeType = null): string
     {
-        if ($file instanceof Asset) {
-            return $file->getDataUrl();
-        }
+        try {
+            if ($file instanceof Asset) {
+                return $file->getDataUrl();
+            }
 
-        return Html::dataUrl(Craft::getAlias($file), $mimeType);
+            return Html::dataUrl(Craft::getAlias($file), $mimeType);
+        } catch (InvalidArgumentException $e) {
+            Craft::warning($e->getMessage(), __METHOD__);
+            return '';
+        }
     }
 
     /**

diff --git a/tests/unit/helpers/ElementHelperTest.php b/tests/unit/helpers/ElementHelperTest.php
@@ -7,9 +7,7 @@
 
 namespace crafttests\unit\helpers;
 
-use Codeception\Stub;
 use Craft;
-use craft\db\Command;
 use craft\errors\OperationAbortedException;
 use craft\helpers\ElementHelper;
 use craft\test\mockclasses\elements\ExampleElement;
@@ -94,29 +92,10 @@ public function testDoesUriFormatHaveSlugTag(bool $expected, string $uriFormat):
      * @dataProvider setUniqueUriDataProvider
      * @param array $expected
      * @param array $config
-     * @param int $duplicates
      * @throws OperationAbortedException
      */
-    public function testSetUniqueUri(array $expected, array $config, int $duplicates = 0): void
+    public function testSetUniqueUri(array $expected, array $config): void
     {
-        if ($duplicates) {
-            $db = Craft::$app->getDb();
-            $this->tester->mockDbMethods([
-                'createCommand' => function($sql, $params) use (&$duplicates, &$db) {
-                    /* @var Command $command */
-                    $command = Stub::construct(Command::class, [
-                        ['db' => $db, 'sql' => $sql],
-                    ], [
-                        'queryScalar' => function() use (&$duplicates) {
-                            return $duplicates-- ? 1 : 0;
-                        },
-                    ]);
-                    $command->bindValues($params);
-                    return $command;
-                },
-            ]);
-        }
-
         $example = new ExampleElement($config);
         ElementHelper::setUniqueUri($example);
 
@@ -258,8 +237,6 @@ public static function setUniqueUriDataProvider(): array
             [['uri' => null], ['uriFormat' => null]],
             [['uri' => null], ['uriFormat' => '']],
             [['uri' => 'craft'], ['uriFormat' => '{slug}', 'slug' => 'craft']],
-            [['uri' => 'craft--3'], ['uriFormat' => '{slug}', 'slug' => 'craft'], 2],
-            [['uri' => 'testing-uri-longer-than-255-chars/arrêté-du-24-décembre-2020-portant-modification-de-larrêté-du-4-décembre-2020-fixant-la-liste-des-personnes-autorisées-à-exercer-en-france-la-profession-de-médecin-dans-la-spécialité-gériatrie-en-application-des-dispos--2'], ['uriFormat' => 'testing-uri-longer-than-255-chars/{slug}', 'slug' => 'arrêté-du-24-décembre-2020-portant-modification-de-larrêté-du-4-décembre-2020-fixant-la-liste-des-personnes-autorisées-à-exercer-en-france-la-profession-de-médecin-dans-la-spécialité-gériatrie-en-application-des-dispositions-de-larti'], 1],
             [['uri' => 'test'], ['uriFormat' => 'test/{slug}']],
             [['uri' => 'test/test'], ['uriFormat' => 'test/{slug}', 'slug' => 'test']],
             [['uri' => 'test/tes.!@#$%^&*()_t'], ['uriFormat' => 'test/{slug}', 'slug' => 'tes.!@#$%^&*()_t']],

diff --git a/tests/unit/web/twig/ExtensionTest.php b/tests/unit/web/twig/ExtensionTest.php
@@ -979,9 +979,9 @@ public function testCloneFunction(): void
      */
     public function testDataUrlFunction(): void
     {
-        $path = dirname(__DIR__, 3) . '/_data/assets/files/craft-logo.svg';
+        $path = dirname(__DIR__, 4) . '/.github/workflows/ci.yml';
         $dataUrl = $this->view->renderString('{{ dataUrl(path) }}', compact('path'));
-        self::assertStringStartsWith('data:image/svg+xml;base64,', $dataUrl);
+        self::assertStringStartsWith('data:application/x-yaml;base64,', $dataUrl);
     }
 
     public function testExpressionFunction(): void