Skip to Content.
Sympa Menu

freetds - RE: [freetds] Small patch

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: "Frediano Ziglio" <freddyz77 AT lycos.com>
  • To: "FreeTDS Development Group" <freetds AT lists.ibiblio.org>
  • Subject: RE: [freetds] Small patch
  • Date: Sat, 15 Nov 2003 01:56:39 -0800


>> From: ZIGLIO Frediano [mailto:Frediano.Ziglio AT vodafone.com]
>> Sent: November 14, 2003 7:41 AM
>>
>> This patch should fix the problem with db-lib.
>>
>> * freetds62/doc/api_status.txt: update
>> * freetds62/src/tds/iconv.c freetds62/src/tds/read.c:
>> - fix problem with iconv
>>
>> In iconv.c *inbuf and *inbytesleft were already updated in
>> skip_one_input_sequence; this caused a E2BIG problem (and
>> also a buffer overflow in iconv !!!)
>
>OK, I can apply that. Thanks for hunting it down!
>

Applied

>> I updates also read_and_convert to discard the rest of string if
>> provided buffer is insufficient. This prevent lost of sync between
>> FreeTDS and server.
>
>I changed read_and_convert() yesterday. Your patch is against the previous
>version. Could you take a look at what I did? I fixed some other problems,
>in addition to changing some names. If you don't have access to current
>CVS, use http://www.schemamania.org/projects/freetds/src/tds/read.c.
>

Very complex... I'll take a look...

>The real question is: Do we want read_and_convert() to discard data? Or
>should the caller re-issue the read when it gets the error? There are three
>errors to consider:
>
>1. E2BIG. IMO, the buffer passed to this low-level routine should *always*
>be big enough. Nothing should be discarded and no error ever returned.
>Just assert() and die. After all, you found the real error, in iconv.c.
>

In current implementation text buffer is insufficient and should be resized
(see utf8_1 test). Also is not so expensive to handle this case. I prefer to
handle strange case (like 40 characters returned for a varchar(1)) due to
man-in-the-middle attacks or other strange events.

>2. tds_iconv() treats EILSEQ correctly for the purpose of
>read_and_convert(). tds_iconv() will generate an error message and convert
>(as best it can) the whole input buffer. read_and_convert() can ignore
>EILSEQ.
>

I think you are right...

>3. EINVAL. Should not happen. read_and_convert() has enough information
>to avoid asking tds_iconv() to convert an incomplete character. If it fails
>to do so, it should assert() and die. We need to correct any such logic
>error.
>

EINVAL can be raised from an invalid input sequence, not only an incomplete
one. In this case we cannot skip character...

>In short, read_and_convert() should be made totally reliable, never
>returning a conversion error. If it doesn't abort, it should return a
>best-effort fully converted buffer, and leave the TDS stream in sync with
>the server.
>
>Communication errors are another matter, but tds_get_n() doesn't return
>those.
>

We should also handle the case of a long string with strange unconvertible
characters. In this case (and current implementation) we issue multiple
warnings due to wrong conversion..

freddy77



____________________________________________________________
Enter now for a chance to win a 42" Plasma Television!
http://ad.doubleclick.net/clk;6413623;3807821;f?http://mocda1.com/1/c/563632/113422/313631/313631
AOL users go here:
http://ad.doubleclick.net/clk;6413623;3807821;f?http://mocda1.com/1/c/563632/113422/313631/313631
This offer applies to U.S. Residents Only




Archive powered by MHonArc 2.6.24.

Top of Page