Adapting lansuite to abide STRICT_TRANS_TABLES #458

t-h-e · 2020-11-10T19:13:48Z

if the database uses a strict setting STRICT_TRANS_TABLES
(https://mariadb.com/kb/en/sql-mode/#strict_trans_tables)
then incorrect queries (empty string instead of zero) will result in an
error.

STRICT_TRANS_TABLES is part of the defaults in mariadb and mysql
https://mariadb.com/kb/en/sql-mode/#defaults
https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sql-mode-strict

Fix setting int and enum correctly in UPDATE/DELETE query in
MasterForm
Setting more reasonable default valuse (date, datetime, time never had
defaults set and have been removed from db.xml)
field module in ip_locklist was increased in size as it was to small
for some module names

fixes parts of #456

if the database uses a strict setting STRICT_TRANS_TABLES (https://mariadb.com/kb/en/sql-mode/#strict_trans_tables) then incorrect queries (empty string instead of zero) will result in an error. STRICT_TRANS_TABLES is part of the defaults in mariadb and mysql https://mariadb.com/kb/en/sql-mode/#defaults https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sql-mode-strict - Fix setting int and enum correctly in UPDATE/DELETE query in MasterForm - Setting more reasonable default valuse (date, datetime, time never had defaults set and have been removed from db.xml) - field module in ip_locklist was increased in size as it was to small for some module names

… Travis (wrong method name applied in PR #458) refs #455 fixes #461

aj-gh · 2023-01-12T20:12:33Z

Still pending .....

andygrunwald

Thanks a lot for this Pull Request, and sorry for letting it slip so long.

I added a few questions - Let me know what you think.

inc/Classes/MasterForm.php

andygrunwald · 2023-05-28T20:13:17Z

inc/Classes/MasterForm.php

@@ -1117,14 +1117,18 @@ public function SendForm($BaseURL, $table, $idname = '', $id = 0)
                                foreach ($this->SQLFields as $key => $val) {
                                    if (($SQLFieldTypes[$val] == 'datetime' or $SQLFieldTypes[$val] == 'date') and $_POST[$val] == 'NOW()') {
                                        $db_query .= "$val = NOW(), ";
-                                    } elseif ($SQLFieldTypes[$val] == 'tinyint(1)') {
+                                    } elseif ($this->is_field_int($SQLFieldTypes[$val])) {
                                        $db_query .= $val .' = '. (int)$_POST[$val] .', ';
                                    } elseif ($SQLFieldTypes[$val] == 'varbinary(16)' and $val == 'ip') {
                                        $db_query .= $val .' = INET6_ATON(\''. $_POST[$val] .'\'), ';
                                    } elseif ($_POST[$val] == '++' and strpos($SQLFieldTypes[$val], 'int') !== false) {
                                        $db_query .= "$val = $val + 1, ";
                                    } elseif ($_POST[$val] == '--' and strpos($SQLFieldTypes[$val], 'int') !== false) {


Does this check also benefit from is_field_int ?

andygrunwald · 2023-05-28T20:13:25Z

inc/Classes/MasterForm.php

@@ -1117,14 +1117,18 @@ public function SendForm($BaseURL, $table, $idname = '', $id = 0)
                                foreach ($this->SQLFields as $key => $val) {
                                    if (($SQLFieldTypes[$val] == 'datetime' or $SQLFieldTypes[$val] == 'date') and $_POST[$val] == 'NOW()') {
                                        $db_query .= "$val = NOW(), ";
-                                    } elseif ($SQLFieldTypes[$val] == 'tinyint(1)') {
+                                    } elseif ($this->is_field_int($SQLFieldTypes[$val])) {
                                        $db_query .= $val .' = '. (int)$_POST[$val] .', ';
                                    } elseif ($SQLFieldTypes[$val] == 'varbinary(16)' and $val == 'ip') {
                                        $db_query .= $val .' = INET6_ATON(\''. $_POST[$val] .'\'), ';
                                    } elseif ($_POST[$val] == '++' and strpos($SQLFieldTypes[$val], 'int') !== false) {


Does this check also benefit from is_field_int ?

modules/install/Classes/Import.php

andygrunwald · 2023-05-28T20:19:21Z

modules/install/Classes/Import.php

@@ -229,7 +233,7 @@ public function ImportXML($rewrite = null)
                                            // Handle structure changes in general
                                        } elseif ($db_field["Type"] != $type
                                        or (!($db_field["Null"] == $null or ($db_field["Null"] == 'YES' and $null == 'NULL')))
-                                        or ($db_field["Default"] != $default_xml and !($db_field["Default"] == 0 and $default_xml == '') and !($db_field["Default"] == '' and $default_xml == 0))
+                                        or ($db_field["Default"] != $default_xml and !($db_field["Default"] == 0 and $default_xml == '') and !($db_field["Default"] == '' and $default_xml === 0))


Will this ever work without casting/double checks?

$default_xml is set by

$default_xml = $this->xml->getFirstTagContent("default", $field);

getFirstTagContent will always return a string, even is 0 is in it.

Maybe it is better to run something like $default_xml === '0'?

See

lansuite/inc/Classes/XML.php

Lines 16 to 41 in 7e16dd2

public function getFirstTagContent($tag, $input, $save = false)

{

$start = strpos($input, '<' . $tag . '>') + strlen($tag) + 2;

// If the tag has attributes, remove them, for the end tag won't contain them

if (strpos($tag, ' ') > 0) {

$tag = substr($tag, 0, strpos($tag, ' '));

}

$end = strpos($input, '</' . $tag . '>') - $start;

if ($end <= 0) {

return '';

}

if ($save) {

return trim(

str_replace(

'--lt--',

'<',

str_replace('--gt--', '>', substr($input, $start, $end))

)

);

}

return trim(substr($input, $start, $end));

}

andygrunwald · 2023-05-28T20:21:38Z

modules/install/mod_settings/db.xml

@@ -116,7 +116,7 @@
                <type>datetime</type>
                <null></null>
                <key></key>
-                <default>0</default>
+                <default></default>


Can you elaborate a bit why removing the default is necessary here? (same question applies to the other datetime fields)

andygrunwald · 2023-05-28T20:22:40Z

modules/party/mod_settings/db.xml

@@ -98,7 +98,7 @@
 				<type>datetime</type>
 				<null></null>
 				<key></key>
-				<default></default>
+				<default>0000-00-00 00:00:00</default>


What makes this datetime field different from the others when it comes to the default value?

andygrunwald · 2023-05-28T20:23:28Z

modules/tournament2/mod_settings/db.xml

@@ -333,7 +333,7 @@
 			<field>
 				<name>rules</name>
 				<type>text</type>
-				<null></null>
+				<null>YES</null>


Can you elaborate why this is needed?

andygrunwald

Thanks a lot for this Pull Request, and sorry for letting it slip so long.

I added a few questions - Let me know what you think.

andygrunwald · 2023-06-01T10:16:39Z

FYI: #634 was implemented to mitigate this issue slightly. Making LanSuite support STRICT_TRANS_TABLES is still the better way.

Co-authored-by: Andy Grunwald <andy.grunwald@aiven.io>

sonarcloud · 2023-06-07T17:59:16Z

SonarCloud Quality Gate failed.

22 Bugs
7 Vulnerabilities
28 Security Hotspots
626 Code Smells

No Coverage information
6.9% Duplication

sonarcloud · 2023-06-07T17:59:16Z

Please retry analysis of this Pull-Request directly on SonarCloud.

t-h-e · 2023-06-07T18:01:52Z

After 2,5 years, I cannot tell you the reasoning behind all the changes anymore. As far as I remember I made one change after another to make lansuite work STRICT_TRANS_TABLES.
We have a version running with those changes for as long as this PR is open. If there have been other changes to make lansuite work with STRICT_TRANS_TABLES that is fine as well.
If this PR is not needed anymore. Feel free to close it.

andygrunwald · 2023-06-25T11:16:54Z

Thx @t-h-e.
This adds a lot of context.

Would you mind sharing the URL of your installation? Or is it locally?
And the list of modules you have activated?
This helps me to evaluate the impact.

I plan to look into this more in detail and get this merged soon(ish). I believe it is the right direction.

t-h-e · 2023-06-28T19:36:11Z

URL: https://www.haag-networx.at

Activedted modules:

Clanmanager
Downloads
Gästeliste
Info
Mai
Messenger
News
Partyverwaltung
Poll
Sitzplan
Sponsoren
Statistiken
Turnier
About
Anmeldung
Benutzermanager
Boxen
Cronjob
Installation
MasterSearch
Startseite

sonarcloud · 2023-10-14T20:28:43Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
13 Code Smells

No Coverage information
4.8% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

sonarcloud · 2024-04-11T10:56:02Z

Quality Gate failed

Failed conditions
3.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

t-h-e and others added 2 commits November 10, 2020 20:02

Merge branch 'master' into fix/strict-trans-tables

2805ca0

M4LuZ added a commit that referenced this pull request Dec 3, 2020

Raised PHP build versions to 7.2 - 7.4 and hopefully fixed issue with…

bf7ebd9

… Travis (wrong method name applied in PR #458) refs #455 fixes #461

M4LuZ mentioned this pull request Dec 3, 2020

Raise build version, fix Travis CI #462

Merged

andygrunwald pushed a commit that referenced this pull request Dec 3, 2020

Raised PHP build versions to 7.2 - 7.4 and hopefully fixed issue with…

aa7d405

… Travis (wrong method name applied in PR #458) refs #455 fixes #461

Merge branch 'master' into fix/strict-trans-tables

5ac9f68

M4LuZ added the database-modification Ticket includes a database modification that may need (manual) application on existing databases label Mar 28, 2022

andygrunwald requested changes May 28, 2023

View reviewed changes

andygrunwald added the pending-submitter-response label May 28, 2023

github-actions bot added the conflicts label Jun 6, 2023

t-h-e and others added 4 commits June 7, 2023 19:56

Update inc/Classes/MasterForm.php

3ae7dac

Co-authored-by: Andy Grunwald <andy.grunwald@aiven.io>

Update inc/Classes/MasterForm.php

c9a2af2

Co-authored-by: Andy Grunwald <andy.grunwald@aiven.io>

Update inc/Classes/MasterForm.php

d95b779

Co-authored-by: Andy Grunwald <andy.grunwald@aiven.io>

Update modules/install/Classes/Import.php

a00840d

Co-authored-by: Andy Grunwald <andy.grunwald@aiven.io>

Merge branch 'master' into fix/strict-trans-tables

18ae9ac

Merge branch 'master' into fix/strict-trans-tables

0bd0307

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapting lansuite to abide STRICT_TRANS_TABLES #458

Adapting lansuite to abide STRICT_TRANS_TABLES #458

t-h-e commented Nov 10, 2020

aj-gh commented Jan 12, 2023

andygrunwald left a comment

andygrunwald May 28, 2023

andygrunwald May 28, 2023

andygrunwald May 28, 2023

andygrunwald May 28, 2023

andygrunwald May 28, 2023

andygrunwald May 28, 2023

andygrunwald left a comment

andygrunwald commented Jun 1, 2023

sonarcloud bot commented Jun 7, 2023

sonarcloud bot commented Jun 7, 2023

t-h-e commented Jun 7, 2023 •

edited

andygrunwald commented Jun 25, 2023

t-h-e commented Jun 28, 2023

sonarcloud bot commented Oct 14, 2023

sonarcloud bot commented Apr 11, 2024

	public function getFirstTagContent($tag, $input, $save = false)
	{
	$start = strpos($input, '<' . $tag . '>') + strlen($tag) + 2;

	// If the tag has attributes, remove them, for the end tag won't contain them
	if (strpos($tag, ' ') > 0) {
	$tag = substr($tag, 0, strpos($tag, ' '));
	}

	$end = strpos($input, '</' . $tag . '>') - $start;
	if ($end <= 0) {
	return '';
	}

	if ($save) {
	return trim(
	str_replace(
	'--lt--',
	'<',
	str_replace('--gt--', '>', substr($input, $start, $end))
	)
	);
	}

	return trim(substr($input, $start, $end));
	}

Adapting lansuite to abide STRICT_TRANS_TABLES #458

Are you sure you want to change the base?

Adapting lansuite to abide STRICT_TRANS_TABLES #458

Conversation

t-h-e commented Nov 10, 2020

aj-gh commented Jan 12, 2023

andygrunwald left a comment

Choose a reason for hiding this comment

andygrunwald May 28, 2023

Choose a reason for hiding this comment

andygrunwald May 28, 2023

Choose a reason for hiding this comment

andygrunwald May 28, 2023

Choose a reason for hiding this comment

andygrunwald May 28, 2023

Choose a reason for hiding this comment

andygrunwald May 28, 2023

Choose a reason for hiding this comment

andygrunwald May 28, 2023

Choose a reason for hiding this comment

andygrunwald left a comment

Choose a reason for hiding this comment

andygrunwald commented Jun 1, 2023

sonarcloud bot commented Jun 7, 2023

sonarcloud bot commented Jun 7, 2023

t-h-e commented Jun 7, 2023 • edited

andygrunwald commented Jun 25, 2023

t-h-e commented Jun 28, 2023

sonarcloud bot commented Oct 14, 2023

sonarcloud bot commented Apr 11, 2024

Quality Gate failed

t-h-e commented Jun 7, 2023 •

edited