Skip to content

Ensure ctype_string is NULL for C locale #5542

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Ensure ctype_string is NULL for C locale #5542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ext/pcre/php_pcre.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,8 +595,7 @@ PHPAPI pcre_cache_entry* pcre_get_compiled_regex_cache_ex(zend_string *regex, in
zend_string *key;
pcre_cache_entry *ret;

if (locale_aware && BG(ctype_string) &&
(ZSTR_LEN(BG(ctype_string)) != 1 && ZSTR_VAL(BG(ctype_string))[0] != 'C')) {
if (locale_aware && BG(ctype_string)) {
key = zend_string_concat2(
ZSTR_VAL(BG(ctype_string)), ZSTR_LEN(BG(ctype_string)),
ZSTR_VAL(regex), ZSTR_LEN(regex));
Expand Down
9 changes: 7 additions & 2 deletions ext/standard/string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,7 @@ PHPAPI zend_string *php_string_tolower(zend_string *s)
unsigned char *c;
const unsigned char *e;

if (EXPECTED(!BG(locale_changed))) {
if (EXPECTED(!BG(ctype_string))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated: What's the reasoning for performing this operation for strtolower() but not strtoupper()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there is no zend_string_toupper :) We only perform ASCII lowercase internally.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

return zend_string_tolower(s);
} else {
c = (unsigned char *)ZSTR_VAL(s);
Expand Down Expand Up @@ -4801,7 +4801,12 @@ PHP_FUNCTION(setlocale)
if (BG(ctype_string)) {
zend_string_release_ex(BG(ctype_string), 0);
}
if (len == ZSTR_LEN(loc) && !memcmp(ZSTR_VAL(loc), retval, len)) {
if (len == 1 && *retval == 'C') {
Copy link
Contributor

@TysonAndre TysonAndre May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly certain that retval was the original locale, not the new locale. - setlocale() and php_my_setlocale() return the original locale if it changed.

This should check ZSTR_VAL(loc) instead, e.g. zend_string_equals_literal(locale, "C")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setlocale() returns the locale that was set, not the previous one. This is necessary because it may not be the same as what was passed to the setlocale() call, e.g. when using "" environment locale.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, good. I was mixed up with other C and PHP APIs that did that - the c code is returning the new locale

https://en.cppreference.com/w/c/locale/setlocale
https://www.php.net/manual/en/function.setlocale.php#refsect1-function.setlocale-returnvalues

/* C locale is represented as NULL. */
BG(ctype_string) = NULL;
zend_string_release_ex(loc, 0);
RETURN_INTERNED_STR(ZSTR_CHAR('C'));
} else if (len == ZSTR_LEN(loc) && !memcmp(ZSTR_VAL(loc), retval, len)) {
BG(ctype_string) = zend_string_copy(loc);
RETURN_STR(BG(ctype_string));
} else {
Expand Down