znuny / Znuny

Znuny/Znuny LTS is a fork of the ((OTRS)) Community Edition, one of the most flexible web-based ticketing systems used for Customer Service, Help Desk, IT Service Management.
https://www.znuny.org
GNU General Public License v3.0
332 stars 82 forks source link

Bug - Fields named content-type are causing issues in AWS Aurora 3.06.0 #548

Open Herrick19 opened 3 months ago

Herrick19 commented 3 months ago

Environment

Expected behavior

Software working normally

Actual behavior

Lots of errors during upgrades, and problems with notification, templates, etc

How to reproduce

We were using AWS Aurora 3.05.2 which is compatible with Mysql 8.0.32 We updated to AWS Aurora 3.06.0 which is compatible with Mysql 8.0.34

We started to have many errors across the Znuny app, errors during upgrades, etc

We were able to isolate the root cause.

Here is how to reproduce:

Try this in Aurora 3.05.2 CREATE TABLE test (`content_type` INT NOT NULL ) ENGINE = InnoDB; INSERT INTO test (`content_type`) VALUES ('1'); INSERT INTO test (content_type) VALUES ('1');

Everything should work fine

Now try the same queries on 3.06.0 The second insert will fail...

It seems that for some reason, "content-type" has become a "reserved word" in Aurora 3.06.0 It's not listed on the official reserved words from mysql https://dev.mysql.com/doc/refman/8.0/en/keywords.html But the problem seems to be related to the addition of Bedrock inside Aurora: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/mysql-ml.html

The problem is that you have a lots of column named "content-type" and that you are not using "backticks" around the field name (like in my insert example above).

Additional information

I've forked the project and did a commit to fix the issue, you can see the diff here: https://github.com/znuny/Znuny/compare/dev...Herrick19:Znuny:dev

All my problems are now fixed so in the meantime if someone finds this, they could use my fork to bypass the problem.

I DID NOT create a pull request as I don't think this is a proper fix. I think it will work ONLY on mysql and will probably break on postgre that you are also supporting (since postgre is not using backtick for field names).

I think the proper fix should probably be in Kernel/System/DB/mysql.pm I would suggest to loop on all field names and table names and pad them with backticks. I tried to implement the fix without success.

I have also reported the problem to AWS since I don't know if they'll fix it or not (depending if they think it's a bug or a feature :). If they don't fix it, everyone using Aurora won't be able to use Znuny.

Herrick19 commented 3 months ago

Update:

I've received a response from AWS that this is not a bug, but a feature :)

Based on this page: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraMySQLReleaseNotes/AuroraMySQL.Updates.3060.html

"content-type" (and some others) are now official reserved words in aurora.

So a fix in Znuny is really important otherwise all installations will break when users of Aurora update to 3.06.0

hanneshal commented 3 months ago

Hi, thanks for the headsup. We already saw this in the docs. Problem beeing, we do not offically support forks of other DBMS then the ones mentioned in the docs. Especially when they to decide to create random lists of reserverd word ins patch or minor releases. If I'm correct aurora started to be 100% compatible with Maria / Mysql - IMHO they dropped this promise with such moves and will start to drift more and more.

If we start going down this rabbit hole, we need to adjust a lot of backend code to handle this properly.

And this is, as you already mentioned, not just simple search and replace of the word itself. We would / will need escaping for the field names from the start on.

We will use this issue for propper research, but wont put a timeline on this (for now).

Regards Johannes

Herrick19 commented 3 months ago

Hi,

Ok, thanks for the follow up.

I understand you do not wish to invest time for a fork, so in the meantime, people car use my fork of znuny to merge the changes.

Would there be an easy way to check if we are on mysql or not so that I can improve my fork with a syntax similar to this:

my $sSQL = "SELECT content-type FROM Table";

To make it like this

my $sSQL = "SELECT " . ($bIsMysql ? "`content-type`" : "content-type") . " FROM Table";

This way, this fix would work without touching the mysql.pm file.

Everywhere where "content-type" is used, we would enclose it in backticks

So basically my question is: Is there a variable to know easily if we are in mysql or not ?

If so, I'll add it to my fork

Thanks in advance

hanneshal commented 3 months ago

Hi,

we ill invest time for this, just not at the moment. There needs to be some planning done first.

Regarding your question: You could try to select the version. As there is a very basic error handling, it would be a bit cleaner. (If the user is allowed to do so.)

At the moment there are no differences (at least for now) with Maria / MySQL so no special variable is available. We will split Maria / MySQL in the future.

The problem with the escapeing in your code is more like a general Znuny one. There are a lot of queries (+addons) where SQL statements are build from text or sysconfig - or even user input (DF Database). It is not always easy to find all occurences - which led us in the team to the conclusion that it will be necessary to escape every single "column" to get rid of this kind of issue. We had the same issue with groups in MySQL 8

Regards Johannes