? Pending

User tests: Successful: Unsuccessful:

avatar wilsonge
wilsonge
29 Jul 2017

Pull Request for Issue #17198 .

Summary of Changes

Fixes a case where parse_ini_file was disabled but for some reason function_exists returns true

Testing Instructions

Disable the function parse_ini_file using an entry like disable_functions=apache_child_terminate, apache_setenv, parse_ini_file (note the spaces after the commas are important)

Expected result

Joomla Language files load

Actual result

They don't

Documentation Changes Required

None

avatar joomla-cms-bot joomla-cms-bot - change - 29 Jul 2017
Category Libraries
avatar wilsonge wilsonge - open - 29 Jul 2017
avatar wilsonge wilsonge - change - 29 Jul 2017
Status New Pending
avatar wilsonge wilsonge - change - 29 Jul 2017
Labels Added: ?
avatar zero-24
zero-24 - comment - 29 Jul 2017

hmm while this may fix this issue but don't we need to implement that kind of thing in all places we use function_exists ? https://github.com/joomla/joomla-cms/search?utf8=%E2%9C%93&q=function_exists&type= and what happen if ini_get is disabled for some reason? For me this is not a bug on our site it is a bug at the hosting site or PHP not triming the values they get from the php ini.

avatar wilsonge
wilsonge - comment - 29 Jul 2017

OK So I just tested locally and even with spaces after the commas' PHP 7.0 still told me that the function didn't exist. So either this is specific to PHP 5.6 or there's something else going on under the hood.

I agree it's not really a bug in the system. But clearly there's something weird going on that guys site and if this fixes the issue I'm ok fixing it for 2 extra lines of code

Most our other uses are in 3rd party libraries or checking for functions inside of Joomla (like the router functions, which practically aren't going to be disabled :) ). The few places remaining are the caching and the session drivers mainly. If we get bug reports there I guess we can look into it. But I don't see anything else in core that screams to me as needing a similar fix

avatar hakanara
hakanara - comment - 29 Jul 2017

@wilsonge I don't know how to test a patch like this. I have downloaded a file called: https://codeload.github.com/joomla/joomla-cms/zip/fix-17198.zip and installed this file via Joomla update component. But nothing changed and the language files are still not parsed. Maybe I did something wrong? If not, I will go into the code and try to change it a bit to see if we did something wrong.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17332.

avatar wilsonge
wilsonge - comment - 29 Jul 2017

@hakanara follow the documentation here https://docs.joomla.org/Testing_Joomla!_patches it should take you through step by step :)

avatar hakanara
hakanara - comment - 29 Jul 2017

@wilsonge I have found an installation directory. Then I started the installation there and everything seems wonderful now. Just give me a couple of minutes to see if I tested it according to the documentation. My first impression is, you've done a great job :-) I will post my test results here.

avatar ggppdk
ggppdk - comment - 29 Jul 2017

The code to trim the 'disable_functions' is already used in installation code, so the trimming issue is not new
https://github.com/joomla/joomla-cms/blob/staging/installation/model/setup.php#L178-L209

But then there is another place that does not do the trimming and needs to be patched ... here:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_admin/models/sysinfo.php#L322-L333

I suggest are static method (located inside some API class) to be used everywhere,
please notice that it includes check for function existence too:

static function functionIsDisabled($function)
{
	static $disabled = null;
	$func = strtolower($function);

	if ($disabled === null)
	{
		$disabled_functions = trim(ini_get('disable_functions'));
		$disabled_functions = $disabled_functions ? explode(',', $disabled_functions) : array();

		$disabled = array();
		foreach ($disabled_functions as $key => $value)
		{
			$disabled[trim($value)] = true;
		}

		// PHP 5.3 only
		if (ini_get('safe_mode'))
		{
			$disabled['shell_exec']     = true;
			$disabled['set_time_limit'] = true;
		}
	}

	// Check both disabled and if it exists
	return isset($disabled[$func]) || !function_exists($func);
}
avatar hakanara hakanara - test_item - 29 Jul 2017 - Tested successfully
avatar hakanara
hakanara - comment - 29 Jul 2017

I have tested this item successfully on 30b3a0a

I tested the patch in a different way than it is being told in the Joomla documentation but the site and the patch is working. I have two Joomla installations under the same domain with the same php configuration. The one I downloaded from here as a patch is working and parsing the language strings but the other one which is Joomla 3.7.4 is not parsing the strings.

Thank you very much who have contributed to the solution.
Kind regards,
Hakan


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17332.

avatar hakanara
hakanara - comment - 29 Jul 2017

I installed Joomla from zero twice. I set up a database from zero twice. But unfortunately the com_patchtester gives me this error:
The file marked for modification does not exist: libraries/src/Joomla/CMS/Language/Language.php

Therefore I downloaded the fix manually, extracted, installed and now confirm that the fix works like a charm :-) I don't really now why I get this error though. @mbabker ?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17332.

avatar zero-24
zero-24 - comment - 29 Jul 2017

You need to install the staging branch and not 3.7.4 ;)

avatar hakanara
hakanara - comment - 29 Jul 2017

@zero-24 Thank you very much. :-D


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17332.

avatar zero-24
zero-24 - comment - 17 Aug 2017

@wilsonge can you please fix the conflicts? Sounds like this is your new hobby these days ;) Should be a simple one for sure.

avatar zero-24
zero-24 - comment - 18 Aug 2017

Merging this based on @mbabker 's review and @hakanara 's test. Thanks @wilsonge

avatar zero-24 zero-24 - close - 18 Aug 2017
avatar zero-24 zero-24 - merge - 18 Aug 2017
avatar zero-24 zero-24 - change - 18 Aug 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-08-18 10:07:10
Closed_By zero-24

Add a Comment

Login with GitHub to post a comment