Skip to content

Commit

Permalink
fix encrpt
Browse files Browse the repository at this point in the history
  • Loading branch information
@taylorotwell
taylorotwell committed Jul 27, 2020
1 parent c9ce261 commit 5786aa4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/Illuminate/Cookie/Middleware/EncryptCookies.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ protected function encrypt(Response $response)
$response->headers->setCookie($this->duplicate(
$cookie,
$this->encrypter->encrypt(
sha1($cookie->getName().'v2').'|'.$cookie->getValue(),
hash_hmac('sha1', $cookie->getName().'v2', $this->encrypter->getKey()).'|'.$cookie->getValue(),
static::serialized($cookie->getName())
)
));
Expand Down

3 comments on commit 5786aa4

@denis-chmel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taylorotwell
Good evening bro,
Found this change as I've got a crash in our code after migrating to 6.x,
You've added the call to $this->encrypter->getKey() here,
where $this->encrypter contract is \Illuminate\Contracts\Encryption\Encrypter
while that contract has no public getKey() declared while I guess now it should be:
https://github.com/laravel/framework/blob/8.x/src/Illuminate/Contracts/Encryption/Encrypter.php

So whoever is customizing Encryptor - isn't aware that this method is now required in 6.x

        $this->app->extend('encrypter', static function (Encrypter $command) {
            return new CustomEncrypterDecorator($command);
        });

I'd make a PR if you agree, I just not sure which branch to, techincally it's in all 6.x, 7.x and 8.x, so requires 3 PRs?

Let me know, thanks.

@smknstd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denis-chmel Hello ! Did you have any answer or did you submit something ? Thanx in advance

@smknstd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#38772

Please sign in to comment.