Thread: pgsql: Fix strftime usage on Win32 when trying to fetch the locale-aware

Log Message:
-----------
Fix strftime usage on Win32 when trying to fetch the locale-aware
parts of a time string so it properly handles different encodings.

Original patch by Hiroshi Saito, heavily reworked by me and
ITAGAKI Takahiro.

Modified Files:
--------------
    pgsql/src/backend/utils/adt:
        pg_locale.c (r1.43 -> r1.44)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/pg_locale.c?r1=1.43&r2=1.44)

mha@postgresql.org (Magnus Hagander) writes:
> Fix strftime usage on Win32 when trying to fetch the locale-aware
> parts of a time string so it properly handles different encodings.

Hmm, this patch has

+     wchar_t    wbuf[MAX_L10N_DATA];
+
+     len = wcsftime(wbuf, sizeof(wbuf), format, tm);

The Single Unix Spec's definition of wcsftime says that the above
risks a buffer overrun, and the correct second argument would be
MAX_L10N_DATA.  Now I realize that SUS is a poor guide for
Windows-specific code, but are you sure this is right?

Also, I believe we've deprecated StrNCpy; use strlcpy if possible.

            regards, tom lane

Tom Lane wrote:
> mha@postgresql.org (Magnus Hagander) writes:
>> Fix strftime usage on Win32 when trying to fetch the locale-aware
>> parts of a time string so it properly handles different encodings.
>
> Hmm, this patch has
>
> +     wchar_t    wbuf[MAX_L10N_DATA];
> +
> +     len = wcsftime(wbuf, sizeof(wbuf), format, tm);
>
> The Single Unix Spec's definition of wcsftime says that the above
> risks a buffer overrun, and the correct second argument would be
> MAX_L10N_DATA.  Now I realize that SUS is a poor guide for
> Windows-specific code, but are you sure this is right?

Now that I read it again, I think you're right. What MS says is:
"If the total number of characters, including the terminating null, is
more than maxsize, both strftime and wcsftime return 0 and the contents
of strDest are indeterminate."

The important difference being "character" vs "bytes", right?


> Also, I believe we've deprecated StrNCpy; use strlcpy if possible.

Ok, I'll change that.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:
> Tom Lane wrote:
>> The Single Unix Spec's definition of wcsftime says that the above
>> risks a buffer overrun, and the correct second argument would be
>> MAX_L10N_DATA.  Now I realize that SUS is a poor guide for
>> Windows-specific code, but are you sure this is right?

> Now that I read it again, I think you're right. What MS says is:
> "If the total number of characters, including the terminating null, is
> more than maxsize, both strftime and wcsftime return 0 and the contents
> of strDest are indeterminate."

> The important difference being "character" vs "bytes", right?

SUS phrases it as

"If the total number of resulting wide-character codes including the
terminating null wide-character code is no more than maxsize, wcsftime()
returns the number of wide-character codes placed into the array pointed
to by wcs, not including the terminating null wide-character
code. Otherwise 0 is returned and the contents of the array are
indeterminate."

so it's very clear that maxsize is counted in wchars.

Perhaps someone could experiment to double-check what Windows does.

            regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Tom Lane wrote:
>>> The Single Unix Spec's definition of wcsftime says that the above
>>> risks a buffer overrun, and the correct second argument would be
>>> MAX_L10N_DATA.  Now I realize that SUS is a poor guide for
>>> Windows-specific code, but are you sure this is right?
>
>> Now that I read it again, I think you're right. What MS says is:
>> "If the total number of characters, including the terminating null, is
>> more than maxsize, both strftime and wcsftime return 0 and the contents
>> of strDest are indeterminate."
>
>> The important difference being "character" vs "bytes", right?
>
> SUS phrases it as
>
> "If the total number of resulting wide-character codes including the
> terminating null wide-character code is no more than maxsize, wcsftime()
> returns the number of wide-character codes placed into the array pointed
> to by wcs, not including the terminating null wide-character
> code. Otherwise 0 is returned and the contents of the array are
> indeterminate."
>
> so it's very clear that maxsize is counted in wchars.
>
> Perhaps someone could experiment to double-check what Windows does.

Read up a bit more and compared, it definitely seems to mean the same
thing. My tests seem to agree as well.

I'll change it to MAX_L10N_DATA and strlcpy.

//Magnus