Bootstrap should reset locale settings

Let's fix that once and for all.

Let's also remove the existing (narrower) call to setlocale().

Makes sense and I don't see any concern why we shouldn't be doing this.

@fliegermaus: I would put that on top of conf_init().

Is it possible to get a test for this somehow?

I think it is testable by setting locale in a test module. But I tried reproducing the bug and I couldn't no matter what I set the locale to. Can anyone else still reproduce this?

I've observed this in drupal_to_js, but I can only reproduce it when using a custom module that erroneously calls setlocale(). In that case, even this patch probably won't help because the borkage happens after the bootstrap.

I don't really understand how we use the PHP locales in Drupal, but if we always want the C locale (and defer formatting to the application, i.e. http://api.drupal.org/api/group/format/7 ) then this patch seems sensible.

Tangentially, it seems we are missing a way to format numbers. You'll see contrib modules (e.g. xmlsitemap) do insane things like t('0.8'), (with accompanying .po files full of number tables) because there's no number_format() that pays attention to global $language. (see #52153: Add Number::format with settings to configure decimal point and thousands separators ).

I don't think this really needs a test, it would just be assertTrue(strval(0.8) === '0.8'), and if that weren't true, db_query() would be exploding all over the place.

Ok. Looks like this is basically just moving some code around, then. Committed to HEAD.

This doesn't apply cleanly to 6.x.

Looks like this is basically just moving some code around, then

Except that it isn't :)

+ // numbers handling.
+ setlocale(LC_ALL, 'C');

- // Set the standard C locale to ensure consistent, ASCII-only string handling.
- setlocale(LC_CTYPE, 'C');

LC_CTYPE is the category for character classification and conversion, LC_ALL all categories (character classification, numerics, monetary etc). That said, it makes perfect sense to have a defined locale for all these categories.

This was definitely intended.

Same problem with decimal delimiter with saving float values to database, example:
When locale is en_US:

INSERT INTO {content_type_node} (vid, nid, field_decimal) VALUES (1, 1, 123.45)
UPDATE content_type_seoreport SET vid = 1, nid = 1, field_decimal = 123.45 WHERE vid = 1

But when I have ru_RU locale, I see the query:

INSERT INTO {content_type_node} (vid, nid, field_decimal) VALUES (1, 1, 123,45)
UPDATE content_type_seoreport SET vid = 1, nid = 1, field_decimal = 123,45 WHERE vid = 1

But changing locale to C globally, as I thing, not very good method, because users use locale settings to get month names, start day of week, currency format and other locale specific settings. For solve the issue we must only change locale for decimal point, so we can change only one locale category:
setlocale(LC_NUMERIC, 'C');

For example, I use ru_RU locale for changing first day of week to monday, because I need this in my modules. And if Drupal changes all locales to C, my module goes to work wrong.

Will this patch be applied on Drupal 6.x?

In my instance this patch did not help as it was an errant module making the change later on. I actually could not find any module using 'setlocale' (other than devel which was using setlocale(LC_NUMERIC, 'C'); ), so I'm not sure how it is happening. It could be something stored in the db and eval'd... it's hard to say.

Either way, I created a module (yes, it is a bit of a kluge) that hooks in a variety of places and resets the LC_NUMERIC. In my case, this module fixed all my comma vs decimal place issues.

Since you could have this issue introduced nearly anywhere on your site (and in a large installation never find it) I think that Drupal itself should ensure that all floats are formatted with periods for decimals before js and sql is written.

hi subscribers, could you please have a look at #1333206: drush eval and php -r do not proceed from consistent encoding settings ?
I think the fix was wrong, application should respect system settings.

To the original poster, if your system is setup to print floats with a comma-separated decimal but want them point-separated, then either:
* change your system settings to use the C locale
* use a string instead of a float (that's advised for version numbering which may contains tons of non-numeric characters like "alpha", "-", ... )
* tweak setlocale() in the very specific part of your code/module then restore it afterward (if it's possible)

Indeed its a very important subject and having more tests would probably help avoiding the data-encoding disaster.

Same comment here: Drupal should not respect the locale settings of the server. Ever. Drupal outputs everything in UTF8, with its own character handling functions, and doesn't depend on UTF8 locales to be installed on the server.

For example of things that can happen when you respect the server settings, see the amazing PHP Bug #18556.

Respecting the locale settings of the shell *might be* in the scope of Drush, but it's not in the scope of Drupal.

So at least the parts of Drupal which need a C locale should be well indexed at documented
so the the system's locale tweaking is reduced to its bare minimum.
Eg: instead of a mandatory setlocale(LC_ALL, "C");
the following would be less error-prone in the long-term up to the point where php itself is fixed.

if(  localeconv() [ "decimal_point" ] != "." )
  setlocale(LC_NUMERIC, "C");

I came up in #1333206: drush eval and php -r do not proceed from consistent encoding settings with an easily reproducible (thus drush)
test-case but my problem was not a problem with drush, I was getting
trouble in an attempt to match accentuated taxonomy terms.
The fact that iconv() fails to transliterate my fr_FR.UTF8 strings is a problem (not to say a bug).
But I'm totally fine with
setlocale(LC_NUMERIC, "C") as it does not [seem to] affect the way iconv() transliterate characters.

The fact that some low-level PHP extensions that rely on the locale settings (like gettext, iconv, etc.) do not work properly inside Drupal is not a bug. Those extensions should not be used in the context of a web-server application because: (1) they depend on the locales that are compiled on the server and (2) because locale settings are per-process, so they are going to fail on multi-threaded servers.

Setting a sane LC_ALL=C is definitely what we want here.

some low-level PHP extensions that rely on the locale settings (like gettext, iconv, etc.) do not work properly inside Drupal

iconv() depends on the locale, and my locale is correctly set. php -r gives the expected result.
The concern is rather about Drupal which stops PHP (-extensions) from behaving correctly.

about 1) how would gettext work without the locale being installed on the system ?
bundling hundreds of locales which would be useless for most people is probably not an option
nor something users would appreciate. As an analogy, how can we have Drupal in french
without a ".fr.po" ?

about 2) no setlocale() ? then Drupal should not use (nor reset the locale) in the first place
and adapt itself to the locale chosen by the administrator instead.
Moreover the "low-level [iconv()] PHP extension" is used by unicode.inc itself,
up to modules/aggregator/aggregator.parser.inc<code> (through <code>drupal_xml_parser_create()).

$ echo héllô|LC_CTYPE=C iconv -f utf8 -t ascii//translit
$ php -r 'echo iconv("utf8", "ASCII//TRANSLIT", " héllô");'
$ drush ev 'echo iconv("utf8", "ASCII//TRANSLIT", " héllô");'

So, if I'm not expected to use setlocale() in a module, how will Drupal
allow me to do my transliterations (in a threadsafe fashion) ?
(or should I completely forget about iconv() ?)

Otherwise, one may want to start implementing the following (in php) => unix philosophy --
http://git.php.net/?p=php-src.git;a=blob;f=ext/iconv/iconv.c;hb=refs/hea...
http://sourceware.org/git/?p=glibc.git;a=tree;f=iconv;hb=master

Drupal should restrict its modification of the environment to the minimum, thus setlocale(LC_NUMERIC) is fine
as long as it workarounds php bugs, but setlocale(LC_ALL, C) will cause more and more trouble
in the long run as it only put the issues under the carpet for a time.

Somehow related, http://sourceware.org/bugzilla/show_bug.cgi?id=6040 :
when glibc guys changed the month/day abbr, this changed the `ls` command output (for the best).
That probably broke some scripts of french people relying on the ls -l output
but those scripts were badly written in first place.

Rather than threadsafety, I would be happy to see Drupal, as a webapp, locale-safe.

let's test this problem about floats.

Here is a backport of #3 to Drupal 6. People are still bumping into this on misconfigured server platforms (see for example #254444-13: RSS Feeds node_feed, request to date(). ).

I marked #1366276: lock_acquire() fails miserably when a comma is used as decimal separator (through LC_ALL or LC_NUMERIC) as a duplicate. A lot of people seem to be bumping into this.

It has not been established that #254444: RSS Feeds node_feed, request to date(). is related to setlocale() : gmdate() is safe (only strftime() is not).
I just replied on #1366276: lock_acquire() fails miserably when a comma is used as decimal separator (through LC_ALL or LC_NUMERIC) : the conversion specifier should be switched from %f to %F because
D6 lock.inc should not behave in a locale-dependant fashion.

[ curious about testing patch #23 against D7 ?
If that fails (if there are enough tests out there), then we may want to test a LC_ALL => LC_NUMERIC patch ]

I discovered a bug with the patch from #3 that has been applied to D7 (and D8). To not hijack this D6 issue, I created a new one for D8/D7: #1561214: Bootstrap sets C locale, but does not set UTF-8 character encoding. So please chime in on that issue as well. Basically it comes down to the PHP function escapeshellarg() removing accented characters.

Note that the change from that issue might be incorporated into the D6 patch here that is awaiting review.

replying myself to #22 : incredible, but Drupal did it: #567832: Transliteration in core (+1 NIH point)