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

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

Closed
JulienPalard opened this issue Jan 6, 2016 · 12 comments
Closed

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

JulienPalard opened this issue Jan 6, 2016 · 12 comments

Comments

@JulienPalard
Copy link

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
Copy link
Author

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

@abrookbanks
Copy link

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

@JulienPalard
Copy link
Author

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.

@abrookbanks
Copy link

Great!!

@wisskid wisskid added the waiting Waiting for answer label Jan 28, 2020
@wisskid
Copy link
Contributor

wisskid commented Jan 28, 2020

@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.

@asmecher
Copy link
Contributor

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
Copy link
Contributor

A proposed PR for this: #586

@asmecher
Copy link
Contributor

(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.)

@wisskid wisskid removed the waiting Waiting for answer label Mar 21, 2021
@drugurkocak
Copy link

drugurkocak commented Apr 25, 2021

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,

@asmecher
Copy link
Contributor

@drugurkocak, to clarify, was your test done with the pull request I proposed above (#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.

@drugurkocak
Copy link

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,

@lkppo
Copy link

lkppo commented Jan 10, 2022

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

wisskid added a commit to asmecher/smarty that referenced this issue Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants