this is just a edge case workaround and not something that should exist in production code

In that case someone needs to make a decision and then we can either change this PR or close the two outstanding issues with one foul sweep of the brush...

$this->isStarted()

This will only return true if the session was started by the Joomla API right, and not if "something else" started the session.

$this->getId()

This is nothing more than a wrapper to session_id() anyway :)

This is nothing more than a wrapper to session_id() anyway :)

Better to use the abstractions than hardcode stuff ?

You have my opinion on the matter, whether others agree or not is to be determined. Honestly, session packages are one part of the PHP ecosystem that lack interoperability and part of it does boil down to the internal uses of ini_set() (at least in the cases of our own Session APIs, Symfony, and Aura; don't have the time at the moment to look for other packages and review those).

IMO this place would be better for such ini_set() https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/session/session.php#L132

Not really, the intent with the abstraction layers is to make it so that configuration changes that don't have to be made for an environment aren't. If you ran a custom app off the CMS app stack using JSession you could use a different JSessionHandlerInterface implementation that either doesn't need that ini_set() call or makes different ones altogether.

JSession (and in 4.0 the Framework's Joomla\Session\Session) are really just the interface for folks to read/write data from the session store. The abstraction layers underneath it (storage and handler) are the internal details where these configs start getting made. So JSession should rightfully be as ignorant to outside influences as practical.

Implemented @mbabker request to use api.
Implemented @csthomas request to move settings.

Reverted @csthomas request to move settings on @mbabker suggestion

@PhilETaylor I have a new suggestion.

What about move it to start() method?

No idea - these kind of decisions are above my pay grade - paging @mbabker :)

That could be viable.

Implemented @csthomas request to move to start() :-)

JSessionHandlerJoomla::start() please. Unless there's some absolute critical reason it HAS to be in JSession::start().

Done.

Great, for me ok

@mbabker I thought about JSessionHandlerJoomla::start(), I forgot to mention class name.

I have this issue using Joomla 3.7.5. Is there anything I can do to resolve it until the next release?
(I'm using an unconventional setup, I accept that, but I'm also fighting a time constraint.)

Warning: ini_set(): A session is active. You cannot change the session module's ini settings at this time in /home/.../libraries/joomla/session/handler/joomla.php on line 46

Warning: Cannot modify header information - headers already sent by (output started at /home/.../libraries/joomla/session/handler/joomla.php:46) in /home/.../libraries/joomla/session/handler/joomla.php on line 104
Error displaying the error page: Application Instantiation Error: Failed to start the session because headers have already been sent by "/home/.../libraries/joomla/session/handler/joomla.php" at line 46.

@wetken please don't post same Comment double like in #15680

@wetken It should be easy to copy this file and replace in 3.7.5. This PR was designed for 3.7.
At J3.8 this file was moved to other place.

If you use joomla git on linux and you have shell access you can add this changes by command:

curl https://patch-diff.githubusercontent.com/raw/joomla/joomla-cms/pull/15742.diff | git apply

I’m sorry, Franz, I realised after I had posted on the closed thread, so I re-posted on the other open one, hoping I had done no harm.

If you have a pressing, intractable, issue and you’re not part of the community it can sometimes be difficult to get any attention at all.

Thank you for taking the trouble.

Ken

From: Franz Wohlkönig [mailto:notifications@github.com]
Sent: 11 September 2017 09:40
To: joomla/joomla-cms
Cc: wetken; Mention
Subject: Re: [joomla/joomla-cms] Check if there is a valid session before messing with session ini values (#15742)

@wetken https://github.com/wetken please don't post same Comment double like in #15680 #15680 (comment)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #15742 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AGAs3S6B5RlRlssb2ViNZJfGHRmyY5Exks5shPHVgaJpZM4NOPie .Image removed by sender.

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com/email-signature
Version: 2016.0.8013 / Virus Database: 4782/14955 - Release Date: 09/11/17

Many thanks, Tomasz, that has helped tremendously.

From: Tomasz Narloch [mailto:notifications@github.com]
Sent: 11 September 2017 09:42
To: joomla/joomla-cms
Cc: wetken; Mention
Subject: Re: [joomla/joomla-cms] Check if there is a valid session before messing with session ini values (#15742)

@wetken https://github.com/wetken It should be easy to copy this file and replace in 3.7.5. This PR was designed for 3.7.
At J3.8 this file was moved to other place.

If you use joomla git on linux and you have shell access you can add this changes by command:

curl https://patch-diff.githubusercontent.com/raw/joomla/joomla-cms/pull/15742.diff | git apply

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #15742 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AGAs3ZaOUejxuUQYTNqzq0G0_Dr3TU6zks5shPJjgaJpZM4NOPie .Image removed by sender.

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com/email-signature
Version: 2016.0.8013 / Virus Database: 4782/14955 - Release Date: 09/11/17

@PhilETaylor and @mbabker I would like to ask you guys if there is still any plan to implement this PR? It would resolve a problem in the CiviCRM extension that has been present since Joomla 3.8.0. As CiviCRM has far more Drupal & Wordpress users and developers than it does Joomla, I've been trying to get this fixed out of necessity.

CiviCRM has a cron script that bootstraps the CMS and runs CiviCRM's scheduled jobs. In some environments (PHP 7) it fails to run due to:

Warning: ini_set(): A session is active. You cannot change the session module's ini settings at this time in /var/www/html/membership/libraries/joomla/session/handler/joomla.php on line 48
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.

CiviCRM's cron.php is doing a session_start() and this occurs before it calls JPluginHelper::importPlugin('civicrm'); which leads to Joomla doing the ini_set() in libraries/joomla/session/handler/joomla.php when there is a session already active.

Almost the same thing happens with its REST API and again it does a session_start() before bootstapping Joomla.

I updated Phil's PR for Joomla 3.8 to test this and it does resolve these particular problems (although I don't really know if it's correct for CiviCRM to be doing the session_start() in the first place).

@andrewpthompson If people test it and provide test results then it will have more of a chance of being accepted for merging.

@PhilETaylor Would you be prepared to update your PR to resolve the conflicts so that it's more likely to get tested by other people? Thanks.

Sorry, came at me out of the blue, wasn’t sure what was happening for a moment.

What exactly do you want me to do – what’s my PR?

From: andrewpthompson [mailto:notifications@github.com]
Sent: 05 March 2018 10:42
To: joomla/joomla-cms
Cc: wetken; Mention
Subject: Re: [joomla/joomla-cms] Check if there is a valid session before messing with session ini values (#15742)

@PhilETaylor https://github.com/philetaylor Would you be prepared to update your PR to resolve the conflicts so that it's more likely to get tested by other people? Thanks.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #15742 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AGAs3fH1UbCQztU3xQtzXl_oemlHrMY3ks5tbRaVgaJpZM4NOPie .Image removed by sender.

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com/email-signature
Version: 2016.0.8013 / Virus Database: 4793/15452 - Release Date: 03/04/18

@wetken My question was to PhilETaylor, the author of this pull request.

@PhilETaylor Can you fix the conflicts so I can get it tested over the next two days?

The joy of having a PR open for over a year. ?

It looks like the conflicts come from @mbabker 's commit here 5b92dc6

I will not be able to fix the conflicts this week. This is my busiest week of the year, the first week of the school holidays, and the week everyone wants (paid) work done before I take several weeks 'off' to go on holiday (Tokyo).

Please feel free to close and replace this PR in my absence.

@PhilETaylor Thank you for replying, I will see what we can do.

#21260