Redirect 301 for unresolved url matching with suffix

[dynasource]

fixes #7670

[samdark]

It should work the other way around as well i.e. if suffix isn't defined than check if there's / in the path. If it's there, remove it and try again.

[dynasource]

there are 2 problems with this other way around:

1. The current UrlManager + UrlRules (2.0.6) accept infinit '/' suffixes in the url (try it yourself with a route like /index.php/test/route//////). This means that a suffix like '/' is always accepted in the URL, despite a suffix not being set (perhabs this is another issue for the tracker)

2. Taken this into account this means that, if you still want to check the other way around, you have to check on a '/' suffix for every request that is correctly parsed (because an given urls with '////' are always parsed correctly). This goes at cost of performance.

[samdark]

1. rtrim /.
2. URLs with //// parsed correctly? What do you mean? It gives 404 if suffix isn't /.

[dynasource]

Dived into it.
I started with a fresh new Advanced project with the following setting:

    'urlManager'=>[
        'class'=>'yii\web\UrlManager',
        'enablePrettyUrl' => true,            
    ]

Both following urls render correctly:

• http://localhost/dmn/yii-application/frontend/web/index.php/site/about
• http://localhost/dmn/yii-application/frontend/web/index.php/site/about/////

Seems like unwanted behavior.

[SilverFire]

Redundant spaces at the end of the line

[klimov-paul]

Solution does not seem right to me. There must be a simplier way to achieve the goal.

[dynasource]

with the current state of the UrlManager + UrlRules + Request, this is a simple way of achieving this goal.

When you desire a cleaner and even more simple way, than more refactoring and cleanups have to be done in the those 3 classes. I would say, this is something for later releases.

[SilverFire]

Nothing is so permanent as a temporary solution.

[dynasource]

nothing is so permanent as a perminant 301 redirect ;)

[samdark]

@klimov-paul do you have an idea for better approach to the problem?

[kathysledge]

Just to throw it out there, we have things setup like below. This way, the default is no trailing slash but it is optional.

<?php
namespace yii\helpers;

use Yii;
use yii\helpers\BaseUrl;

class Url extends BaseUrl {

    public static function to($url = '', $scheme = false)
    {
        if (is_array($url)) {

            if (!isset($url['tbs']) && $url[0] !== 'home/index') {

                $url['tbs'] = '';
            }

            return static::toRoute($url, $scheme);
        }

and at the end of urls.php:

$newUrls = [];

foreach ($urls as $url => $route) {

    if ($url !== '/') {

        $url .= '<tbs:\/?>';
    }

    $newUrls[$url] = $route;
}

return $newUrls;

[dynasource]

@df2, this patch is about automatic redirection

[klimov-paul]

@klimov-paul do you have an idea for better approach to the problem?

This is hard to tell for sure. But at first glance just analyzing of $_SERVER['REQUEST_URI'] and $_SERVER['QUERY_STRING'] should be enough to find out if suffix is present or not.

[samdark]

Correct but we need to get the suffix we're checking for from somewhere. It could be defined either in UrlManager or in UrlRule.

[klimov-paul]

We may advance UrlManaer::parseRequest() so it can return URL suffix as a third element of the result array.
This line:
https://github.com/yiisoft/yii2/blob/master/framework/web/UrlManager.php#L268

can be following:

return [(string) $route, [], $urlSuffix];

This will not cause a BC break.

[klimov-paul]

Inside Request::resolve(), we parse 3 elements instead of 2 and do something if $urlSuffix is present:

public function resolve()
    {
        $result = Yii::$app->getUrlManager()->parseRequest($this);
        if ($result !== false) {
            list ($route, $params, $urlSuffix) = $result;

            if (isset($urlSuffix)) {
                 // check and redirect if necessary
            }

            //...
    }

[dynasource]

well, it looks like your excluding the (rare) usecase of #9207 (comment)

Based on your example:

• you are matching by route & params first
• to get a 100% match (whether its with or without suffix)
• then you do decide to redirect

If you do this, than someone cannot have both Urls as UrlRules:

• http://exampe.com/contact/ -> pageX
• http://exampe.com/contact -> pageY

Because pageX is matched first, pageY is ignored

[klimov-paul]

it looks like your excluding the (rare) usecase

Why? Url suffix is used for URL rule parsing already. Does not it?
Parsing http://exampe.com/contact/ will return result:

[
    'site/contact',
    [],
    '/'
]

And everything will be fine.

Parsing http://exampe.com/contact will return

[
    'site/contact',
    [],
    ''
]

Also should be fine.

Then we compare returned 3rd parameter with the request URI, checking if it is present or not.

[dynasource]

the issue is about hard & softmatching and the correct order which means, first hard matching, then softmatching.

If you are doing a redirect, than it means, the URL is not right by hard matching, but with softmatching on a suffix, it should be executed.

If you are using the current implementation of UrlManager::parseRequest, you are looping on the UrlRules. You are returning [(string) $route, [], $urlSuffix] if a match of the UrlRule has been found.

With your example, this means a softmatching on suffix is done in an early stage in the UrlRule itself, otherwise it could never return the array with the correct suffix.

This also means that a hard match on a following UrlRule is ignored.

[klimov-paul]

If you are doing a redirect, than it means, the URL is not right by hard matching, but with softmatching on a suffix, it should be executed.

Are you sure about that?
UrlRule matchign includes the suffix:
https://github.com/yiisoft/yii2/blob/master/framework/web/UrlRule.php#L238

If suffix is missmatch the Rule will return false instead of array and thus will be skipped.

The goal is enforce some trainling suffix ("/") in case NO suffix is explicitly macthed by Rule and it is present in UrlManager. Thus is should be fine.

The only problem I can see if you want enforce some suffix like "/", but still keep a route without any suffix. For this use case some magic value should be reserved like false, indicating Rule should not have a suffix.

[klimov-paul]

I have no garantee my idea will work. But at least it worth trying.

[dynasource]

true. Your solution is cleaner & faster. If you get it more robust, fixing the problem you mentioned, I would prefer yours. It requires more refactoring though, which would make this PR bigger. I tried to keep my PR a small as possible.

[klimov-paul]

See #10745