diff --git a/src/Integrations/Divi/Compose/DynamicContentRules.php b/src/Integrations/Divi/Compose/DynamicContentRules.php index 3c78568..6ad585c 100644 --- a/src/Integrations/Divi/Compose/DynamicContentRules.php +++ b/src/Integrations/Divi/Compose/DynamicContentRules.php @@ -68,6 +68,9 @@ final class DynamicContentRules private const META_PREFIX = 'custom_meta_'; + /** Divi's internal link options: post_link_url_ (F27). */ + public const LINK_PREFIX = 'post_link_url_'; + /** * Prefixes Divi strips from a meta key option with str_replace – every occurrence, not just * the leading one (DynamicContentOptionPostMetaKey.php:232,249,275,286; @@ -135,8 +138,13 @@ final class DynamicContentRules * * @throws InvalidArgumentException */ - public static function cleanSetting(string $key, mixed $value, string $fieldType, ValueSanitizer $sanitizer): mixed - { + public static function cleanSetting( + string $key, + mixed $value, + string $fieldType, + ValueSanitizer $sanitizer, + string $option = '' + ): mixed { // Ruling S32/S32a: control characters go on the RAW value, before every check and the // sanitizer ("<" + VT stays a literal "<" for strip_tags; a later strip would build a tag). if (is_string($value)) { @@ -149,7 +157,7 @@ final class DynamicContentRules $clean = match ($fieldType) { 'text', 'richtext', 'html', 'url' => self::textSetting($key, $value, $fieldType, $sanitizer), - 'select_post' => self::selectPostSetting($key, $value, $sanitizer), + 'select_post' => self::selectPostSetting($key, $value, $sanitizer, $option), 'select', 'yes_no_button', 'number' => self::scalarSetting($key, $value, $sanitizer), // Ruling S37: map types come from a filter (untrusted) – unknown types are refused. default => throw new InvalidArgumentException(sprintf( @@ -238,19 +246,28 @@ final class DynamicContentRules } /** - * Ruling S37: a numeric post id the user may read – no slugs (Divi resolves those itself). - * Returned as sent (int or digit string). + * Ruling S37/P10: a numeric post id (PostIdParser). In an internal link post_link_url_ the + * id must name a link target of exactly that type (LinkTargets rules, like {{link:}}); in any + * other option a post the user may read. Returned as sent (int or digit string). * * @throws InvalidArgumentException */ - private static function selectPostSetting(string $key, mixed $value, ValueSanitizer $sanitizer): mixed - { - $digits = is_string($value) && preg_match('/^[1-9][0-9]*$/D', $value) === 1; - $id = is_int($value) ? $value : ($digits ? (int) $value : 0); - if ($id <= 0 || ! $sanitizer->canReadPost($id)) { + private static function selectPostSetting( + string $key, + mixed $value, + ValueSanitizer $sanitizer, + string $option + ): mixed { + $id = PostIdParser::parse($value); + $linkType = str_starts_with($option, self::LINK_PREFIX) ? substr($option, strlen(self::LINK_PREFIX)) : null; + $valid = $id !== null && ($linkType === null + ? $sanitizer->canReadPost($id) + : $sanitizer->linkTargetType($id) === $linkType); + if (! $valid) { throw new InvalidArgumentException(sprintf( - 'Einstellung "%s" im Divi-Verweis: Beitrags-ID eines lesbaren Eintrags erwartet.', - $key + 'Einstellung "%s" im Divi-Verweis: Beitrags-ID eines lesbaren Eintrags erwartet%s.', + $key, + $linkType === null ? '' : sprintf(' (Typ „%s“, nicht im Papierkorb, kein Anhang)', $linkType) )); } diff --git a/src/Integrations/Divi/Compose/Placeholders.php b/src/Integrations/Divi/Compose/Placeholders.php index c7439b4..606bc8f 100644 --- a/src/Integrations/Divi/Compose/Placeholders.php +++ b/src/Integrations/Divi/Compose/Placeholders.php @@ -158,15 +158,14 @@ final class Placeholders * {{link:}} → Divi's own internal link post_link_url_ with settings.post_id (F27): * rendered as get_permalink() at view time, so a draft target gets its pretty URL once published. * Whether Divi offers that option (public post types only) VariableRef checks against the registry; - * that the value goes into a URL field and the target is readable (S37) the choke point checks again. + * that the value goes into a URL field and names a link target of that type (P10) the choke point checks again. * * @throws InvalidArgumentException */ private function link(string $name): string { - // FILTER_VALIDATE_INT: no id beyond PHP_INT_MAX that would saturate to another entry. - $id = preg_match('/^[1-9][0-9]{0,18}$/D', $name) === 1 ? filter_var($name, FILTER_VALIDATE_INT) : false; - if (! is_int($id)) { + $id = PostIdParser::parse($name); + if ($id === null) { throw new InvalidArgumentException(sprintf( 'Ungültige Beitrags-ID "%s" in {{link:…}} (positive Zahl erwartet, z. B. {{link:42}}).', $name @@ -181,7 +180,7 @@ final class Placeholders )); } - return VariableRef::serialize('content', 'post_link_url_' . $type, ['post_id' => $name]); + return VariableRef::serialize('content', DynamicContentRules::LINK_PREFIX . $type, ['post_id' => $name]); } private function field(string $name): string diff --git a/src/Integrations/Divi/Compose/PostIdParser.php b/src/Integrations/Divi/Compose/PostIdParser.php new file mode 100644 index 0000000..b6c8658 --- /dev/null +++ b/src/Integrations/Divi/Compose/PostIdParser.php @@ -0,0 +1,31 @@ +}}, + * post_link_url_* settings.post_id and Theme Builder conditions. A positive int, or its plain + * decimal digits without sign, padding or spaces; nothing beyond PHP_INT_MAX (FILTER_VALIDATE_INT + * refuses it instead of saturating to another entry). Pure PHP. + */ +final class PostIdParser +{ + public static function parse(mixed $value): ?int + { + if (is_int($value)) { + return $value > 0 ? $value : null; + } + + if (! is_string($value) || preg_match('/^[1-9][0-9]{0,18}$/D', $value) !== 1) { + return null; + } + + $id = filter_var($value, FILTER_VALIDATE_INT); + + return is_int($id) ? $id : null; + } +} diff --git a/src/Integrations/Divi/Compose/ValueSanitizer.php b/src/Integrations/Divi/Compose/ValueSanitizer.php index 42877e9..f04bee5 100644 --- a/src/Integrations/Divi/Compose/ValueSanitizer.php +++ b/src/Integrations/Divi/Compose/ValueSanitizer.php @@ -34,7 +34,14 @@ interface ValueSanitizer /** * Whether the current user may read post $id: select_post settings of dynamic content - * references (post_link_url_*) name a post whose link Divi renders (ruling S37). + * references outside internal links (ruling S37); post_link_url_* use linkTargetType() (P10). */ public function canReadPost(int $id): bool; + + /** + * Ruling P10: the post type of entry $id when it is a link target – exists, lives (no trash, + * auto-draft or revision/attachment status), is no attachment and is readable – like {{link:}} + * (LinkTargets); else null. + */ + public function linkTargetType(int $id): ?string; } diff --git a/src/Integrations/Divi/Compose/VariableRef.php b/src/Integrations/Divi/Compose/VariableRef.php index 61260e0..9fd7008 100644 --- a/src/Integrations/Divi/Compose/VariableRef.php +++ b/src/Integrations/Divi/Compose/VariableRef.php @@ -252,7 +252,7 @@ final class VariableRef */ private function contentSettings(string $name, array $settings, ValueSanitizer $sanitizer, bool $url): array { - if (! $url && str_starts_with($name, 'post_link_url_')) { + if (! $url && str_starts_with($name, DynamicContentRules::LINK_PREFIX)) { throw new InvalidArgumentException(sprintf( '"%s" (Link auf einen Eintrag, {{link:…}}) nur in Link- und URL-Feldern.', $name @@ -269,7 +269,7 @@ final class VariableRef if ($url && ($key === 'before' || $key === 'after') && $value !== '') { throw new InvalidArgumentException(sprintf('"%s" im Divi-Verweis: in URL-Feldern verboten.', $key)); } - $clean[$key] = DynamicContentRules::cleanSetting($key, $value, $fields[$key], $sanitizer); + $clean[$key] = DynamicContentRules::cleanSetting($key, $value, $fields[$key], $sanitizer, $name); } return $clean; diff --git a/src/Integrations/Divi/Pages/DiviDynamicSourcesAbility.php b/src/Integrations/Divi/Pages/DiviDynamicSourcesAbility.php index 9863548..29700f9 100644 --- a/src/Integrations/Divi/Pages/DiviDynamicSourcesAbility.php +++ b/src/Integrations/Divi/Pages/DiviDynamicSourcesAbility.php @@ -205,12 +205,12 @@ final class DiviDynamicSourcesAbility implements Ability return '{{post:' . $post . '}}'; } - if (! str_starts_with($name, 'post_link_url_')) { + if (! str_starts_with($name, DynamicContentRules::LINK_PREFIX)) { return null; } - // Review M-9: {{link:}} refuses attachments (status inherit); internal types ("_…") are no link targets. - $type = substr($name, strlen('post_link_url_')); + // Review M-9/P10: {{link:}} and the full form refuse attachments; internal types ("_…") are no link targets. + $type = substr($name, strlen(DynamicContentRules::LINK_PREFIX)); return $type === 'attachment' || str_starts_with($type, '_') ? null : '{{link:}}'; } diff --git a/src/Integrations/Divi/Pages/WpLinkTargets.php b/src/Integrations/Divi/Pages/WpLinkTargets.php index 3db9db8..fac4c6a 100644 --- a/src/Integrations/Divi/Pages/WpLinkTargets.php +++ b/src/Integrations/Divi/Pages/WpLinkTargets.php @@ -10,9 +10,10 @@ use WP_Post; defined('ABSPATH') || exit; /** - * {{link:}} targets from WordPress: the entry exists, lives (no trash, auto-draft or - * revision) and the current user may read it (read_post, like select_post settings, S37). - * Missing and unreadable entries give the same null (no existence oracle). + * {{link:}} and post_link_url_* targets from WordPress (ruling P10): the entry exists, lives + * (no trash, auto-draft or revision), is no attachment (whatever its status) and the current user + * may read it (read_post, like select_post settings, S37). Missing and unreadable entries give the + * same null (no existence oracle). */ final class WpLinkTargets implements LinkTargets { @@ -21,7 +22,11 @@ final class WpLinkTargets implements LinkTargets public function postTypeOf(int $id): ?string { $post = get_post($id); - if (! $post instanceof WP_Post || in_array((string) $post->post_status, self::DEAD, true)) { + if ( + ! $post instanceof WP_Post + || in_array((string) $post->post_status, self::DEAD, true) + || $post->post_type === 'attachment' + ) { return null; } diff --git a/src/Integrations/Divi/Pages/WpValueSanitizer.php b/src/Integrations/Divi/Pages/WpValueSanitizer.php index 9e6cb0a..0b372b2 100644 --- a/src/Integrations/Divi/Pages/WpValueSanitizer.php +++ b/src/Integrations/Divi/Pages/WpValueSanitizer.php @@ -48,4 +48,9 @@ final class WpValueSanitizer implements ValueSanitizer { return current_user_can('read_post', $id); } + + public function linkTargetType(int $id): ?string + { + return (new WpLinkTargets())->postTypeOf($id); + } } diff --git a/tests/Support/ComposeFixture.php b/tests/Support/ComposeFixture.php index a8abb15..d14a261 100644 --- a/tests/Support/ComposeFixture.php +++ b/tests/Support/ComposeFixture.php @@ -105,6 +105,16 @@ final class ComposeFixture { return $id !== 666; } + + /** Ruling P10: 840 is a post, 666 no link target, 667 a readable attachment (none), every other ID a page. */ + public function linkTargetType(int $id): ?string + { + return match ($id) { + 666, 667 => null, + 840 => 'post', + default => 'page', + }; + } }; } diff --git a/tests/Unit/Integrations/Divi/Compose/PostIdParserTest.php b/tests/Unit/Integrations/Divi/Compose/PostIdParserTest.php new file mode 100644 index 0000000..5ddc76f --- /dev/null +++ b/tests/Unit/Integrations/Divi/Compose/PostIdParserTest.php @@ -0,0 +1,30 @@ +inner->canReadPost($id); } + + public function linkTargetType(int $id): ?string + { + return $this->inner->linkTargetType($id); + } }; $references = new VariableRef( DiviFixtures::designSystem(), diff --git a/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php b/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php index c2d86de..e1058f2 100644 --- a/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php +++ b/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php @@ -449,6 +449,64 @@ final class VariableRefTest extends TestCase } } + /** Ruling P10: a full-form internal link follows the {{link:}} target rules, its type included. */ + public function testFullFormInternalLinksFollowTheLinkTargetRules(): void + { + $ref = self::ref([ + 'post_link_url_page' => ['post_id' => 'select_post'], + 'post_link_url_post' => ['post_id' => 'select_post'], + ]); + $sanitizer = ComposeFixture::sanitizer(); + + self::assertSame( + self::content('post_link_url_post', '{"post_id":"840"}'), + $ref->canonical(self::content('post_link_url_post', '{"post_id":"840"}'), $sanitizer, true) + ); + self::assertSame( + self::content('post_link_url_post', '{"post_id":840}'), + $ref->canonical(self::content('post_link_url_post', '{"post_id":840}'), $sanitizer, true) + ); + $refused = [ + 'a post is no page' => self::content('post_link_url_page', '{"post_id":"840"}'), + 'a post is no page (int)' => self::content('post_link_url_page', '{"post_id":840}'), + 'a page is no post' => self::content('post_link_url_post', '{"post_id":"12"}'), + 'no link target' => self::content('post_link_url_page', '{"post_id":"666"}'), + 'attachment' => self::content('post_link_url_page', '{"post_id":"667"}'), + 'overflowing id' => self::content('post_link_url_page', '{"post_id":"99999999999999999999"}'), + 'padded id' => self::content('post_link_url_page', '{"post_id":"0840"}'), + 'float id' => self::content('post_link_url_page', '{"post_id":12.0}'), + 'bool id' => self::content('post_link_url_page', '{"post_id":true}'), + ]; + foreach ($refused as $case => $value) { + try { + $ref->canonical($value, $sanitizer, true); + self::fail($case . ' was accepted'); + } catch (InvalidArgumentException $e) { + self::assertStringContainsString('Beitrags-ID', $e->getMessage(), $case); + } + } + } + + /** Ruling S37/P10: select_post outside an internal link stays "a post the user may read", of any type. */ + public function testOtherPostSettingsOnlyNeedAReadablePost(): void + { + $ref = self::ref(['evil' => ['p' => 'select_post']]); + $sanitizer = ComposeFixture::sanitizer(); + + self::assertSame( + self::content('evil', '{"p":"840"}'), + $ref->canonical(self::content('evil', '{"p":"840"}'), $sanitizer) + ); + foreach (['"666"', '"0840"', '"99999999999999999999"'] as $id) { + try { + $ref->canonical(self::content('evil', '{"p":' . $id . '}'), $sanitizer); + self::fail($id . ' was accepted'); + } catch (InvalidArgumentException $e) { + self::assertStringContainsString('Beitrags-ID', $e->getMessage(), $id); + } + } + } + /** E13a: an internal link (post_link_url_) is a URL – refused in every other field. */ public function testInternalLinksOnlyInUrlFields(): void { diff --git a/tests/Unit/Integrations/Divi/Pages/WpLinkTargetsTest.php b/tests/Unit/Integrations/Divi/Pages/WpLinkTargetsTest.php index d62ef23..bade31c 100644 --- a/tests/Unit/Integrations/Divi/Pages/WpLinkTargetsTest.php +++ b/tests/Unit/Integrations/Divi/Pages/WpLinkTargetsTest.php @@ -19,6 +19,9 @@ final class WpLinkTargetsTest extends TestCase 842 => new WP_Post(['ID' => 842, 'post_type' => 'page', 'post_status' => 'private']), 843 => new WP_Post(['ID' => 843, 'post_type' => 'revision', 'post_status' => 'inherit']), 844 => new WP_Post(['ID' => 844, 'post_type' => 'page', 'post_status' => 'auto-draft']), + 845 => new WP_Post(['ID' => 845, 'post_type' => 'attachment', 'post_status' => 'inherit', 'post_parent' => 0]), + 846 => new WP_Post(['ID' => 846, 'post_type' => 'attachment', 'post_status' => 'trash']), + 847 => new WP_Post(['ID' => 847, 'post_type' => 'attachment', 'post_status' => 'private']), ]; Functions\when('get_post')->alias(static fn (int $id): ?WP_Post => $posts[$id] ?? null); Functions\when('current_user_can')->alias( @@ -33,5 +36,8 @@ final class WpLinkTargetsTest extends TestCase self::assertNull($links->postTypeOf(843), 'revision'); self::assertNull($links->postTypeOf(844), 'auto-draft'); self::assertNull($links->postTypeOf(900), 'missing'); + self::assertNull($links->postTypeOf(845), 'unattached attachment'); + self::assertNull($links->postTypeOf(846), 'trashed attachment'); + self::assertNull($links->postTypeOf(847), 'attachment with an own status (ruling P10: no attachment)'); } } diff --git a/tests/Unit/Integrations/Divi/Pages/WpValueSanitizerTest.php b/tests/Unit/Integrations/Divi/Pages/WpValueSanitizerTest.php index 4e5ac15..d1013bb 100644 --- a/tests/Unit/Integrations/Divi/Pages/WpValueSanitizerTest.php +++ b/tests/Unit/Integrations/Divi/Pages/WpValueSanitizerTest.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Tests\Unit\Integrations\Divi\Pages; use Brain\Monkey\Functions; use LIT\MCP\Integrations\Divi\Pages\WpValueSanitizer; use LIT\MCP\Tests\Unit\TestCase; +use WP_Post; final class WpValueSanitizerTest extends TestCase { @@ -55,5 +56,32 @@ final class WpValueSanitizerTest extends TestCase self::assertTrue($sanitizer->canReadPost(12)); self::assertFalse($sanitizer->canReadPost(13)); } -} + /** Ruling P10: the full form uses the {{link:}} targets – attachments, trash and unreadable entries are none. */ + public function testLinkTargetTypeFollowsTheLinkTargets(): void + { + Functions\when('get_post')->alias(static fn (int $id): ?WP_Post => match ($id) { + 12 => new WP_Post(['ID' => 12, 'post_type' => 'page', 'post_status' => 'draft']), + 13 => new WP_Post(['ID' => 13, 'post_type' => 'attachment', 'post_status' => 'inherit', 'post_parent' => 12]), + 14 => new WP_Post(['ID' => 14, 'post_type' => 'page', 'post_status' => 'trash']), + 16 => new WP_Post(['ID' => 16, 'post_type' => 'page', 'post_status' => 'private']), + 17 => new WP_Post(['ID' => 17, 'post_type' => 'attachment', 'post_status' => 'inherit', 'post_parent' => 0]), + 18 => new WP_Post(['ID' => 18, 'post_type' => 'attachment', 'post_status' => 'trash', 'post_parent' => 0]), + 19 => new WP_Post(['ID' => 19, 'post_type' => 'attachment', 'post_status' => 'private', 'post_parent' => 0]), + 20 => new WP_Post(['ID' => 20, 'post_type' => 'post', 'post_status' => 'publish']), + default => null, + }); + Functions\when('current_user_can')->alias(static fn (string $cap, int $id): bool => $id !== 16); + $sanitizer = new WpValueSanitizer(); + + self::assertSame('page', $sanitizer->linkTargetType(12)); + self::assertSame('post', $sanitizer->linkTargetType(20)); + self::assertNull($sanitizer->linkTargetType(13), 'attached attachment'); + self::assertNull($sanitizer->linkTargetType(14), 'trashed'); + self::assertNull($sanitizer->linkTargetType(15), 'missing'); + self::assertNull($sanitizer->linkTargetType(16), 'unreadable'); + self::assertNull($sanitizer->linkTargetType(17), 'unattached attachment'); + self::assertNull($sanitizer->linkTargetType(18), 'trashed attachment'); + self::assertNull($sanitizer->linkTargetType(19), 'attachment with an own status'); + } +}