Skip to content

Commit

Permalink
MAGETWO-64885: Wrong cookies set for store views with multidomain
Browse files Browse the repository at this point in the history
  • Loading branch information
@nikshostko
nikshostko committed May 18, 2017
1 parent 2498417 commit 508f1ef
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
2 changes: 1 addition & 1 deletion app/code/Magento/Store/Block/Switcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public function getTargetStorePostData(\Magento\Store\Model\Store $store, $data
{
$data[\Magento\Store\Api\StoreResolverInterface::PARAM_NAME] = $store->getCode();
return $this->_postDataHelper->getPostData(
$this->getUrl('stores/store/switch'),
$store->getCurrentUrl(false),

This comment has been minimized.

Sign in to view

Copy link
@hostep

hostep Aug 4, 2017

Contributor

@nikshostko, @balex13: I believe this change is incorrect.

We backported MAGETWO-64885 on top of Magento 2.1.7, because we were getting endless redirect loops when using multiple domains, which this commit seems to fix.

But when you now switch using the language switcher, you go to a url with '?___store=storeLanguage1', and then when you switch back again, you end up with the '?___store=storeLanguage1?___store=storeLanguage2' query in your url and this crashes then because Magento can't find a storecode 'storeLanguage1?___store=storeLanguage2'

I can get rid of the problem by using:

preg_replace('/([\?&]___store=[^\?]+)(\?___store=[^&]+)/', '$2', $store->getCurrentUrl(false)),

But this doesn't feel like the correct solution.

I believe the issue may be deeper, probably in: Magento\Store\Model\Store::getCurrentUrl
I think the assignment of the $requestString variable is a mistake, because the query parameters are included in that variable, and then later at the bottom, some extra query parameters are added, so it is possible to end up with two '?___store=xxxx' params in the url.
I think $this->_request->getRequestString() should be replaced with $this->_request->getOriginalPathInfo(), but I'm not sure if this will break other functionalities in Magento...

This is just what I quickly discovered, I'll try to verify this on a clean installation from the develop branch later on and create a proper issue, but I'm in a bit of a hury now, so I decided to quickly write down what I discovered for now.

This comment has been minimized.

Sign in to view

Copy link
@hostep

hostep Aug 5, 2017

Contributor

Just found out that in Magento 2.1.7, the Magento\Framework\App\Request\Http::setPathInfo method has changed drastically, which causes the above issue. See: a2f2893#commitcomment-23494452

This comment has been minimized.

Sign in to view

Copy link
@PieterCappelle

PieterCappelle Sep 8, 2018

Contributor

Hi @hostep, I'm having the same problem in 2.1.14. Is this still an issue?

This comment has been minimized.

Sign in to view

Copy link
@hostep

hostep Sep 8, 2018

Contributor

Hi!

No idea, the storeswitcher code is being changed in almost every single release as far as I can see. In one release it works (mostly), then in the next it's completely broken, and so on and so forth, it's a bit annoying and I've completely given up on attempting to keep make it work.

For example, in 2.2.5 it works, but not on product and category pages if the product or category url_key is different in between different storeviews. But if they are the same, it works again.

No idea about the state of 2.1.14.

Good luck with figuring this out! :)
$data
);
}
Expand Down
20 changes: 19 additions & 1 deletion app/code/Magento/Store/Model/Plugin/StoreCookie.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
use Magento\Store\Model\StoreIsInactiveException;
use Magento\Framework\Exception\NoSuchEntityException;
use \InvalidArgumentException;
use Magento\Store\Api\StoreResolverInterface;
use Magento\Framework\App\ObjectManager;

/**
* Class StoreCookie
Expand All @@ -33,19 +35,27 @@ class StoreCookie
*/
protected $storeRepository;

/**
* @var StoreResolverInterface
*/
private $storeResolver;

/**
* @param StoreManagerInterface $storeManager
* @param StoreCookieManagerInterface $storeCookieManager
* @param StoreRepositoryInterface $storeRepository
* @param StoreResolverInterface $storeResolver
*/
public function __construct(
StoreManagerInterface $storeManager,
StoreCookieManagerInterface $storeCookieManager,
StoreRepositoryInterface $storeRepository
StoreRepositoryInterface $storeRepository,
StoreResolverInterface $storeResolver = null
) {
$this->storeManager = $storeManager;
$this->storeCookieManager = $storeCookieManager;
$this->storeRepository = $storeRepository;
$this->storeResolver = $storeResolver ?: ObjectManager::getInstance()->get(StoreResolverInterface::class);
}

/**
Expand All @@ -72,5 +82,13 @@ public function beforeDispatch(
$this->storeCookieManager->deleteStoreCookie($this->storeManager->getDefaultStoreView());
}
}
if (
$this->storeCookieManager->getStoreCodeFromCookie() === null
|| $request->getParam(StoreResolverInterface::PARAM_NAME) !== null
) {
$storeId = $this->storeResolver->getCurrentStoreId();
$store = $this->storeRepository->getActiveStoreById($storeId);
$this->storeCookieManager->setStoreCookie($store);
}
}
}

0 comments on commit 508f1ef

Please sign in to comment.