Обсуждение: BUG #16143: PGTYPEStimestamp_fmt_asc() returns the incorrect month when the format specifier %b is used.

The following bug has been logged on the website:

Bug reference:      16143
Logged by:          Paul Spencer
Email address:      paul@intekon.com
PostgreSQL version: 11.5
Operating system:   Redhat and Debian
Description:

PGTYPEStimestamp_fmt_asc() returns the incorrect month when the format
specifier %b is used.  The returned month is one greater then the expected
month.  If the expected month is “Dec”, the application may crash with a
segment fault.  The format specifier %B has a similar issue.

** Investigation Notes
- The month is increased by one at line 143 in timestamp2tm() defined in
timestamp.c

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/interfaces/ecpg/pgtypeslib/timestamp.c;h=810dd06ee68b9e39bfbb8d1fb4b58b8205f24246;hb=HEAD
- The month number is converted to the abbreviation at line 337 in
dttofmtasc_replace() defined in timestamp.c

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/interfaces/ecpg/pgtypeslib/timestamp.c;h=810dd06ee68b9e39bfbb8d1fb4b58b8205f24246;hb=HEAD
- Month abbreviations are defined at line 499 in dt_common.c 


https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/interfaces/ecpg/pgtypeslib/dt_common.c;h=c1a3a3e2cb7e2d4f375a3b1a2e858f7347a867ea;hb=HEAD

***
* Use Case
***
pi@raspberrypi4:~/projects/postgres_month_date $ ./pg_month_date
PostgreSQL timestamp_fmt_asc()
PGTYPEStimestamp_to_asc returns 1999-01-08 04:06:06
Format string = %Y-%m-%d %H:%M:%S, Formated Date = 1999-01-08 04:06:06
Format string = %Y-%b-%d %H:%M:%S, Formated Date = 1999-Feb-08 04:06:06
pi@raspberrypi4:~/projects/postgres_month_date $

***
* Source code for the use case
***
/*
 * pg_month_date.c
 */
#include <stdio.h>
#include <string.h>

#include "pgtypes_timestamp.h"

int main(int argc, char **argv)
{
  char formatString[255] = "";
  char stringBuffer[255] = "";
  timestamp testTimestamp;
  char * endPtr = NULL;

  printf("%s\n","PostgreSQL timestamp_fmt_asc()");

  testTimestamp = PGTYPEStimestamp_from_asc("1999-01-08 04:06:06",
&endPtr);
  printf("PGTYPEStimestamp_to_asc returns %s\n",
PGTYPEStimestamp_to_asc(testTimestamp));
  
  strncpy(formatString, "%Y-%m-%d %H:%M:%S", sizeof(formatString));
  PGTYPEStimestamp_fmt_asc(&testTimestamp, stringBuffer,
sizeof(stringBuffer),formatString);
  printf( "Format string = %s, Formated Date = %s\n", formatString ,
stringBuffer);

  strncpy(formatString, "%Y-%b-%d %H:%M:%S", sizeof(formatString));
  PGTYPEStimestamp_fmt_asc(&testTimestamp, stringBuffer,
sizeof(stringBuffer),formatString);
  printf( "Format string = %s, Formated Date = %s\n", formatString ,
stringBuffer);
  return 0;
} 


Paul Spencer

On Fri, Nov 29, 2019 at 07:40:37PM +0000, PG Bug reporting form wrote:
>The following bug has been logged on the website:
>
>Bug reference:      16143
>Logged by:          Paul Spencer
>Email address:      paul@intekon.com
>PostgreSQL version: 11.5
>Operating system:   Redhat and Debian
>Description:
>
>PGTYPEStimestamp_fmt_asc() returns the incorrect month when the format
>specifier %b is used.  The returned month is one greater then the expected
>month.  If the expected month is “Dec”, the application may crash with a
>segment fault.  The format specifier %B has a similar issue.
>
>** Investigation Notes
>- The month is increased by one at line 143 in timestamp2tm() defined in
>timestamp.c

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/interfaces/ecpg/pgtypeslib/timestamp.c;h=810dd06ee68b9e39bfbb8d1fb4b58b8205f24246;hb=HEAD
>- The month number is converted to the abbreviation at line 337 in
>dttofmtasc_replace() defined in timestamp.c

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/interfaces/ecpg/pgtypeslib/timestamp.c;h=810dd06ee68b9e39bfbb8d1fb4b58b8205f24246;hb=HEAD
>- Month abbreviations are defined at line 499 in dt_common.c 


>https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/interfaces/ecpg/pgtypeslib/dt_common.c;h=c1a3a3e2cb7e2d4f375a3b1a2e858f7347a867ea;hb=HEAD
>

Yeah, seems like a simple off-by-one mistake. Our tm->tm_mon is 1-based,
but dttofmtasc_replace uses it directly to access elements of arrays
with month names. Hence the "next" month is returned, and crash for the
last month (access out of bounds).

The attached patch should fix this, I believe - both for %b and %B.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

PG Bug reporting form <noreply@postgresql.org> writes:
> PGTYPEStimestamp_fmt_asc() returns the incorrect month when the format
> specifier %b is used.

Yeah, you're clearly right.  Defining struct pg_tm's tm_mon differently
from the common understanding of struct tm's tm_mon may not have been
the greatest idea we ever had :-(.  I dug through the uses of tm_mon in
the rest of ecpglib and didn't find any other similar mistakes, though.

Will fix, thanks for the report!

            regards, tom lane

Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> The attached patch should fix this, I believe - both for %b and %B.

Yeah, I'd just come to the same conclusion.  You're welcome to
do the commits, of course.

            regards, tom lane

On Fri, Nov 29, 2019 at 03:43:44PM -0500, Tom Lane wrote:
>Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>> The attached patch should fix this, I believe - both for %b and %B.
>
>Yeah, I'd just come to the same conclusion.  You're welcome to
>do the commits, of course.
>

OK, I'll commit shortly.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 

On Fri, Nov 29, 2019 at 10:36:14PM +0100, Tomas Vondra wrote:
>On Fri, Nov 29, 2019 at 03:43:44PM -0500, Tom Lane wrote:
>>Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>>>The attached patch should fix this, I believe - both for %b and %B.
>>
>>Yeah, I'd just come to the same conclusion.  You're welcome to
>>do the commits, of course.
>>
>
>OK, I'll commit shortly.
>

OK, pushed with backpatch to all supported branches. Turns out this is a
pretty ancient bug - it's there since at least 2003, I stopped digging
after that.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services