Skip to content
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

if globalCookiesDomain is empty? #3309

Open
ahrasis opened this issue Apr 23, 2019 · 3 comments
Open

if globalCookiesDomain is empty? #3309

ahrasis opened this issue Apr 23, 2019 · 3 comments

Comments

@ahrasis
Copy link
Contributor

ahrasis commented Apr 23, 2019

I was reading and testing the Session.php again after the PR is merged. I noticed that it is possible for globalCookiesDomain to be emptied (filled but later deleted) or remain empty (not filled at all) while subdomain cookies is selected, but the session.cookie_domain is already changed to top level domain.

I think it should not be changed unless the globalCookiesDomain field is not empty. The suggested change is as follows:

The original:

	if (!empty($modSettings['globalCookies']))
	{
		if (preg_match('~^\d{1,3}(\.\d{1,3}){3}$~', $parsed_url['host']) == 0 && preg_match('~(?:[^\.]+\.)?([^\.]{2,}\..+)\z~i', $parsed_url['host'], $parts) == 1) {
			@ini_set('session.cookie_domain', '.' . $parts[1]);

The change:

	if (!empty($modSettings['globalCookies']))
	{
		if (preg_match('~^\d{1,3}(\.\d{1,3}){3}$~', $parsed_url['host']) == 0 && preg_match('~(?:[^\.]+\.)?([^\.]{2,}\..+)\z~i', $parsed_url['host'], $parts) == 1) {
			if (!empty($modSettings['globalCookiesDomain']))
				@ini_set('session.cookie_domain', '.' . $parts[1]);
			else
				@ini_set('session.cookie_domain', '.' . $parts[0]);
		}

Or:

	if (!empty($modSettings['globalCookies']) && !empty($modSettings['globalCookiesDomain']))
	{
		if (preg_match('~^\d{1,3}(\.\d{1,3}){3}$~', $parsed_url['host']) == 0 && preg_match('~(?:[^\.]+\.)?([^\.]{2,}\..+)\z~i', $parsed_url['host'], $parts) == 1)
			@ini_set('session.cookie_domain', '.' . $parts[1]);
@Spuds Spuds added this to the 1.1.6 milestone Apr 23, 2019
@emanuele45 emanuele45 modified the milestones: 1.1.6, 1.1.7 Jul 21, 2019
@Spuds
Copy link
Contributor

Spuds commented Apr 21, 2021

Im not very versed in how Local/Global/subdomain cookies are implemented. @emanuele45 any ideas on this one?

@ahrasis
Copy link
Contributor Author

ahrasis commented Apr 21, 2021

I thought my suggestion was clear enough, however, I already left that to your good hands.

@Spuds
Copy link
Contributor

Spuds commented Apr 21, 2021

It was very clear :D what is not clear is my understanding of cookies!

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

No branches or pull requests

3 participants