From 7daef2617fc8d32343e5a856c1a63af9cb6a3b84 Mon Sep 17 00:00:00 2001 From: Regan Lawton Date: Fri, 1 May 2026 11:11:02 +1000 Subject: [PATCH] fix: reject HTML embed codes in OembedField and OembedService to prevent invalid URLs (fixes #181) --- CHANGELOG.md | 13 ++++ composer.json | 2 +- src/fields/OembedField.php | 72 ++++++++++++++++++++--- src/services/OembedService.php | 6 ++ tests/unit/fields/OembedFieldTest.php | 42 +++++++++++++ tests/unit/services/OembedServiceTest.php | 28 +++++++++ 6 files changed, 154 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b018970..de840cb 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # oEmbed Changelog +## 3.2.2 - 2026-05-01 + +### Fixed + +- Pasting an HTML embed code (e.g. from Vimeo or YouTube) instead of a URL no longer breaks the entry edit page (fixes #181). Thanks @tomfischerNL +- HTML embed codes are now rejected in `normalizeValue()` and `OembedService::embed()` — including any bad data already stored in the database — so they are never saved or rendered +- The CP field input value is now properly HTML-escaped, preventing HTML injection from malformed stored values +- A clear validation error ("Please enter a URL, not an HTML embed code.") is now surfaced inline in the Control Panel when an embed code is submitted + +### Added + +- Unit tests for HTML embed code rejection in `OembedFieldTest` and `OembedServiceTest` + ## 3.2.1 - 2026-03-03 ### Fixed diff --git a/composer.json b/composer.json index 06b67b4..c001d36 100755 --- a/composer.json +++ b/composer.json @@ -2,7 +2,7 @@ "name": "wrav/oembed", "description": "A simple plugin to extract media information from websites, like youtube videos, twitter statuses or blog articles.", "type": "craft-plugin", - "version": "3.2.1", + "version": "3.2.2", "keywords": [ "craft", "cms", diff --git a/src/fields/OembedField.php b/src/fields/OembedField.php index 2a59a6f..c322353 100644 --- a/src/fields/OembedField.php +++ b/src/fields/OembedField.php @@ -134,6 +134,14 @@ public function getContentGqlType(): \GraphQL\Type\Definition\Type|array ]; } + /** + * Detect whether a string contains HTML tags (e.g. an embed code pasted instead of a URL). + */ + private function isHtmlEmbedCode(string $value): bool + { + return trim($value) !== strip_tags(trim($value)); + } + /** * Normalize a URL by adding https:// scheme if missing. * @@ -170,7 +178,7 @@ private function normalizeUrl(string $url): string */ public function normalizeValue(mixed $value, ?craft\base\ElementInterface $element = null): mixed { - // If null, don’t proceed + // If null, don't proceed if ($value === null) { if ($this->required) { return null; @@ -182,10 +190,13 @@ public function normalizeValue(mixed $value, ?craft\base\ElementInterface $eleme // If an instance of `OembedModel` and URL is set, return it if ($value instanceof OembedModel && $value->url) { $normalizedUrl = $this->normalizeUrl($value->url); + if ($this->isHtmlEmbedCode($normalizedUrl)) { + return new OembedModel(null); + } if (UrlHelper::isFullUrl($normalizedUrl)) { return $this->value = new OembedModel($normalizedUrl); } else { - // If we get here, something’s gone wrong + // If we get here, something's gone wrong return new OembedModel(null); } } @@ -193,7 +204,7 @@ public function normalizeValue(mixed $value, ?craft\base\ElementInterface $eleme // If JSON object string, decode it and use that as the value $value = Json::decodeIfJson($value); // Returns an array - // If array with `url` attribute, that’s our url so update the value + // If array with `url` attribute, that's our url so update the value // Run `getValue` to avoid https://github.com/wrav/oembed/issues/74 while(is_array($value)) { $value = ArrayHelper::getValue($value, 'url'); @@ -204,12 +215,17 @@ public function normalizeValue(mixed $value, ?craft\base\ElementInterface $eleme $value = $this->normalizeUrl($value); } + // Reject HTML embed codes — catches raw HTML and any already-prefixed bad data (fixes #181) + if (is_string($value) && $this->isHtmlEmbedCode($value)) { + return new OembedModel(null); + } + // If URL string, return an instance of `OembedModel` if (is_string($value) && UrlHelper::isFullUrl($value)) { return $this->value = new OembedModel($value); } - // If we get here, something’s gone wrong + // If we get here, something's gone wrong return new OembedModel(null); } @@ -217,7 +233,7 @@ public function normalizeValue(mixed $value, ?craft\base\ElementInterface $eleme * Modifies an element query. * * @param ElementInterface $query The element query - * @param mixed $value The value that was set on this field’s corresponding [[ElementCriteriaModel]] + * @param mixed $value The value that was set on this field's corresponding [[ElementCriteriaModel]] * param, if any. * @return null|false `false` in the event that the method is sure that no elements are going to be found. */ @@ -234,9 +250,35 @@ public function getSettingsHtml(): ?string return null; } + /** + * @inheritdoc + */ + public function getElementValidationRules(): array + { + $rules = parent::getElementValidationRules(); + $handle = $this->handle; + + $rules[] = [ + $handle, + function($attribute, $params) use ($handle) { + $request = Craft::$app->getRequest(); + if (!$request instanceof \craft\web\Request || !$request->getIsPost()) { + return; + } + $fields = $request->getBodyParam('fields', []); + $rawValue = $fields[$handle] ?? null; + if (is_string($rawValue) && $rawValue !== strip_tags($rawValue)) { + $this->addError($attribute, Craft::t('oembed', 'Please enter a URL, not an HTML embed code.')); + } + }, + ]; + + return $rules; + } + /** * @param ElementInterface|null $element The element the field is associated with, if there is one - * @param mixed $value The field’s value. This will either be the [[normalizeValue() normalized + * @param mixed $value The field's value. This will either be the [[normalizeValue() normalized * value]], raw POST data (i.e. if there was a validation error), or null * @return string The input HTML. */ @@ -246,13 +288,27 @@ public function getInputHtml($value, ElementInterface $element = null): string $hidden = $settings['previewHidden']; $previewIcon = $hidden ? 'expand' : 'collapse'; + // Safely extract the URL string and detect HTML embed codes (#181) + $urlString = ''; + $isHtmlInput = false; + if ($value instanceof OembedModel) { + $urlString = is_string($value->url) ? $value->url : ''; + } elseif (is_string($value)) { + if ($this->isHtmlEmbedCode($value)) { + $isHtmlInput = true; + } else { + $urlString = $value; + } + } - $input = ''; + $input = ''; $preview = '

Preview

'; - if ($value) { + if ($isHtmlInput) { + $preview .= '

'.Craft::t('oembed', 'Please enter a URL, not an HTML embed code.').'

'; + } elseif ($value) { try { if ($embed = new OembedModel($value)) { $embed = $embed->embed(); diff --git a/src/services/OembedService.php b/src/services/OembedService.php index 4d8610a..cbb72c0 100755 --- a/src/services/OembedService.php +++ b/src/services/OembedService.php @@ -578,6 +578,12 @@ public function embed($url, array $options = [], array $cacheProps = [], $factor $url = $url ?: ''; if (is_string($url)) { + // Reject HTML embed codes before attempting any fetch (fixes #181) + $trimmed = trim($url); + if ($trimmed !== strip_tags($trimmed)) { + return null; + } + $url = $this->normalizeUrl($url); } diff --git a/tests/unit/fields/OembedFieldTest.php b/tests/unit/fields/OembedFieldTest.php index 48e8278..2426065 100644 --- a/tests/unit/fields/OembedFieldTest.php +++ b/tests/unit/fields/OembedFieldTest.php @@ -93,4 +93,46 @@ public function testNormalizeValueWithJsonString() $this->assertInstanceOf(OembedModel::class, $result); $this->assertEquals('https://youtube.com/watch?v=dQw4w9WgXcQ', $result->url); } + + /** + * Test that HTML embed codes are rejected and return an empty OembedModel (fixes #181) + * @dataProvider htmlEmbedCodeProvider + */ + public function testNormalizeValueRejectsHtmlEmbedCode(string $input) + { + $result = $this->field->normalizeValue($input, null); + + $this->assertInstanceOf(OembedModel::class, $result); + $this->assertEmpty($result->url, 'HTML embed code should produce an empty URL, not be stored or rendered'); + } + + public static function htmlEmbedCodeProvider(): array + { + return [ + 'vimeo full embed code' => [ + '
', + ], + 'youtube iframe embed' => [ + '', + ], + 'bare iframe tag' => [ + '', + ], + 'already-prefixed bad data from db' => [ + 'https://
', + ], + ]; + } + + /** + * Test that an OembedModel containing an HTML embed code URL is also rejected + */ + public function testNormalizeValueRejectsOembedModelWithHtmlUrl() + { + $model = new OembedModel('https://
'); + $result = $this->field->normalizeValue($model, null); + + $this->assertInstanceOf(OembedModel::class, $result); + $this->assertEmpty($result->url); + } } diff --git a/tests/unit/services/OembedServiceTest.php b/tests/unit/services/OembedServiceTest.php index c1cdb4a..ce127bb 100644 --- a/tests/unit/services/OembedServiceTest.php +++ b/tests/unit/services/OembedServiceTest.php @@ -223,6 +223,34 @@ public function testRenderHandlesEmptyUrl() $this->assertInstanceOf(\Twig\Markup::class, $result); } + /** + * Test that HTML embed codes are rejected immediately without attempting a fetch (fixes #181) + * @dataProvider htmlEmbedCodeProvider + */ + public function testEmbedRejectsHtmlEmbedCode(string $input) + { + $result = $this->service->embed($input); + $this->assertNull($result, 'embed() must return null for HTML embed codes, not attempt a fetch'); + } + + public static function htmlEmbedCodeProvider(): array + { + return [ + 'vimeo full embed code' => [ + '
', + ], + 'youtube iframe embed' => [ + '', + ], + 'bare iframe' => [ + '', + ], + 'already-prefixed html stored in db' => [ + 'https://
', + ], + ]; + } + /** * Test broken URL notification system handles empty URLs correctly */