Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A hashtag prefixing the default value leads to the default value being stripped #9251

Closed
schultz-it-solutions opened this issue Feb 29, 2016 · 24 comments

Comments

@schultz-it-solutions
Copy link
Contributor

I am not sure where the actual issue sits, so please bear with me, if this is the wrong place to report it... (it very well might be my own misunderstanding of some basics)

Steps to reproduce the issue

clean install of Joomla CMS 3.4.8
update of Joomla CMS 3.5.0-beta3
update database to correct all "database schema errors".
attempt to install (thirdparty) component jDBexport (which I am responsible for).

Expected result

successful installation of the component

Actual result

During installation I get an SQL error

You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use ...
The SQL query shows like the following in this message (I am omitting irelevant parts of it):

CREATE TABLE IF NOT EXISTS #__jdbexport_worksheets
( id int(11) NOT NULL AUTO_INCREMENT,
title varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
......
colheading_text_align_vert varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'top',
colheading_text_col varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ' colheading_bg_col varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ' data_text_font varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'Arial',
...
ordering int(11) NOT NULL DEFAULT '0',
PRIMARY KEY (id)
)
ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

.....
The interesting part comes in the colheading_text_col and colheading_bg_col columns (and a few other columns as well):
The default values (in the SQL install-script) for these columns are "hex color codes" like #000000 or #eeeeee.

** colheading_text_col varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '#eeeeee',
**

however the JOOMLA installation process seems to strip these default values (and the corresponding close-quote and comma) away from the SQL statement, resulting in an SQL error of course!

System information (as much as possible)

Additional comments

@brianteeman
Copy link
Contributor

brianteeman commented Feb 29, 2016 via email

@schultz-it-solutions
Copy link
Contributor Author

sorry, yes of course!

@brianteeman
Copy link
Contributor

brianteeman commented Feb 29, 2016 via email

@schultz-it-solutions
Copy link
Contributor Author

Sure!
However, I changed the install SQL to support "utf8mb4" for this installation. I will verify that the previous "utf8" supporting installation indeed will install on Joomla 3.5.0-beta3 -

@schultz-it-solutions
Copy link
Contributor Author

I do indeed get the very same error when trying to create the table without specific collation and charsets (as the component installation did successfully in the last 3,5 years for a few hundred users worldwide).
So from my point of view, this seems to be linked to 3.5.0 somehow...
The 3.5.0-beta3 testsite itself is running without issues (as far as I can tell)

@schultz-it-solutions
Copy link
Contributor Author

submitting the respective SQL "create table" statement from the install-script directly into PHPMYADMIN on the same server, the table is created successfully, including the named default values!

@andrepereiradasilva
Copy link
Contributor

could the problem be related to this change? #9137

just guessing, but since you use # ...

@schultz-it-solutions
Copy link
Contributor Author

Indeed this sounds like either a very unlikely coincicdence, or actually more probable "the cause of the issue" (although I did not - yet - inspect the actual patch and what it does)...

@yvesh
Copy link
Member

yvesh commented Feb 29, 2016

Can confirm this issue.. This is going to break a lot of extensions which are storing for example color values in the database. You can test it with the attached sample extension.

sampleerror.zip

@zero-24
Copy link
Member

zero-24 commented Feb 29, 2016

@wilsonge This is another 3.5 blocker i guess :(

@andrepereiradasilva
Copy link
Contributor

yes but it doesn't seem related to utf-8 mb4

@zero-24
Copy link
Member

zero-24 commented Feb 29, 2016

Yes the Problem is this PR: #9137

Maybe we just revert or fix the regex

@wilsonge
Copy link
Contributor

@marcodings can you have a look please?

@wilsonge
Copy link
Contributor

Hopefully Marco can have a look and fix it - if not we will revert. I've added a 3.5 blocker label to the issue so we know it needs to be fixed before release!

@andrepereiradasilva
Copy link
Contributor

can a mantainer change the title of this?

@wilsonge wilsonge changed the title Error on converting to UTF-8 Multibyte A hashtag prefixing the default value leads to the default value being stripped Feb 29, 2016
@wilsonge
Copy link
Contributor

Done :)

@schultz-it-solutions
Copy link
Contributor Author

Thank you wilsonge, sorry for the wrong title...
I can test a patch as soon as it is available...

By the way, it is not only 'default values' that are relevant, also 'comments' containing a hashtag (# is used as 'number' or 'No.' representation in english), or maybe even simple UPDATE or INSERT statements, where the value contains a hashtag...

@wilsonge
Copy link
Contributor

wilsonge commented Mar 1, 2016

OK I've tested a patch at work which I've applied directly to the CMS - can you test with the latest staging please?

@schultz-it-solutions
Copy link
Contributor Author

I can positively confirm that the installation of my component (jDBexport) is successful now (after I updated the "driver.php" . I will have to do a full test (as described in "steps to reproduce") yet, but it looks like the issue is solved.
Thank you.

@wilsonge
Copy link
Contributor

wilsonge commented Mar 1, 2016

Awesome :) Thanks for the test :)

wilsonge pushed a commit that referenced this issue Mar 1, 2016
@wilsonge
Copy link
Contributor

wilsonge commented Mar 1, 2016

We've just added a tad more to improve the robustness slightly (thanks @marcodings ) - I can't imagine there being anymore problems - but if you have any time to check would be appreciated!

@schultz-it-solutions
Copy link
Contributor Author

yes, will do - however I seem to have difficulties to make a clean install with the "download-from-staging" version:

Before I can open the second INSTALLATION page, I get:
ReferenceError: Install is not defined

This certainly does not have to do anything with the issue at hand, but I need first find out what is going wrong here... sorry

@schultz-it-solutions
Copy link
Contributor Author

I have successfully tested this latest patch (though I am having the above mentioned issues with installing from "cms-staging" ) :
Steps performed:

  1. download joomla-3.5.0.beta3.zip
  2. exchange "libraries/joomla/database/driver.php" with that from "cms-staging"
  3. clean-install Joomla
  4. clean-install jDBexport : SUCCESSFUL
  5. initiated a "INSERT INTO" for the respective database table: SUCCESSFUL
  6. initiated a "UPDATE TABLE" for the respective database table record: SUCCESSFUL
  7. executed the component's "create excel workbook" command, which itself does use these tables and it's color-fields: SUCCESSFUL (everything as expected)

@yvesh
Copy link
Member

yvesh commented Mar 1, 2016

Patch is working.. Thanks @marcodings @wilsonge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants