smarty icon indicating copy to clipboard operation
smarty copied to clipboard

strtolower in Autoloader.php won't work using tr_TR locale

Open JulienPalard opened this issue 9 years ago • 12 comments

smarty is using strtolower in Autoload.php but have classes using capitalized "i", this won't work using the tr_TR.utf8 locale:

$ php -r 'var_dump(setlocale(LC_ALL, "tr_TR.utf-8")); echo strtolower("Smarty_Internal") . "\n";'
string(11) "tr_TR.utf-8"
smarty_Internal

Notice the still capitalized I.

Ensure you dpkg-reconfigure locales and install the "tr_TR.utf8" before doing the test, having "tr_TR.utf8" in the first lines shows that the setlocale worked, if you have bool(false), the setlocal did not change the locale so the strtolower test may wrongly succeed.

This happen because in their country they have dotted and undotted i, see: https://en.wikipedia.org/wiki/Dotted_and_dotless_I

JulienPalard avatar Jan 06 '16 09:01 JulienPalard

Also there's other places it won't work, like in loadPlugin, probably other places too.

JulienPalard avatar Jan 06 '16 09:01 JulienPalard

Suggest using mb_strtolower instead if PHP is compiled with multibyte string function.

abrookbanks avatar Apr 12 '16 14:04 abrookbanks

According to my git log, I tried (on Jan 6, the day I reported it), to use mb_strtolower, and it worked like a charm as I was having a site using the tr_TR locale at this time, so yes, it look like the right fix.

JulienPalard avatar Apr 13 '16 08:04 JulienPalard

Great!!

abrookbanks avatar Apr 13 '16 08:04 abrookbanks

@JulienPalard I can confirm this issue still exists. I've beem doing a little debugging, but it seems we need to fix ucfirst too? Without fixing that I quickly run into errors in Smarty_Internal_TemplateCompilerBase::getTagCompiler. There are 40+ occurances throughout de codebase, so this might be somewhat dangerous to fix properly. There may be other upper-/lowercase conversions that need changing.

I've been looking through a couple of bugreports on bugs.php.net and they don't sound encouraging: https://bugs.php.net/bug.php?id=18556

Could you help in creating a PR and test it the tr_TR.utf8 locale? Remember to delete compiled templates regularly to trigger a fresh compile.

wisskid avatar Jan 28 '20 11:01 wisskid

We've just run into this with https://github.com/pkp/ojs, which uses Smarty. In our own code, we resolved it with a special set of functions that are intended for "codesafe" uses (i.e. not locale dependent). See https://github.com/pkp/pkp-lib/blob/master/includes/functions.inc.php#L273..L281 for an example. I'd suggest a similar solution for calls to strtoupper, strtolower, ucfirst, etc. that operate on function/class names.

asmecher avatar Jan 28 '20 17:01 asmecher

A proposed PR for this: https://github.com/smarty-php/smarty/pull/586

asmecher avatar Apr 14 '20 22:04 asmecher

(I'm hoping someone can have a look at the PR I proposed above! We continue to field requests from Turkish-language users of Open Journal Systems who are running into the bug.)

asmecher avatar Feb 22 '21 20:02 asmecher

Hi @asmecher I just did a new git install from OJS origin/stable-3_3_0 branch. I selected only Turkish locale during site install. Without modifying PKPLocale.inc.php file, I immediately generated my first journal in Turkish backend. I see that the site runs properly, and I don't get any fatal error due to Turkish locale. You may see the test site for a temporary time at;

https://ojs3305test.adlitip.net/index.php/itd/index

I can confirm that I can directly install OJS 3.3.0.5 by selecting Turkish locale (alone) and then generate the first journal in the Turkish backend.

Thank you very much for your efforts.

Regards,

drugurkocak avatar Apr 25 '21 22:04 drugurkocak

@drugurkocak, to clarify, was your test done with the pull request I proposed above (https://github.com/smarty-php/smarty/pull/586) in place, or without any changes to OJS?

If you were able to test successfully without any changes to Smarty, I suspect it's because your server doesn't have the Turkish locale installed/enabled, so the setLocale call to select tr_TR isn't actually doing anything. As far as I'm aware, this bug is still present in Smarty.

asmecher avatar Apr 26 '21 01:04 asmecher

Hi @asmecher You are right. I have 2 virtual servers, one Centos 7, and the new one Ubuntu 18.04 LTS. When I ask for locale on Ubuntu server which I made the test; adlitip@ns1:~$ locale -a C C.UTF-8 en_AG en_AG.utf8 en_AU.utf8 en_BW.utf8 en_CA.utf8 en_DK.utf8 en_GB.utf8 en_HK.utf8 en_IE.utf8 en_IL en_IL.utf8 en_IN en_IN.utf8 en_NG en_NG.utf8 en_NZ.utf8 en_PH.utf8 en_SG.utf8 en_US.utf8 en_ZA.utf8 en_ZM en_ZM.utf8 en_ZW.utf8 POSIX So, no Turkish locale are enabled I think.

Whereas on Centos Server which is 2 years old, I get tr_CY tr_CY.iso88599 tr_CY.utf8 tr_TR tr_TR.iso88599 tr_TR.utf8

So, I will install and enable Turkish locale on Ubuntu server, and retest on both. Then, I will share the result in a few days. Regards,

drugurkocak avatar Apr 26 '21 12:04 drugurkocak

PHP RFC: Locale-independent case conversion corrects it for PHP 8.2.

saulery avatar Jan 10 '22 11:01 saulery