[#25144] - [4.0] Update PrefixField.php in Installer
- Closed
- 5 Sep 2020
- Medium
- Build: 4.0-dev
- # 25144
- Diff
- Duke3D:patch-3
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
First report.
Summary of Changes
Added to validation of user-entered database prefix.
- user's response is converted to lowercase so that value stored in configuration.php is in alignment with the default behavior (convert to lowercase) of the MySQL database.
- test for and append underscore, if missing.
Testing Instructions
Run Installer.
A) At Database Prefix field, enter mixed or upper case prefix ending with underscore.
B) Repeat except enter mixed or upper case prefix without underscore.
Expected result
Installed database has lowercase prefix and value matches database prefix as stored in configuration.php.
Actual result
If not patched, installed database has lowercase prefix. Database prefix stored in configuration.php has mixed case prefix. Can lead to database issues on Linux servers and reference errors that appear on server migration.
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Installation |
franz-wohlkoenig
- | Title |
|
||||||
To satisfy the codestyle requirements please address the following issues
FILE: /********/src/installation/src/Form/Field/Installation/PrefixField.php
--
46 | --------------------------------------------------------------------------------
47 | FOUND 4 ERROR(S) AFFECTING 3 LINE(S)
48 | --------------------------------------------------------------------------------
49 | 82 \| ERROR \| Please consider a blank line preceding your comment
50 | 85 \| ERROR \| Please consider a blank line preceding your comment
51 | 86 \| ERROR \| Expected "if (...)\n"; found "if (...) "
52 | 86 \| ERROR \| Closing brace must be on a line by itself
| Labels |
Added:
?
|
||
@Duke3D Please do as requested here: #25144 (comment)
I have tested this item
After applying the patch, the prefix in the database is in lower case (without the patch it's also in lower case). In the configuration.php is the prefix still in mixed cases. So the pr doesen't work well.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25144.
To fix #25144 (comment) , the checks have to be added to 2 places:
- https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Model/SetupModel.php#L313
- https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Model/DatabaseModel.php#L95
(Or maybe just the regex needs to be modified).
If my PR #29362 will be ready and merged, it will be only one place where to change that.
But I am in doubt if this PR here is right, because we had such check already for PostgreSQL, where it seems to matter, e.g. here:
https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Model/DatabaseModel.php#L133
So I think it could have had a reason why we only did it for PostgreSQL and not for MySQL.
But who knows, maybe it was wrong all the time for MySQL.
Closing due to no response.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-09-05 21:52:12 |
| Closed_By | ⇒ | Quy | |
| Labels |
Added:
?
|
||
This deals only for the session prefix. It should also deal with new prefix when modified, therefore out of the if/else.
Also cs is bad.