[BUGFIX] Prevent endless loop when provider resolving fails

astehlik · Jun 21, 2019 · c921a3b · c921a3b
1 parent dc33b9b
commit c921a3b
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 21 deletions.
diff --git a/Classes/Controller/OembedController.php b/Classes/Controller/OembedController.php
@@ -124,8 +124,10 @@ protected function startRequestLoop($content)
         $response = null;
         $request = null;
 
+        $this->providerResolver->initialize($content);
+
         do {
-            $provider = $this->providerResolver->getNextMatchingProvider($content);
+            $provider = $this->providerResolver->getNextMatchingProvider();
 
             if ($provider === false) {
                 break;
@@ -147,13 +149,16 @@ protected function startRequestLoop($content)
                 }
 
                 $request = $this->requestBuilder->buildNextRequest($provider);
-            } while ($response === null);
+            } while ($response === null && $request);
         } while ($response === null);
 
         if ($response === null) {
             throw new \Sto\Mediaoembed\Exception\RequestException(
-                'No provider returned a valid result. Giving up.'
-                . ' Please make sure the URL is valid and you have configured a provider that can handle it.'
+                'No provider returned a valid result. Giving up.' .
+                sprintf(
+                    ' Please make sure the URL %s is valid and you have configured a provider that can handle it.',
+                    $content->getUrl()
+                )
             );
         }
 

diff --git a/Classes/Exception/HttpUnauthorizedException.php b/Classes/Exception/HttpUnauthorizedException.php
@@ -20,7 +20,7 @@
  * of any embedding any extra information, and rely on the provider
  * to provide access control.
  */
-class UnauthorizedException extends RequestException
+class HttpUnauthorizedException extends RequestException
 {
     /**
      * Initializes the Exception with a default message and a default code (1303402203).

diff --git a/Classes/Request/HttpRequest.php b/Classes/Request/HttpRequest.php
@@ -204,7 +204,7 @@ protected function getErrorCode($report)
      * @return string response data
      * @throws \Sto\Mediaoembed\Exception\HttpNotFoundException
      * @throws \Sto\Mediaoembed\Exception\HttpNotImplementedException
-     * @throws \Sto\Mediaoembed\Exception\UnauthorizedException
+     * @throws \Sto\Mediaoembed\Exception\HttpUnauthorizedException
      */
     protected function sendRequest($requestUrl)
     {
@@ -224,7 +224,7 @@ protected function sendRequest($requestUrl)
                     );
                     break;
                 case 401:
-                    throw new \Sto\Mediaoembed\Exception\UnauthorizedException($this->url, $requestUrl);
+                    throw new \Sto\Mediaoembed\Exception\HttpUnauthorizedException($this->url, $requestUrl);
                     break;
                 default:
                     throw new \RuntimeException(

diff --git a/Classes/Request/ProviderResolver.php b/Classes/Request/ProviderResolver.php
@@ -47,23 +47,27 @@ public function injectProviderRepository(\Sto\Mediaoembed\Domain\Repository\Prov
     }
 
     /**
-     * Returns the next active provider whos url scheme matches the URL in
-     * the current configuration
+     * Initializes the resolver for the given content.
      *
      * @param \Sto\Mediaoembed\Domain\Model\Content $content
-     * @return Provider The next matching provider
      */
-    public function getNextMatchingProvider($content)
-    {
+    public function initialize($content) {
         $this->url = $content->getUrl();
         $this->checkIfUrlIsValid();
 
         $this->providerResult = $this->providerRepository->findByIsGeneric(false);
         $this->providerResult->rewind();
+    }
 
-        $provider = $this->detectNextMatchingProvider();
-
-        return $provider;
+    /**
+     * Returns the next active provider whos url scheme matches the URL in
+     * the current configuration
+     *
+     * @return Provider|false The next matching provider
+     */
+    public function getNextMatchingProvider()
+    {
+        return $this->detectNextMatchingProvider();
     }
 
     /**
@@ -122,17 +126,13 @@ protected function detectNextMatchingProvider()
                 $urlScheme = str_replace('\*', '.*', $urlScheme);
                 if (preg_match('/' . $urlScheme . '/', $this->url)) {
                     $matchingProvider = $currentProvider;
-                    break 2;
+                    break;
                 }
             }
 
             $this->providerResult->next();
         } while ($matchingProvider === false);
 
-        if ($matchingProvider === false) {
-            throw new \Sto\Mediaoembed\Exception\NoMatchingProviderException($this->url);
-        }
-
         return $matchingProvider;
     }
 }
diff --git a/Classes/Request/RequestBuilder.php b/Classes/Request/RequestBuilder.php
@@ -54,7 +54,7 @@ class RequestBuilder
      * given provider data.
      *
      * @param Provider $provider
-     * @return HttpRequest or FALSE if no further requests are available
+     * @return HttpRequest|bool or FALSE if no further requests are available
      */
     public function buildNextRequest($provider)
     {