Multi-language redirects #2435

parkersweb · 2019-04-08T21:37:39Z

We've got a live site running 1.3.1 which has the re-direct /about-us/company: /about-us. Our new site has an upgraded version of Grav (I've tried 1.5.10 and also 1.6-rc4).

On the old site that re-direct would match for all languages i.e. only the slug part is important - as per the docs (https://learn.getgrav.org/15/content/routing#site-level-routes-and-redirects). On the upgraded site however, whilst the root https://example.com/about-us/company redirects it no longer matches the language versions i.e. https://example.com/en-gb/about-us/company and https://example.com/en-us/about-us/company

If I modify the redirect to use/en-(gb|us)/about-us/company: /about-us it does correctly redirect.

Not sure if the logic has changed in this area - but any help you could give would be very much appreciated!

The text was updated successfully, but these errors were encountered:

nbusseneau · 2020-09-12T18:16:37Z

Bumping this up: I can confirm it is still happening in 1.6.27.

I have a multi-language website (en and fr). When setting up site.yaml like this, redirects for requests to /en/foo/2020 won't work:

redirects:
  /foo/(\d+): /bar/$1

But they do with the following:

redirects:
  (/.*)?/foo/(\d+): /bar/$2

I expected the first to work, as the documentation mentions:

All redirect rules apply on the slug-path beginning after the language part (if you use multi-language pages)

Investigation

I noticed there was a big refactor in Grav 1.5 around multi-language redirects, perhaps explaining why OP had it working in prior versions. I tried to skim through it, but couldn't make out how the changes would impact matching source URL with language prefixes.

Instead, I had a look at the dispatch() code responsible for handling redirects. Here is the code for regex redirects:

grav/system/src/Grav/Common/Page/Pages.php

Lines 537 to 556 in 75cb4e3

    
           /** @var Uri $uri */ 
        
           $uri = $this->grav['uri']; 
        
           /** @var \Grav\Framework\Uri\Uri $source_url */ 
        
           $source_url = $uri->uri(false); 
        
           // Try Regex style redirects 
        
           $site_redirects = $config->get('site.redirects'); 
        
           if (is_array($site_redirects)) { 
        
               foreach ((array)$site_redirects as $pattern => $replace) { 
        
                   $pattern = '#^' . str_replace('/', '\/', ltrim($pattern, '^')) . '#'; 
        
                   try { 
        
                       $found = preg_replace($pattern, $replace, $source_url); 
        
                       if ($found !== $source_url) { 
        
                           $this->grav->redirectLangSafe($found); 
        
                       } 
        
                   } catch (ErrorException $e) { 
        
                       $this->grav['log']->error('site.redirects: ' . $pattern . '-> ' . $e->getMessage()); 
        
                   } 
        
               } 
        
           }

As we can see, it tries to match redirect source $pattern (after sanitizing) against $source_url retrieved via $uri->uri(false). However, $source_url contains the language prefix, and thus cannot match $pattern as-is.

So I tried to use $uri->path() instead, as it does not contain the language prefix, and hooray! It does seem to work. Considering the $route parameter to the dispatch() function is actually already supposed to be $uri->path() (as per PagesServiceProvider), we can directly use $route for matching purposes instead of $source_url (which becomes an unused variable and can be cleaned up).

Thus, the question is: why are regex redirects and routes relying on $uri->uri(false) rather than the given dispatch() $route? Is there any specific reason? If not, then I am ready to submit a PR to fix this.

Multilang redirects and routes were not working due to language prefixes being included in URL when trying to match source URL and redirect/route source pattern. Using `$route` parameter fixes the issue as given route does not include language prefixes (coming from `$uri->path()` in `PagesServiceProvider`). Fixes getgrav#2435.

mahagr · 2020-09-14T07:21:25Z

I agree that based on the documentation, your proposal seems to be the better choice.

rhukster added the evaluating label Apr 9, 2019

mahagr self-assigned this Dec 13, 2019

nbusseneau linked a pull request Sep 12, 2020 that will close this issue

Fix multilang redirects and routes #3004

Open

mahagr mentioned this issue Mar 12, 2021

Method $pages->find() should never redirect #3266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi-language redirects #2435

Multi-language redirects #2435

parkersweb commented Apr 8, 2019

nbusseneau commented Sep 12, 2020

mahagr commented Sep 14, 2020

Multi-language redirects #2435

Multi-language redirects #2435

Comments

parkersweb commented Apr 8, 2019

nbusseneau commented Sep 12, 2020

Investigation

mahagr commented Sep 14, 2020