@MichaelRoosz opened this Pull Request on June 18th 2020 Contributor

Since this commit has been merged, setting the cookie path is broken.
https://github.com/matomo-org/matomo/pull/15185/commits/3b69290a957e3ff68176481b8b0e63dd704e5d76
https://github.com/matomo-org/matomo/pull/15185

Cookie path in the Set-Cookie header must not be escaped, or the browser will fall back to the current URL path.

For example:
If $Path === '/' and the cookie is set from /js/tracker.php, the browser will save the cookie path as "/js" and not "/".

Reading the docu here https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie, I think we should not escape the path at all.
Same applies for the domain, but since the dot is not escaped by rawurlencode, I think it does not hurt

@sgiehl commented on June 18th 2020 Member

Might be something we maybe should add to 3.x-dev. Seems to be the same issue we had with session cookies. See https://github.com/matomo-org/matomo/pull/15602

@tsteur commented on June 20th 2020 Member

Haven't looked... can we always trust the $path?

@sgiehl commented on June 22nd 2020 Member

@tsteur In most cases it should use config values for cookie_path / login_cookie_path or doesn't have a value at all.
But we can't encode the path that way. We had the same problem in the linked PR. The browser simply discards the path if it's encoded...

@tsteur commented on June 22nd 2020 Member

👍 sweet. Was just meaning in case there is user input somewhere we'd need to do maybe some validation or so. That's all.

@diosmosis commented on June 23rd 2020 Member

👍 to merge into 3.x-dev

This Pull Request was closed on June 24th 2020
Powered by GitHub Issue Mirror