New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[yii2] Headers are cached when flushInterval is low. #4587

Closed
SamMousa opened this Issue Oct 24, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@SamMousa
Collaborator

SamMousa commented Oct 24, 2017

This is a niche issue; I'll update it with more information and a proposed solution in the coming days.
Also I'll clean it up soon ^^
@samdark please have a look as well and see if understand what I mean.

What are you trying to achieve?

Functional test, specifically sendPost and amBearerAuthenticated.

What do you get instead?

The header is not interpreted by Yii.

The problem is caused by the following chain of events:

  1. Yii2 module _before loads the application.
  2. Yii2 Application always loads the request component in its bootstrap:
protected function bootstrap()
    {
        $request = $this->getRequest();
        Yii::setAlias('@webroot', dirname($request->getScriptFile()));
        Yii::setAlias('@web', $request->getBaseUrl());
        parent::bootstrap();
    }
  1. The Yii2 yii\log\Target tries to get the user IP.
public function getMessagePrefix($message)
    {
        if ($this->prefix !== null) {
            return call_user_func($this->prefix, $message);
        }

        if (Yii::$app === null) {
            return '';
        }

        $request = Yii::$app->getRequest();
        $ip = $request instanceof Request ? $request->getUserIP() : '-';
  1. Yii2 only recently uses headers to get the remote users' IP.
public function getUserIP()
    {
        foreach ($this->ipHeaders as $ipHeader) {
            if ($this->headers->has($ipHeader)) {
                return trim(explode(',', $this->headers->get($ipHeader))[0]);
            }
        }

        return $this->getRemoteIP();
    }
  1. This causes the header collection to be loaded before the $_SERVER variable gets initialized for the request.

Known causes

  • Log flushing
  • Using yii\helpers\Url::to (sidenote passing an array to sendPOST doesn't work but works for _request; itwould still create a bad cache for the Request component).

Proposed solution with changes to Yii2 and Codeception:

The Request component could be improved to allow for clearing the header cache; or a configuration option could be added to disable it.
The Codeception code should then clear the header cache via the configuration option or method.

Proposed solution with changes to Codeception (Yii2 only):

Alternatively we could forcefully go into the object and set it's private property to null, we should do this everytime after we change $_SERVER:

        // Clear the headers cache.
        if (Yii::$app->has('request', true)) {
            $property = (new \ReflectionClass(Request::class))->getProperty('_headers');
            $property->setAccessible(true);
            $property->setValue(\Yii::$app->request, null);
        }

Conclusion

The first approach makes it testable in Yii2 code and is future proof. The second approach could theoretically break at any moment where the internal structure of yii\web\Request changes.

@samdark samdark added the Yii label Oct 24, 2017

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Oct 24, 2017

Collaborator

Understood more or less. No immediate thoughts on fixing it though.

Collaborator

samdark commented Oct 24, 2017

Understood more or less. No immediate thoughts on fixing it though.

@SamMousa

This comment has been minimized.

Show comment
Hide comment
@SamMousa

SamMousa Oct 24, 2017

Collaborator

Adding this after reinitializing $_SERVER in the Yii2 connector seems to work; it is a bit ugly though:

// Clear the headers cache.
        if (Yii::$app->has('request', true)) {
            $property = (new \ReflectionClass(Request::class))->getProperty('_headers');
            $property->setAccessible(true);
            $property->setValue(\Yii::$app->request, null);
        }
Collaborator

SamMousa commented Oct 24, 2017

Adding this after reinitializing $_SERVER in the Yii2 connector seems to work; it is a bit ugly though:

// Clear the headers cache.
        if (Yii::$app->has('request', true)) {
            $property = (new \ReflectionClass(Request::class))->getProperty('_headers');
            $property->setAccessible(true);
            $property->setValue(\Yii::$app->request, null);
        }
@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Oct 25, 2017

Collaborator

I guess same should be done to _cookies, right?

Collaborator

samdark commented Oct 25, 2017

I guess same should be done to _cookies, right?

@SamMousa

This comment has been minimized.

Show comment
Hide comment
@SamMousa

SamMousa Nov 13, 2017

Collaborator

Probably.

Collaborator

SamMousa commented Nov 13, 2017

Probably.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Nov 13, 2017

Collaborator

A pull request?

Collaborator

samdark commented Nov 13, 2017

A pull request?

@SamMousa

This comment has been minimized.

Show comment
Hide comment
@SamMousa

SamMousa Nov 23, 2017

Collaborator

Which approach?

I found some other things that initiate the Request component before $_SERVER is initialized, I work around them with:

\Yii::$app->request->ipHeaders = [];
\Yii::$app->request->secureProtocolHeaders = [];

The problem here is that calling $I->amLoggedIn() will try to log the remote users' IP address.

Collaborator

SamMousa commented Nov 23, 2017

Which approach?

I found some other things that initiate the Request component before $_SERVER is initialized, I work around them with:

\Yii::$app->request->ipHeaders = [];
\Yii::$app->request->secureProtocolHeaders = [];

The problem here is that calling $I->amLoggedIn() will try to log the remote users' IP address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment