Hello Guys,

I am handling few scenarios in my project where I have bunch of internal path's (these can be redirects / aliases) for which I am using 'findMatchingRedirect' method of RedirectRepository Class, to get a redirect for given path.

I have a node (nid = 123) having alias : /alias-exists-for-node-123

I have chained redirects for this node (as below)

From                To                Status code           Original language

/this-is-test-redirect         /node/123        301      Not specified
/this-is-test-redirect-10      /this-is-test-redirect       301       Not specified
/this-is-test-redirect-11      /this-is-test-redirect-10       301      Not specified
/this-is-test-redirect-12      /this-is-test-redirect-11       301       Not specified

In current scenario, I have below URL's

/this-is-test-redirect
/this-is-test-redirect-12

I have used above urls in foreach loop and findMatchingRedirect method for getting a redirect for a url.

$redirects = null; $alias_list = [];
$current_lang = \Drupal::languageManager()->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
foreach ($urls as $path) { 
     try {
       $redirects = \Drupal::service('redirect.repository')->findMatchingRedirect($path, [], $current_lang);
     } catch (RedirectLoopException $e) {
       \Drupal::logger('redirect')->warning($e->getMessage());
     }
    
     if($redirects) {
          $internal_redirect_uri = $redirects->getRedirect()['uri'];
         //  $alias_list[] = 'get-alias-by-path'; // use method to get alias by path
     }
}

When I Pass below paths, it returns alias of node 123

/this-is-test-redirect-12
/this-is-test-redirect

But when I change the order of paths,
/this-is-test-redirect
/this-is-test-redirect-12

I get caught in to Redirect loop identified issue from findMatchingRedirect method.
Redirect loop identified at /this-is-test-redirect path for redirect 1223

I debug the findMatchingRedirect method, I observed that private array variable $foundRedirects does carry the old redirect-id even after execution completed for first path (i.e /this-is-test-redirect) and about to start for next path(/this-is-test-redirect-12).

Any help would be appreciated! Also correct me if I am doing something wrong.

Thanks !

Comments

nileema19 created an issue. See original summary.

nileema19’s picture

Issue summary: View changes
nileema19’s picture

Issue summary: View changes
nileema19’s picture

Issue summary: View changes
kyuubi’s picture

This seems to be related to #3061173: scope foundRedirects to the current request from the request stack. Would be awesome if someone could have a look at this as in the our case it breaks redirects in production (GraphQL subrequests)

nileema19’s picture

StatusFileSize
new392 bytes

I have tested some test-cases with attached patch, this has solved my issue.
Though I am still testing this patch. Please review.

Meanwhile I am looking for other solution.

berdir’s picture

Status: Active » Needs review
+++ b/src/RedirectRepository.php
@@ -98,8 +98,11 @@ class RedirectRepository {
-
-    return NULL;
+    else {
+        // Reset found redirects.
+        $this->foundRedirects = [];
+        return NULL;
+    }
   }
 

Adding the else seems unnecessary/unrelated and the patch doesn't follow coding standards (2 spaces). Leaving at needs work to have the tests run.

berdir’s picture

Status: Needs review » Needs work
nileema19’s picture

StatusFileSize
new490 bytes

@Berdir, thank you for the inputs!

I have attached the updated patch.

berdir’s picture

Status: Needs work » Needs review
mbovan’s picture

Title: Redirect loop occurs » Redirect loop can occur in sub-requests
Status: Needs review » Reviewed & tested by the community

We use a batch process to update aliases/redirects for multiple entities and we had the same problem as described in the issue summary.

I can confirm that patch from #9 fixes the issue.

kyuubi’s picture

#9 works like a charm for me.

berdir’s picture

Here's a test for this.

The last submitted patch, 13: resetFoundRedirect-3059894-13-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Berdir committed c8e8d6d on 8.x-1.x
    Issue #3059894 by nileema19, Berdir: Redirect loop can occur in sub-...
berdir’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

kyuubi’s picture

Awesome, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.