LemonLDAPNG / Apache-Session-Browseable

Apache::Session::Browseable Perl module
http://search.cpan.org/dist/Apache-Session-Browseable/
Other
3 stars 5 forks source link

Regression in 1.3.10 in MariaDB #35

Closed maxbes closed 1 year ago

maxbes commented 2 years ago

After upgrading to 1.3.10, MariaDB no longer works:

ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ''id','a_session'

Related to https://github.com/LemonLDAPNG/Apache-Session-Browseable/commit/94fa4eec

Quoting column names works in PostgreSQL, but not in MariaDB apparently

guimard commented 2 years ago

Hi @maxbes,

could you test this :

--- a/lib/Apache/Session/Browseable/Store/DBI.pm
+++ b/lib/Apache/Session/Browseable/Store/DBI.pm
@@ -19,9 +19,9 @@ sub insert {

     if ( !defined $self->{insert_sth} ) {
         $self->{insert_sth} =
-          $self->{dbh}->prepare_cached( "INSERT INTO $self->{table_name} (\""
-              . join( '","', 'id', 'a_session', map { s/'/''/g; $_ } @$index )
-              . '") VALUES ('
+          $self->{dbh}->prepare_cached( "INSERT INTO $self->{table_name} ('"
+              . join( "','", 'id', 'a_session', map { s/'/''/g; $_ } @$index )
+              . "') VALUES ("
               . join( ',', ('?') x ( 2 + @$index ) )
               . ')' );
     }
maxbes commented 2 years ago
MariaDB [llng]> INSERT INTO sessions ('id') values(1);
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ''id') values(1)' at line 1
MariaDB [llng]> INSERT INTO sessions ("id") values(1);
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '"id") values(1)' at line 1

In MySQL, simple or double quotes don't work, only back-quotes work. But back-quotes don't work in PostgreSQL

I think you should simply revert your change, https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/issues/2722 can be worked around by using PgJSON or not indexing "user" (only used in MailPasswordReset)

maxbes commented 2 years ago

We could use $dbh->quote_identifier to escape table names in a way that is compatible with all databases.

guimard commented 2 years ago

I tried it : 2912976

coudot commented 1 year ago

What is the status of this issue?

maxbes commented 1 year ago

Regression is no longer present, we rolled back https://github.com/LemonLDAPNG/Apache-Session-Browseable/commit/94fa4eec and https://github.com/LemonLDAPNG/Apache-Session-Browseable/commit/291297615279136d61be293668d96e19b3f79c16