diff --git a/core/modules/media/src/OEmbed/Provider.php b/core/modules/media/src/OEmbed/Provider.php index 954dac1c9fd3748b9a3024e74c6126e4d357c02b..c3ea5a4855b4283e5164bb35f7152848c435915f 100644 --- a/core/modules/media/src/OEmbed/Provider.php +++ b/core/modules/media/src/OEmbed/Provider.php @@ -43,11 +43,11 @@ class Provider { * @throws \Drupal\media\OEmbed\ProviderException */ public function __construct($name, $url, array $endpoints) { + $this->name = $name; + if (!UrlHelper::isValid($url, TRUE) || !UrlHelper::isExternal($url)) { throw new ProviderException('Provider @name does not define a valid external URL.', $this); } - - $this->name = $name; $this->url = $url; try { diff --git a/core/modules/media/src/OEmbed/ProviderRepository.php b/core/modules/media/src/OEmbed/ProviderRepository.php index 159bafda55755ffb60bce8ac7c1eaefa93211723..5f190c0d9245aaa5a49eb6c7ed41dab559fdddb6 100644 --- a/core/modules/media/src/OEmbed/ProviderRepository.php +++ b/core/modules/media/src/OEmbed/ProviderRepository.php @@ -158,8 +158,9 @@ public function getAll() { $keyed_providers[$name] = new Provider($provider['provider_name'], $provider['provider_url'], $provider['endpoints']); } catch (ProviderException $e) { - // Just skip all the invalid providers. - // @todo Log the exception message to help with debugging. + // Skip invalid providers, but log the exception message to help with + // debugging. + $this->logger->warning($e->getMessage()); } } diff --git a/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php b/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php index 359c3cc87d60f04084e8e1b9a53fb0cd8a441b8b..7bd6cfbd8c204d66cfbf5f1b54bd3cc1bc20beef 100644 --- a/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php +++ b/core/modules/media/tests/src/Unit/ProviderRepositoryTest.php @@ -4,6 +4,7 @@ use Drupal\Core\KeyValueStore\KeyValueMemoryFactory; use Drupal\Core\Logger\LoggerChannelFactory; +use Drupal\Core\Logger\RfcLogLevel; use Drupal\media\OEmbed\ProviderException; use Drupal\media\OEmbed\ProviderRepository; use Drupal\Tests\UnitTestCase; @@ -11,6 +12,7 @@ use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Psr7\Response; +use Prophecy\Argument; /** * @coversDefaultClass \Drupal\media\OEmbed\ProviderRepository @@ -47,6 +49,13 @@ class ProviderRepositoryTest extends UnitTestCase { */ private $currentTime; + /** + * The mocked logger channel. + * + * @var \Psr\Log\LoggerInterface|\Prophecy\Prophecy\ObjectProphecy + */ + private $logger; + /** * {@inheritdoc} */ @@ -66,6 +75,10 @@ protected function setUp(): void { $time = $this->prophesize('\Drupal\Component\Datetime\TimeInterface'); $time->getCurrentTime()->willReturn($this->currentTime); + $this->logger = $this->prophesize('\Psr\Log\LoggerInterface'); + $logger_factory = new LoggerChannelFactory(); + $logger_factory->addLogger($this->logger->reveal()); + $this->responses = new MockHandler(); $client = new Client([ 'handler' => HandlerStack::create($this->responses), @@ -75,7 +88,7 @@ protected function setUp(): void { $config_factory, $time->reveal(), $key_value_factory, - new LoggerChannelFactory() + $logger_factory ); } @@ -217,6 +230,13 @@ public function testCorruptProviderIgnored(): void { $response = new Response(200, [], $body); $this->responses->append($response); + // The corrupt provider should cause a warning to be logged. + $this->logger->log( + RfcLogLevel::WARNING, + "Provider Uncle Rico's football videos does not define a valid external URL.", + Argument::type('array') + )->shouldBeCalled(); + $youtube = $this->repository->get('YouTube'); // The corrupt provider should not be stored. $stored_data = [