Yii2: UrlRule::defaults not working with parameters not present in pattern

[yuniorsk]

What steps will reproduce the problem?

We have set this url rule

[
    'pattern' => 'search',
    'route' => 'search/index',
    'defaults' => ['page' => 1],
],

and hit this url address /search?page=2

What is the expected result?

Page parameter should have value 2

What do you get instead?

Page parameter has value 1

Additional info

| Q | A
| ---------------- | ---
| Yii version | 2.0.23
| PHP version | 7.3.13
| Operating system | Ubuntu

This won't happen with named parameters e.g. pattern is search/<page:\d+> and requested url is /search/2

[samdark]

Any idea about the fix?

[yuniorsk]

My first instinct was that this union should be other way around, but it looks it won't be that easy
https://github.com/yiisoft/yii2/blob/b1da57ddf979d2d3b01bd607707bd4e589913875/framework/web/Request.php#L279

I can look into it further and try to cover it with unit tests.

[djidji01]

My first instinct was that this union should be other way around, but it looks it won't be that easy

https://github.com/yiisoft/yii2/blob/b1da57ddf979d2d3b01bd607707bd4e589913875/framework/web/Request.php#L279

I can look into it further and try to cover it with unit tests.

What steps will reproduce the problem?

We have set this url rule

[
    'pattern' => 'search',
    'route' => 'search/index',
    'defaults' => ['page' => 1],
],

and hit this url address /search?page=2

What is the expected result?

Page parameter should have value 2

What do you get instead?

Page parameter has value 1

Additional info

Q A
Yii version 2.0.23
PHP version 7.3.13
Operating system Ubuntu

This won't happen with named parameters e.g. pattern is search/<page:\d+> and requested url is /search/2

I think this is the proper functionality. A default of nothing (not declared page in the pattern) should stay the only possibility!!!

Otherwise the default value should be unset from route params, if in $_GET at this level:

        foreach ($this->defaults as $name => $value) {
            if (!isset($matches[$name]) || $matches[$name] === '') {
                $matches[$name] = $value;
            }
        }
        $params = $this->defaults;

yii2/web/UrlRule.php
line 417

to:

       $params = $this->defaults;
        foreach ($this->defaults as $name => $value) {
            if (!isset($matches[$name]) || $matches[$name] === '') {
                $matches[$name] = $value;
            }
            if(isset($_GET['name'])){
                unset($params[$name]);
            }
        }

[yuniorsk]

Yeah, I somehow didn't realize that default parameters are bound to named params and are not intended to work with query params (maybe there should be some notice about it in docs). Nonetheless I think the default value should not overwrite query value and should be ignored in this case.

[djidji01]

maybe there should be some notice about it in docs

I think it is what documented here and here.

I think the default value should not overwrite query value and should be ignored in this case.

Sure. shouldn't it be featured and documented as it is for? I suggest bellow correction of code above my be you can try it with unit tests. Thanks

        $params = $this->defaults;
        foreach ($params as $name => $value) {
            if (!isset($matches[$name]) || $matches[$name] === '') {
                $matches[$name] = $value;
                $params[$name]  =$request->getQueryParam($name, $value);
            }
        }

[samdark]

Won't that change behavior significantly? That may break existing apps...