@niklas-deworetzki-thm #15742 is an Pull Request, so you wan't to close this?

@franz-wohlkoenig i guess it's because of this #15742 (comment)

Phil is not able to fix conflicts these days

@jeckodevelopment thanks, so all fine.

I looked into Joomla! 4.0, where this issue also exists. But after adding the two checks from this PR to /libraries/src/Session/Storage/JoomlaStorage.php the application still fails and I'm greeted with following error site:

The reason for this lies within /libraries/vendor/joomla/session/src/Storage/NativeStorage.php:

The method setName on line 403 checks for an active session and terminates

	public function setName(string $name)
	{
		if ($this->isActive())
		{
			throw new \LogicException('Cannot change the name of an active session');
		}

		session_name($name);

		return $this;
	}

So I have two questions now:

1. Should we add this behavior to the current version instead of ignoring the active session?
2. Should the current 'fixes' be applied to 4.0 if its intended to fail?

@wilsonge @mbabker perhaps you can help me, deciding how Joomla! 3.8 should behave, when a php session was started before Joomla! itself is starting?

With this fix Joomla! 3.8 ignores the fact, that it can't change the ini values
But Joomla! 4.0 displays the error message shown above.

I'm happy for 3.8 to ignore I think - especially as in 3.x we don't have good session management in the CLI scripts - and just to tighten the screw in 4.x

Okay, that's fine. So we just need some testers before we can finally merge this PR

So we just need some testers before we can finally merge this PR

@niklas-deworetzki-thm correct. Every Pull Request need 2 successfully Tests (not by PR-Dev.) before Maintainer decide if PR gets merged.

@niklas-deworetzki-thm I have tested this patch on Joomla 3.9.1 and it works as expected.

My interest in it is that it would resolve a longstanding problem (since J3.8) with CiviCRM's cron jobs failing because of Error: Failed to start application: Failed to start the session because headers have already been sent by "/var/www/html/libraries/joomla/session/handler/joomla.php" at line 48.. I used this in part to test the patch: works fine with it applied and broken without.

I am wondering about the other instances of headers_sent() in the Joomla code (or maybe what I'm about to say would be scope creep...). This commit is a good place to see the most relevant instances, i.e. where there is an ini_set() after if (!headers_sent()), and as is the case inJSessionHandlerNative as it currently stands (before this PR), these don't have && session_id() == '' so is there potential for the same problem? I don't know if there is a real problem though, because with this PR's patch applied I can't produce any problem with Joomla's session handler set to database or redis (I didn't test others).

@niklas-deworetzki-thm can you please comment @andrewpthompson above?

@andrewpthompson I don't know about any other instances of ini_set() in the Joomla code. But having an "unprotected" ini_set() doesn't necessarily cause problems or a crash, since it's a timing issue. As long as no data has been sent, there is nothing to fear from setting ini-variables.

Since those variables should only be changed during Joomlas startup procedure, it is relatively safe to assume no other errors will occur at runtime.

Thanks @niklas-deworetzki-thm. That makes sense. I hadn't seen any problem arise from other instances of ini_set() anyway. This patch worked well for me.

@andrewpthompson please mark your test as successfully > https://docs.joomla.org/Testing_Joomla!_patches#Recording_test_results

I have tested this item ✅ successfully on 1d52b99

First, I reproduced the problem using the CiviCRM extension. CiviCRM has had a longstanding problem (since J3.8) with its cron jobs failing to run. I used this command to test:

wget -O - -q -t 1 'https://localhost/testsite/administrator/components/com_civicrm/civicrm/bin/cron.php?name=civicrm_cron&pass=password&key=mysitekey'

Prior to applying the patch I received this error:

Error: Failed to start application: Failed to start the session because headers have already been sent by "/var/www/html/testsite/libraries/joomla/session/handler/joomla.php" at line 48.

After applying the patch, no error.

No adverse effects seen from the patch. I tested logging in and out of J Admin console and frontend, editing article.

I have tested this item ✅ successfully on 1d52b99

First, I reproduced the problem using the CiviCRM extension. CiviCRM has had a longstanding problem (since J3.8) with its cron jobs failing to run. I used this command to test:

wget -O - -q -t 1 'https://localhost/testsite/administrator/components/com_civicrm/civicrm/bin/cron.php?name=civicrm_cron&pass=password&key=mysitekey'

Prior to applying the patch I received this error:

Error: Failed to start application: Failed to start the session because headers have already been sent by "/var/www/html/membership/libraries/joomla/session/handler/joomla.php" at line 48.

After applying the patch, no error.

No adverse effects seen from the patch. I tested logging in and out of J Admin console and frontend, editing article.

@PhilETaylor can you please test as Opener of #15742?

@franz-wohlkoenig No. Sorry but I no longer contribute to the Joomla project as is not a safe place for contributors.

as is not a safe place for contributors.

didn't get what this mean.

@PhilETaylor got it and sorry to read. I try to make this better.

It can't be applied:

The file marked for modification does not exist: libraries/joomla/session/handler/joomla.php

@anibalsanchez did you by chance test this on 4.0? This PR is for 3.9.x.

We are testing on J4.