Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate shortcode controllername service configurations (Case 173693) #40

Merged
merged 14 commits into from
Oct 30, 2024
Merged
15 changes: 8 additions & 7 deletions src/Handler/EmbeddedShortcodeHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,18 @@ class EmbeddedShortcodeHandler
/** @var RequestStack */
private $requestStack;

/**
* @param string $controllerName
* @param string $renderer
*/
public function __construct(
FragmentHandler $fragmentHandler,
$controllerName,
$renderer,
string $controllerName,
string $renderer,
RequestStack $requestStack,
?LoggerInterface $logger = null
LoggerInterface $logger = null
) {
$callableFragments = explode('::', $controllerName);
Copy link
Member Author

@FabianSchmick FabianSchmick Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if I should also check for single : if I remember correctly, this was valid some time ago, too?

In the docs https://www.php.net/manual/en/language.oop5.paamayim-nekudotayim.php there is no : alternate

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK not in PHP, you might be thinking about Symfonys homebrew "bundle notation", which got deprecated in 4.1.

More importantly, I think we should allow invokable controllers as well. I'll add some changes shortly.

if (!is_array($callableFragments) || !isset($callableFragments[1]) || !method_exists($callableFragments[0], $callableFragments[1])) {
throw new \InvalidArgumentException('The controller method: "'.$controllerName.'" does not exist.');
}

$this->fragmentHandler = $fragmentHandler;
$this->controllerName = $controllerName;
$this->renderer = $renderer;
Expand Down
44 changes: 41 additions & 3 deletions tests/Functional/EmbeddedShortcodeHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpKernel\Fragment\FragmentHandler;
use Webfactory\ShortcodeBundle\Controller\GuideController;
use Webfactory\ShortcodeBundle\Handler\EmbeddedShortcodeHandler;
use Webfactory\ShortcodeBundle\Test\EndToEndTestHelper;

/**
Expand Down Expand Up @@ -37,7 +40,7 @@ public function expand_shortcodes_registered_in_different_ways(string $shortcode
self::assertSame('test foo=bar', $this->processShortcodes("[$shortcodeName foo=bar]"));
}

public static function provideShortcodeNames(): Generator
public function provideShortcodeNames(): Generator
{
yield 'Inline shortcode defined in bundle config' => ['test-config-inline'];
yield 'ESI-based shortcode defined in bundle config' => ['test-config-esi'];
Expand All @@ -58,13 +61,48 @@ public function processing_with_esi_fragments(string $shortcodeName): void
self::assertStringContainsString('<esi:include ', $this->processShortcodes("[$shortcodeName foo=bar]", $request));
}

public static function provideEsiShortcodes(): Generator
public function provideEsiShortcodes(): Generator
{
yield 'ESI-based shortcode defined in bundle configuration' => ['test-config-esi'];
yield 'ESI-based shortcode defined in service configuration' => ['test-service-esi'];
}

private function processShortcodes(string $content, ?Request $request = null): string
/**
* @test
*/
public function validate_valid_controller_names(): void
{
$this->expectNotToPerformAssertions();

new EmbeddedShortcodeHandler(
$this->createMock(FragmentHandler::class),
GuideController::class.'::detailAction',
'inline',
$this->createMock(RequestStack::class)
);
}

/**
* @test
*
* @dataProvider provideControllerNames
*/
public function validate_invalid_controller_names(string $controllerName): void
{
$this->expectException(\InvalidArgumentException::class);

new EmbeddedShortcodeHandler($this->createMock(FragmentHandler::class), $controllerName, 'inline', $this->createMock(RequestStack::class));
}

public function provideControllerNames(): Generator
{
yield 'Empty string' => [''];
yield 'Not existing controller' => ['Foo/Bar/Not/Exist::method'];
yield 'Missing method name' => [GuideController::class];
yield 'Not existing method' => [GuideController::class.'_notExistingMethod'];
}

private function processShortcodes(string $content, Request $request = null): string
{
self::bootKernel();

Expand Down
Loading