Re: [PATCH] tds_get_locale() update (was Re: SQL Server version and TDS Version)

On Monday, May 24, 2010, at 07:11AM, <[email protected]> wrote:
> On Sun, May 23, 2010 at 06:13:44PM -0500, Craig A. Berry wrote:
>
>> The following calling sequences show how we
>> end up getting what's done in tds_alloc_connection():
>>
>> a.)  tds_alloc_locale*
>>       tds_get_locale
>>         tds_alloc_context
>>
>> b.)  tds_alloc_connection*
>>       tds_read_config_info
>>
>> and what happens in either of these can be overridden from
>> freetds.conf because the following happens later:
>>
>> c.)  tds_try_conf_file
>>       tds_read_conf_file
>>         tds_read_config_info
>>
>>
>> The problem in step b above is that tds_alloc_connection() has the
>> following code in it:
>>
>>        /* TODO use system default ?? */
>>        if (!tds_dstr_copy(&connection->client_charset, "ISO-8859-1"))
>>                goto Cleanup;
>>
>> There is no checking to see if the client charset is already set in
>> the locale structure, so in effect, locale->client_charset is an
>> appendage that is never used for anything.  It's already been  
>> orphaned
>> so we might as well deprecate it.
>
> That sounds right.
>
>> So the next step is to move the nl_langinfo(CODESET) call out of
>> tds_alloc_locale() and into tds_alloc_connection(), replacing the  
>> hard-
>> wired ISO-8859-1 with what we get by interrogating the locale.
>
> Yes, leaving ISO 8859-1 as a fallback if nl_langinfo/setlocale isn't  
> available.
>
>> But I think that needs more discussion because anyone with a
>> misconfigured locale will suddenly not be able to construct a valid
>> login packet; basically we'll be doing at the library level what tsql
>> has been doing for a long time at the application level, and which
>> generated the problem report that led us down this road in the first
>> place.
>
> You lost me there.  tsql isn't most applications; most applications  
> use one of the client libraries, which *should* call these things in  
> the same order that tsql does.  If tsql is effectively broken, we  
> can fix it.

Currently tsql does its own interrogation of the locale and puts the  
resulting client charset directly in the login structure. I believe  
this trumps a, b, and c above, and this is what makes tsql behave  
differently from the other utility applications.  The patch I proposed  
a couple of weeks ago changes that behavior of tsql so that only if  
the client charset is explicitly specified with -J on the command line  
do we override the usual sequence of selecting it.  So yes, we can fix  
tsql, and in fact we already have :-).

> I must be missing something, because "anyone with a misconfigured  
> locale will suddenly not be able to construct a valid login packet"  
> shouldn't be so, for reasons I'm sure you know full well: anyone  
> with a misconfigured locale can resort to the freetds.conf override  
> (level #2 above).  IOW, once step B is fixed, step C still happens.   
> So, what's the problem again?  :-)

Yes, step C still happens (getting the client charset from  
freetds.conf).  But that requires a non-default setting.  So I just  
want us to be aware that there will likely be an uptick in problem  
reports from people who have a login failure.  They were formerly  
insulated from this by the hard-wired ISO-8859-1, which is a valid  
character set and does allow them to log in, even if they didn't come  
by it honestly (i.e., they got it from the hard-wired default rather  
than from their locale).

> It's great you're working on this.  Would you like to be able to  
> commit these things yourself?  No good deed goes unpunished, you know.

Gulp.  Um, ok.  I guess I will have to hold my nose and learn cvs  
after all these years.