? Success

User tests: Successful: Unsuccessful:

avatar zero-24
zero-24
18 Jul 2016

Pull Request for Issue #11117 .

Summary of Changes

set $this->input if you update from 2.5 to 3.x

Why

Since 3.5 we need to relogin if we update from an old version. But only if you relogin the old files get deleted.
The relogin fails because of files that did not removed yet.

Testing Instructions

install 2.5
Update to 3.6
see the issue

Notice: Undefined property: LoginController::$input in ...administrator\components\com_login\controller.php on line 36
Fatal error: Call to a member function set() on null in ...administrator\components\com_login\controller.php on line 36

roll back to 2.5
install the update to 3.6 with the changes in this patch (i'm going to prepare a update server and package later)
there should be no issues

avatar zero-24 zero-24 - open - 18 Jul 2016
avatar zero-24 zero-24 - change - 18 Jul 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Jul 2016
Labels Added: ?
avatar mbabker
mbabker - comment - 18 Jul 2016

Considering the files are supposed to be deleted within the restore.php
file now this to me looks more like a hack similar to all the "sanity
checks" we have in plugins because the app and DB properties aren't being
set correctly versus a correct fix. Why aren't the files being deleted
before being redirected back to Joomla?

If it's because of that overzealous session "fix" in the update script
that's causing a break, that needs to be fixed. Not resetting a class
property because something else in the system is failing to do its job
right.

On Monday, July 18, 2016, zero-24 notifications@github.com wrote:

Pull Request for Issue #11117
#11117 .
Summary of Changes

set $this->input if you update from 2.5 to 3.x
Why

Since 3.5 we need to relogin if we update from an old version. But only if
you relogin the old files get deleted.
The relogin fails because of files that did not removed yet.
Testing Instructions

install 2.5
Update to 3.6
see the issue

Notice: Undefined property: LoginController::$input in ...administrator\components\com_login\controller.php on line 36
Fatal error: Call to a member function set() on null in ...administrator\components\com_login\controller.php on line 36

roll back to 2.5
install the update to 3.6 with the changes in this patch (i'm going to
prepare a update server and package later)

there should be no issues

You can view, comment on, or merge this pull request online at:

#11174
Commit Summary

  • Update controller.php

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#11174, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoRDkPQXQLjk-hYIADB6_E3ihhQ5kks5qW2xAgaJpZM4JOpgZ
.

avatar zero-24
zero-24 - comment - 18 Jul 2016

Considering the files are supposed to be deleted within the restore.php

correct but bevor that script run you need to relogin and that fails because of that issue ;)

avatar mbabker
mbabker - comment - 18 Jul 2016

Sounds like there is a major issue in the update steps then. Because
restore.php itself does not force you to reauthenticate, that comes from
the update script's overzealous session "fix", long after restore.php is
finished running.

On Monday, July 18, 2016, zero-24 notifications@github.com wrote:

Considering the files are supposed to be deleted within the restore.php

correct but bevor that script run you need to relogin and that fails
because of that issue ;)


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11174 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoRHHbwNaS7nGnfM0BeNH9UZ0h-A0ks5qW28tgaJpZM4JOpgZ
.

avatar zero-24
zero-24 - comment - 18 Jul 2016

Sounds like there is a major issue in the update steps then.
long after restore.php is finished running.

hmm i have never understand that script. I'm out than. If I'm correct you got logged out that time we change the session stuff in the lib folder and than reload the page (during the update steps). If i remember correct you should never be logged out by the script.php (or something bad happen) but this is above my pay grade and just my thinking ;)

avatar mbabker
mbabker - comment - 18 Jul 2016

So first thing is the restore.php file itself is stateless with regards to
the session (see
https://github.com/joomla/joomla-cms/search?utf8=✓&q=session+path%3Aadministrator%2Fcomponents%2Fcom_joomlaupdate&type=Code
https://github.com/joomla/joomla-cms/search?utf8=%E2%9C%93&q=session+path%3Aadministrator%2Fcomponents%2Fcom_joomlaupdate&type=Code
for references). As it runs totally outside the regular CMS environment
this makes sense. FWIW it tracks state in other manners, not really
important here.

The restore.php file itself has provisions to include a custom finalization
step, which we started doing with 3.5.1. I'm on mobile so finding these
references isn't easy, but in the relevant PR it was brought up how the
system refreshes itself to ensure that finalization step runs with the
current code. In there is where we placed code to clear byte cache systems
(APC and OPCache) as well as deleting our old files.

If there were an issue with the session related changes in the 3.4
releases, this reauthentication thing would have popped up before now.
That tells me there isn't an issue with those if it's only an issue with
3.6.

The only real change from 3.5 to 3.6 with sessions is how the JUser object
is serialized and refreshed. If that were the culprit it would affect 3.5
upgrades too; it doesn't.

This leads me to believe something is not happening in the order expected
causing the need for this hack to be submitted. Does it fix an immediate
issue? Sure, but by no means is this a right fix. What is the underlying
cause for this reauthentication issue? Is it happening within restore.php
or after it redirects to Joomla? If after, why are the deleted files not
yet deleted?

All this PR tells me as a developer is that I cannot rely on Joomla to
properly initialize class member variables, based on this and past hacks.
I'd rather not have that feeling, I want to know the root cause for the
hacks regarding the need to reinitialize those variables versus us
continuing to accept hacks.

On Monday, July 18, 2016, zero-24 notifications@github.com wrote:

Sounds like there is a major issue in the update steps then.
long after restore.php is finished running.

hmm i have never understand that script. I'm out than. If I'm correct you
got logged out that time we change the session stuff in the lib folder and
than reload the page (during the update steps). If i remember correct you
should never be logged out by the script.php (or something bad happen) but
this is above my pay grade and just my thinking ;)


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11174 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoYxQqvwKtpe6_ta1YBA51b5Heij-ks5qW3HpgaJpZM4JOpgZ
.

avatar zero-24
zero-24 - comment - 18 Jul 2016

Does it fix an immediate issue? Sure, but by no means is this a right fix.

I agree with you. But i have no idea what gets wrong. maybe this here: c864a01

Where is this magic javascript that make sure it works in 2.5.28?

I have found: https://github.com/joomla/joomla-cms/blob/2.5.28/media/com_joomlaupdate/update.js but i can't find any call for the php file.

https://github.com/joomla/joomla-cms/blob/3.6.0/media/com_joomlaupdate/js/update.js#L239

The 2.5.28 code don't run any finalizeRestore step.
https://github.com/joomla/joomla-cms/blob/2.5.28/media/com_joomlaupdate/update.js#L161

But I'm sure it is just may bad JS Skills that i can't find that finalizeRestore call in 2.5

avatar mbabker
mbabker - comment - 18 Jul 2016

I agree with you. But i have no idea what gets wrong. maybe this here: c864a01

That added the restoration file, changed nothing else with the update process.

If the call to finalizeRestore isn't in the 2.5 JavaScript that would probably be why files don't get deleted before hitting the 3.x filesystem. Which means reverting #9655 might fix something but it's still a flaky fix at best because it still relies on being able to route into the application stack correctly with a mixed filesystem in place. What assurances do we have that a full request to the update.finalise task in the update component will work correctly with a mixed 2.5/3.6 filesystem in place? Because that's ultimately what we're going to have to maintain.

avatar zero-24
zero-24 - comment - 18 Jul 2016

That added the restoration file, changed nothing else with the update process.

Sorry i don't mean the code i mean the comments there about removing the call ;)

What assurances do we have that a full request to the update.finalise task in the update component will work correctly with a mixed 2.5/3.6 filesystem in place? Because that's ultimately what we're going to have to maintain.

I have not tested it yet but it looks ok as it exists in 3.6.0 and 2.5.28

https://github.com/joomla/joomla-cms/blob/2.5.28/administrator/components/com_joomlaupdate/controllers/update.php#L79
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_joomlaupdate/controllers/update.php#L91

But where we need to add that coding to call the method?
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_joomlaupdate/models/default.php#L564

Maybe here: https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_joomlaupdate/models/default.php#L608

Or re-add the preflight method and add it there? (i get called a few lines above but don't exists in the script.php (staging))

But i have no idea if that method get called after the new files are there?

This way (method preflight) we are sure the delete is called before the update method that maybe log you out if you come from 2.5.

preflight:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_joomlaupdate/models/default.php#L601
update:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_joomlaupdate/models/default.php#L716
postflight:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_joomlaupdate/models/default.php#L746

And the same order is used in 2.5.28 too:
https://github.com/joomla/joomla-cms/blob/2.5.28/administrator/components/com_joomlaupdate/models/default.php

Can you follow / agree with me?

avatar mbabker
mbabker - comment - 18 Jul 2016

I have not tested it yet but it looks ok as it exists in 3.6.0 and 2.5.28

The task exists in both versions, yes. But what assurances do we have that ensure every step from when index.php is told to be executed to the point that the finalise task is triggered that Joomla itself can work in a mixed 2.5 to 3.x filesystem? That's where I'm going with that question.

The use of a preflight method solves nothing. It still comes after the request gets routed to the update component. Really you're just deleting files a few method calls sooner but stuff is still loaded into memory. The whole point of getting that step into a file called by restore.php was to prevent Joomla from executing with a mixed filesystem; if that isn't possible with the 2.5 to 3.x upgrade path the PR I referenced needs to be reverted and we need to take steps to ensure that the required code paths for the upgrade to finish successfully will work with a mixed 2.5 to 3.x filesystem.

The long and short of this is that there are incompatibilities with the 2.5 to 3.current versus 3.old to 3.current upgrade paths and that the code in 3.current needs to be compatible with both, without the introduction of hackish behaviors.

avatar zero-24
zero-24 - comment - 18 Jul 2016

Ah ok. Thanks for the explanation.

But what assurances do we have that ensure every step from when index.php is told to be executed to the point that the finalise task is triggered that Joomla itself can work in a mixed 2.5 to 3.x filesystem?

If i think about it Joomla can't handle that. Before the delete method was moved to the new file we need to hack some plugins (I guess SEF and Debug?!) to work with the mixed filesystem. (defined or not definded $this->input / $this->app etc)

I have no solution for that expected to hack all places that are really needed on the route on the update, removal of the unneded files and next page load.

The other way arround would be that such places (on the route to update) aren't allowed to use $this->app / $this->input etc. as well as system plugins. But this is also not a good solution.

avatar csthomas
csthomas - comment - 18 Jul 2016

Why we can not add a few additional lines to
https://github.com/joomla/joomla-cms/blob/staging/libraries/import.legacy.php#L93
like:

JLoader::register('JControllerLegacy', JPATH_LIBRARIES . '/legacy/controller/legacy.php');
avatar mbabker
mbabker - comment - 18 Jul 2016

The fact we have to manipulate our own autoloader for things to work correctly means we have introduced backward incompatible changes that affect the "one click migration" option.

avatar brianteeman brianteeman - change - 21 Jul 2016
Category Installation
avatar zero-24
zero-24 - comment - 23 Jul 2016

Ok so how to fix it properly?

avatar joomla-cms-bot joomla-cms-bot - change - 23 Jul 2016
Category Installation Administration Components
avatar mbabker
mbabker - comment - 23 Jul 2016

Revert the change that pulled out removing deleted files from the update script for starters. Then start working through the upgrade path to ensure everything runs smoothly. Autoloader overloading should ONLY be added as an absolute last resort. Let's stop adding quick hacks to the core please and properly debug and fix issues!

avatar zero-24
zero-24 - comment - 23 Jul 2016

Revert the change that pulled out removing deleted files from the update script for starters.

done #11282

Then start working through the upgrade path to ensure everything runs smoothly.

The problem is that the new file is not called on update (2.5 -> 3.6). So there is no way i can think off to call that file on update expected call it on script.php

avatar zero-24 zero-24 - change - 23 Jul 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-07-23 17:49:19
Closed_By zero-24
avatar zero-24 zero-24 - close - 23 Jul 2016

Add a Comment

Login with GitHub to post a comment