Skip to main content ↑ ↓
ElkArte 1.1.7 Patch Testing Started by Spuds · · Read 73894 times 0 Members and 3 Guests are viewing this topic. previous topic - next topic

Re: ElkArte 1.1.7 Patch Testing

Reply #15 –

Quote from: tino –
Quote from: Spuds – Well, darn!  Thank you for the extra testing!

I think we should back up a bit and try a simple single line capture and replace allowing for same line "stuff", including none, after the limit.  The old regex with the $ is probably just to greedy. Could you try this, it only has the case modifier and is only trying to find the limit code.

Code: [Select]
		$db_string = preg_replace('~\sLIMIT\s(\d+|{int:.+}),\s*(\d+|{int:.+}).*~i', 'LIMIT $2 OFFSET $1', $db_string);

I’ll try this later, what I posted here seems to work on all the pages I’ve tried so far. Even if it is a bit greedy.

The better fix is to correct the query at source, rather than rewrite it imo.

Re: ElkArte 1.1.7 Patch Testing

Reply #16 –

Quote from: Spuds – Well, darn!  Thank you for the extra testing!

I think we should back up a bit and try a simple single line capture and replace allowing for same line "stuff", including none, after the limit.  The old regex with the $ is probably just to greedy. Could you try this, it only has the case modifier and is only trying to find the limit code.

Code: [Select]
		$db_string = preg_replace('~\sLIMIT\s(\d+|{int:.+}),\s*(\d+|{int:.+}).*~i', 'LIMIT $2 OFFSET $1', $db_string);

Code: [Select]
ERROR: syntax error at or near "JOIN"
LINE 12: JOIN elkarte_messages as m ON o.id_msg=m.id_msg
^
File: /var/www/html/Elkarte/sources/subs/Topic.subs.php
Line: 2198

Note: Your database version is 1.1.6.

Before;
Code: [Select]
string(323) "
		SELECT 
			m.id_msg, m.id_member
		FROM (
			SELECT
				id_msg
			FROM {db_prefix}messages
			WHERE id_topic = {int:current_topic}
			AND (approved = {int:is_approved} OR id_member = {int:current_member})
			ORDER BY id_msg 
			LIMIT 0, 15) AS o 
		JOIN {db_prefix}messages as m ON o.id_msg=m.id_msg
		ORDER BY m.id_msg "
After;
Code: [Select]
string(321) "
		SELECT 
			m.id_msg, m.id_member
		FROM (
			SELECT
				id_msg
			FROM {db_prefix}messages
			WHERE id_topic = {int:current_topic}
			AND (approved = {int:is_approved} OR id_member = {int:current_member})
			ORDER BY id_msg 
		LIMIT 15 OFFSET 0
		JOIN {db_prefix}messages as m ON o.id_msg=m.id_msg
		ORDER BY m.id_msg "

Re: ElkArte 1.1.7 Patch Testing

Reply #17 –

The following works if you want a more limited capture group. You could use \d if you wanted rather than [0-9] I just prefer that.

The (\s*) capture groups are there to catch where you have varying whitespace between the LIMIT and or , I just ignore them in the replacement.

Code: [Select]
$db_string = preg_replace('~LIMIT (\s*)([0-9]+|{int:.+}),(\s*)([0-9]+|{int:.+})~im', 'LIMIT $4 OFFSET $2', trim($db_string));

Re: ElkArte 1.1.7 Patch Testing

Reply #18 –

nods ... or back to the beginning and dropping that $ and s
Code: [Select]
		$db_string = preg_replace('~\sLIMIT\s(\d+|{int:.+}),\s*(\d+|{int:.+})(.*)~i', 'LIMIT $2 OFFSET $1 $3', $db_string);
which should capture any fluff after the limit and add it back in.

QuoteThe better fix is to correct the query at source, rather than rewrite it imo.
Well my preference "way back" was to only support mysql  O:-)

I imagine what is in the code is just a left over, perhaps mysql did not always support compatibility with PostgreSQL LIMIT row_count OFFSET offset syntax?  If it did (I have no idea) it could date way back to old times of the software only supporting mysql.  A redo of those could be done, it would be one of those thankless jobs that makes no real difference but its certainly doable.


Re: ElkArte 1.1.7 Patch Testing

Reply #19 –

Quote from: Spuds – nods ... or back to the beginning and dropping that $ and s
Code: [Select]
		$db_string = preg_replace('~\sLIMIT\s(\d+|{int:.+}),\s*(\d+|{int:.+})(.*)~i', 'LIMIT $2 OFFSET $1 $3', $db_string);
which should capture any fluff after the limit and add it back in.

QuoteThe better fix is to correct the query at source, rather than rewrite it imo.
Well my preference "way back" was to only support mysql  O:-)

I imagine what is in the code is just a left over, perhaps mysql did not always support compatibility with PostgreSQL LIMIT row_count OFFSET offset syntax?  If it did (I have no idea) it could date way back to old times of the software only supporting mysql.  A redo of those could be done, it would be one of those thankless jobs that makes no real difference but its certainly doable.




Not tested it, but that looks like it’ll work. Depends on if you want to stop at the end of the LIMIT OFFSET group or continue.

MySQL has to my knowledge always supported LIMIT OFFSET it’s also had the LIMIT 0,10 and people used that as they are lazy. Fixing it here helps in the fact it catches modifications also.

SimplePortal for instance doesn’t run on PostgreSQL. I run PostgreSQL and MSSQL on my machines, I actually have Elkarte running on MSSQL somewhere just as I was playing with MSSQL on Linux and wanted to see if it’d work. That’s full of regex mind to replace strings and is a bit brittle.

Re: ElkArte 1.1.7 Patch Testing

Reply #20 –

I can definitely see the lazy aspect .... it saved some typing, and replaced it with a glorious debug cycle!

No reason not to change it on 2.0 but pretty well stuck with in in the 1.1x branch. 

I recall SP was MySQL only, but I don't think it was many querys that limit that.  I'd have to check the elk branch of SP to see where the restriction remains but thats a ways down on the old todo list these days.  

Again thanks for your help and debug .. this time to test I added that line to the MySQL query class so all of its limit statements are converted to row_count OFFSET offset syntax

Re: ElkArte 1.1.7 Patch Testing

Reply #21 –

Quote from: Spuds – Again thanks for your help and debug .. this time to test I added that line to the MySQL query class so all of its limit statements are converted to row_count OFFSET offset syntax

Just to confirm no issues with PostgreSQL either and your last suggested fix.

Re: ElkArte 1.1.7 Patch Testing

Reply #22 –

Attached is the latest 1.1.7 patch package ... it incorporates feedback from this thread, a few items from php7.4, error log entries that could be address, and another small drafts bug.  Oh yes I forgot about that double // issue  :-[ if I remember to check I'll add it in the final release.

If you are running the previous patch, just uninstall first, then replace the old package with this one and reinstall.
Last Edit: April 15, 2021, 04:36:35 pm by Spuds

Re: ElkArte 1.1.7 Patch Testing

Reply #23 –

Cool. As it turns out I have been too distracted to test the earlier patch anyway. :D
I'll have a go at this one though.
Master of Expletives: Now with improved family f@&king friendliness! :D

Sources code: making easy front end changes difficult since 1873. :P

Re: ElkArte 1.1.7 Patch Testing

Reply #24 –

I haven't test the new one but I hope I can try it soon.

And once this patch already work on 7.4 and even 8.0, I think halting 1.1 is the best to focus on 2.0.

Re: ElkArte 1.1.7 Patch Testing

Reply #25 –

That is the plan, or at least mine :)

Re: ElkArte 1.1.7 Patch Testing

Reply #26 –

Various errors while uninstalling the old package and reinstalling new one. I was using php7.4, custom theme.

Code: [Select]
Type of error: General
Unknown Error: Invalid characters passed for attempted conversion, these have been ignored
index.php?action=admin;area=packages
File: /var/www/clients/client2/web1/web/sources/subs/UnTgz.class.php
Line: 390


Type of error: General
Notice: Trying to access array offset on value of type null
index.php?action=admin;area=packages;sa=showoperations;operation_key=652;package=ElkArte_117_patch.zip;filename=modifications.xml
File: /var/www/clients/client2/web1/web/sources/subs/BBC/BBCParser.php
Line: 285


Type of error: General
Notice: Trying to access array offset on value of type null
index.php?action=admin;area=packages;sa=showoperations;operation_key=651;package=ElkArte_117_patch.zip;filename=modifications.xml
File: /var/www/clients/client2/web1/web/sources/subs/BBC/BBCParser.php
Line: 285


Type of error: General
Notice: Trying to access array offset on value of type null
index.php?action=admin;area=packages;sa=showoperations;operation_key=640;package=ElkArte_117_patch.zip;filename=modifications.xml
File: /var/www/clients/client2/web1/web/sources/subs/BBC/BBCParser.php
Line: 285


Type of error: General
Notice: Trying to access array offset on value of type null
index.php?action=admin;area=packages;sa=showoperations;operation_key=631;package=ElkArte_117_patch.zip;filename=modifications.xml
File: /var/www/clients/client2/web1/web/sources/subs/BBC/BBCParser.php
Line: 285


Type of error: General
Notice: Trying to access array offset on value of type null
index.php?action=admin;area=packages;sa=showoperations;operation_key=630;package=ElkArte_117_patch.zip;filename=modifications.xml
File: /var/www/clients/client2/web1/web/sources/subs/BBC/BBCParser.php
Line: 285


Type of error: General
Notice: Trying to access array offset on value of type null
index.php?action=admin;area=packages;sa=showoperations;operation_key=628;package=ElkArte_117_patch.zip;filename=modifications.xml
File: /var/www/clients/client2/web1/web/sources/subs/BBC/BBCParser.php
Line: 285


Type of error: General
Unknown Error: Invalid characters passed for attempted conversion, these have been ignored
index.php?action=admin;area=packages;sa=install;package=ElkArte_117_patch.zip
File: /var/www/clients/client2/web1/web/sources/subs/Package.subs.php
Line: 2929


Type of error: General
Unknown Error: Invalid characters passed for attempted conversion, these have been ignored
index.php?action=admin;area=packageservers;sa=upload2
File: /var/www/clients/client2/web1/web/sources/subs/UnTgz.class.php
Line: 390


Type of error: General
Unknown Error: Invalid characters passed for attempted conversion, these have been ignored
index.php?action=admin;area=packages;sa=browse
File: /var/www/clients/client2/web1/web/sources/subs/UnTgz.class.php
Line: 390


Type of error: General
Unknown Error: Invalid characters passed for attempted conversion, these have been ignored
index.php?action=admin;area=packages
File: /var/www/clients/client2/web1/web/sources/subs/UnTgz.class.php
Line: 390


Guest
Type of error: General
Unknown Error: Array and string offset access syntax with curly braces is deprecated
index.php?action=login
File: /var/www/clients/client2/web1/web/sources/subs/Compat.subs.php
Line: 36


Guest
Type of error: General
Unknown Error: Array and string offset access syntax with curly braces is deprecated
index.php?action=login
File: /var/www/clients/client2/web1/web/sources/subs/Package.subs.php
Line: 2420


Type of error: General
Unknown Error: Invalid characters passed for attempted conversion, these have been ignored
index.php?action=admin;area=packages
File: /var/www/clients/client2/web1/web/sources/subs/UnTgz.class.php
Line: 390

Code related to line 390 /sources/subs/UnTgz.class.php
Code: [Select]
	private function _read_current_header()
	{
		$octdec = array('mode', 'uid', 'gid', 'size', 'mtime', 'checksum', 'type');

		// Each file object is preceded by a 512-byte header record on 512 boundaries
		$this->_header = substr($this->data, $this->_offset << 9, 512);

		// Unpack
		$this->_current = unpack('a100filename/a8mode/a8uid/a8gid/a12size/a12mtime/a8checksum/a1type/a100linkname/a6magic/a2version/a32uname/a32gname/a8devmajor/a8devminor/a155path', $this->_header);

		// Clean the header fields, convert octal to decimal as needed
		foreach ($this->_current as $key => $value)
		{
			if (in_array($key, $octdec))
				$this->_current[$key] = octdec(trim($value));
			else
				$this->_current[$key] = trim($value);
		}
	}

Code related to line 285 /sources/subs/BBC/BBCParser.php
Code: [Select]
			// The only special case is 'html', which doesn't need to close things.
			if ($tag[Codes::ATTR_BLOCK_LEVEL] && $tag[Codes::ATTR_TAG] !== 'html' && empty($this->inside_tag[Codes::ATTR_BLOCK_LEVEL]))
			{
				$this->closeNonBlockLevel();
			}

Code related to line 2420 /sources/subs/Package.subs.php
Code: [Select]
function package_crypt($pass)
{
	$n = strlen($pass);

	$salt = session_id();
	while (strlen($salt) < $n)
		$salt .= session_id();

	for ($i = 0; $i < $n; $i++)
		$pass[$i] = chr(ord($pass[$i]) ^ (ord($salt[$i]) - 32));

	return $pass;
}


Code related to line 2929 /sources/subs/Package.subs.php
Code: [Select]
function elk_chmod($file, $mode = null)
{
	$result = false;

	if (!isset($mode))
	{
		if (is_dir($file))
		{
			$mode = 0755;
		}
		else
		{
			$mode = 0664;
		}
	}

	// Make sure we have a form of 0777 or '777' or '0777' so its safe for intval '8'
	if ($mode == decoct(octdec("$mode")))
		$result = @chmod($file, intval($mode, 8));

	return $result;
}


Code related to line 36 /sources/subs/Compat.subs.php
Code: [Select]
function sha1_smf($str)
{
	// If we have mhash loaded in, use it instead!
	if (function_exists('mhash') && defined('MHASH_SHA1'))
		return bin2hex(mhash(MHASH_SHA1, $str));

	$nblk = (strlen($str) + 8 >> 6) + 1;
	$blks = array_pad(array(), $nblk * 16, 0);

	$str_len = strlen($str);
	for ($i = 0; $i < $str_len; $i++)
		$blks[$i >> 2] |= ord($str[$i]) << (24 - ($i % 4) * 8);

	$blks[$i >> 2] |= 0x80 << (24 - ($i % 4) * 8);

	return sha1_core($blks, $str_len * 8);
}

Re: ElkArte 1.1.7 Patch Testing

Reply #27 –

Thanks @ahrasis I've seen some of those errors during the uninstall as well but they only showed up right after the uninstall of 1.1.7 and all seem related to php 7.4 compatibility (that 1.1.7 fixes)

I'm not sure what I can do about those, it almost seems like the error display is set to allow them to display during uninstall and then goes back to normal don't display warnings. 

Are you also only seeming these on the first uninstall screen from 1.1.7 -> 1.1.6 ?  and then no more (well other than the server error log)

Maybe @emanuele has an idea.

Re: ElkArte 1.1.7 Patch Testing

Reply #28 –

If you are moving towards php 8.0 here are the errors I am getting right now not usable
Code: [Select]
PHP 8.0 errors

Deprecated: Required parameter $table follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db.php on line 179

Deprecated: Required parameter $columns follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db.php on line 179

Deprecated: Required parameter $data follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db.php on line 179

Deprecated: Required parameter $keys follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db.php on line 179

Deprecated: Required parameter $table follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db-mysql.class.php on line 711

Deprecated: Required parameter $columns follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db-mysql.class.php on line 711

Deprecated: Required parameter $data follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db-mysql.class.php on line 711

Deprecated: Required parameter $keys follows optional parameter $method in /home/host/public_html/testelk/sources/database/Db-mysql.class.php on line 711
 

Re: ElkArte 1.1.7 Patch Testing

Reply #29 –

Indeed php8 is a strict little bugger!

I think I fixed those errors in the 2.0 branch (but that has a much newer db scheme).  In 1.1.7, hey "depreciated" warnings are not the worst thing, does the site work on 8 otherwise?