Hi Cyril,

On Tue, May 17, 2016 at 07:55:01PM +0200, Cyril Bonté wrote:
> Hi all,
> 
> Le 14/05/2016 00:13, Willy Tarreau a écrit :
> > Hi Maxime,
> > 
> > On Sat, May 14, 2016 at 12:09:07AM +0200, Maxime de Roucy wrote:
> > > Hello Willy,
> > > 
> > > Le vendredi 13 mai 2016 à 19:22 +0200, Willy Tarreau a écrit :
> > > > Maxime, could you please add a call to setlocale(LC_ALL, "") as
> > > > suggested by Nenad before starting to scan the directories ? If
> > > > something isn't 100% clear to you, do not hesitate to ask for more
> > > > details, you won't look stupid, I did before you :-)
> > > 
> > > It's clear.
> > > I didn't know either that I had to call setlocale to get the locale
> > > from the environment. I read the man strcoll and thought it would use
> > > LC_COLLATE (from the environment) without other call.
> > 
> > That's the benefit of posting patches to the list as you can see :-)
> > 
> > > I add the setlocale call in the main function because it doesn't set
> > > any haproxy variable. But "init" also sound like a good place.
> > > Let me know if you prefer to move setlocale from main.
> > 
> > Let's leave it there at least we can't miss it if we look for it. It's
> > easy to move if someone is unhappy with it.
> 
> Now we have setlocale(LC_ALL, ""), I was wondering what potential impact we
> could meet with existing configurations and if we should consider it normal.
> 
> For example, some regexes may have different behaviours depending on the
> locale (using the OS libc standard regex implementation, not PCRE).
> 
> Example :
> # locale.cfg content :
>   defaults
>     mode http
> 
>   frontend test
>     bind :9000
>     mode http
>     use_backend testbk if { hdr_reg(X-Test) ^\w+$ }
> 
>   backend testbk
>     mode http
>     server s 127.0.0.1:80
> 
> Launching haproxy with an UTF-8 locale :
> $ LANG=fr_FR.UTF-8 ./haproxy -f locale.cfg
> $ curl -i -H "X-Test: échec" localhost:9000
> HTTP/1.1 200 OK
> ...
> 
> But the same with the C locale :
> $ LANG=C ./haproxy -f locale.cfg
> curl -i -H "X-Test: échec" localhost:9000
> HTTP/1.0 503 Service Unavailable
> ...

Hmmm that's really really bad :-(  Welcome to the world of locales :-(

> Some LUA scripts may change their behaviour too. For more details :
> http://lua-users.org/wiki/LuaLocales
> 
> So, I wonder if we shouldn't limit the setlocale() to the
> cfgfiles_expand_directories() function only.
> - setlocale(LC_ALL, "") when entering the function
> - setlocale(LC_ALL, "C") when leaving the function

Probably. Another possibility would simply be to totally get rid of the
sorting function which requires the locale, implement the sort ourselves
and precisely document how it works.

At least we now need to find a way to fix this because we can't accept
to have such random breakage in a way that it all but auditable. When
you host 1000 customers and 50 of them start to report random breakage
after an upgrade, you can only downgrade with no way to spot what's
wrong.

Willy


Reply via email to