Re: BUG #1671: Long interval string representation rejected - Mailing list pgsql-bugs
From | Neil Conway |
---|---|
Subject | Re: BUG #1671: Long interval string representation rejected |
Date | |
Msg-id | 4292CD86.20601@samurai.com Whole thread Raw |
In response to | Re: BUG #1671: Long interval string representation rejected (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: BUG #1671: Long interval string representation rejected
|
List | pgsql-bugs |
Tom Lane wrote: > I'm not set on it --- fix it as you suggested. Attached is a patch that implements this. I'm not especially happy about the implementation: defining _two_ local macros (that both doubly-evaluate one of their arguments) is pretty ugly, but I didn't see a cleaner alternative -- suggestions welcome. This patch fixes the stack overrun. Since we now pass the length of the working space into ParseDateTime(), it is also easy to fix Mark's bug: I just increased the size of the interval_in() working buffer to 256 bytes. I didn't touch any of the other input buffer sizes, although that shouldn't be taken as a vote of confidence that they are correct :) At least the consequence of an insufficiently-large buffer is limited to rejecting otherwise valid input, rather than potentially scribbling on the stack. -Neil Index: src/backend/utils/adt/date.c =================================================================== RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/date.c,v retrieving revision 1.108 diff -c -r1.108 date.c *** src/backend/utils/adt/date.c 24 May 2005 02:09:45 -0000 1.108 --- src/backend/utils/adt/date.c 24 May 2005 06:24:54 -0000 *************** *** 65,76 **** int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char lowstr[MAXDATELEN + 1]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tzp); if (dterr != 0) --- 65,74 ---- int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char workbuf[MAXDATELEN + 1]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tzp); if (dterr != 0) *************** *** 894,908 **** int tz; int nf; int dterr; ! char lowstr[MAXDATELEN + 1]; char *field[MAXDATEFIELDS]; int dtype; int ftype[MAXDATEFIELDS]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) --- 892,904 ---- int tz; int nf; int dterr; ! char workbuf[MAXDATELEN + 1]; char *field[MAXDATEFIELDS]; int dtype; int ftype[MAXDATEFIELDS]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) *************** *** 1733,1747 **** int tz; int nf; int dterr; ! char lowstr[MAXDATELEN + 1]; char *field[MAXDATEFIELDS]; int dtype; int ftype[MAXDATEFIELDS]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) --- 1729,1741 ---- int tz; int nf; int dterr; ! char workbuf[MAXDATELEN + 1]; char *field[MAXDATEFIELDS]; int dtype; int ftype[MAXDATEFIELDS]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) Index: src/backend/utils/adt/datetime.c =================================================================== RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/datetime.c,v retrieving revision 1.144 diff -c -r1.144 datetime.c *** src/backend/utils/adt/datetime.c 24 May 2005 02:09:45 -0000 1.144 --- src/backend/utils/adt/datetime.c 24 May 2005 06:40:01 -0000 *************** *** 699,719 **** } } - /* ParseDateTime() * Break string into tokens based on a date/time context. * Returns 0 if successful, DTERR code if bogus input detected. * * timestr - the input string ! * lowstr - workspace for field string storage (must be large enough for ! * a copy of the input string, including trailing null) * field[] - pointers to field strings are returned in this array * ftype[] - field type indicators are returned in this array * maxfields - dimensions of the above two arrays * *numfields - set to the actual number of fields detected * ! * The fields extracted from the input are stored as separate, null-terminated ! * strings in the workspace at lowstr. Any text is converted to lower case. * * Several field types are assigned: * DTK_NUMBER - digits and (possibly) a decimal point --- 699,721 ---- } } /* ParseDateTime() * Break string into tokens based on a date/time context. * Returns 0 if successful, DTERR code if bogus input detected. * * timestr - the input string ! * workbuf - workspace for field string storage. This must be ! * larger than the largest legal input for this datetime type -- ! * some additional space will be needed to NUL terminate fields. ! * buflen - the size of workbuf * field[] - pointers to field strings are returned in this array * ftype[] - field type indicators are returned in this array * maxfields - dimensions of the above two arrays * *numfields - set to the actual number of fields detected * ! * The fields extracted from the input are stored as separate, ! * null-terminated strings in the workspace at workbuf. Any text is ! * converted to lower case. * * Several field types are assigned: * DTK_NUMBER - digits and (possibly) a decimal point *************** *** 729,740 **** * DTK_DATE can hold Posix time zones (GMT-8) */ int ! ParseDateTime(const char *timestr, char *lowstr, char **field, int *ftype, int maxfields, int *numfields) { int nf = 0; const char *cp = timestr; ! char *lp = lowstr; /* outer loop through fields */ while (*cp != '\0') --- 731,765 ---- * DTK_DATE can hold Posix time zones (GMT-8) */ int ! ParseDateTime(const char *timestr, char *workbuf, size_t buflen, char **field, int *ftype, int maxfields, int *numfields) { int nf = 0; const char *cp = timestr; ! char *bufp = workbuf; ! const char *bufend = workbuf + buflen; ! ! /* ! * Append the character at "inptr" to the current position ! * "bufptr", and increment both pointers. Returns an error if ! * there is no space left in bufptr ("end" marks the last element ! * of the buffer). Beware of the double evaluation of "bufptr". ! */ ! #define APPEND_CHAR(bufptr, inptr, end) \ ! do \ ! { \ ! if (((bufptr) + 1) >= (end)) \ ! return DTERR_BAD_FORMAT; \ ! *(bufptr)++ = *(inptr)++; \ ! } while (0); ! ! #define APPEND_LOWER_CHAR(bufptr, inptr, end) \ ! do \ ! { \ ! if (((bufptr) + 1) >= (end)) \ ! return DTERR_BAD_FORMAT; \ ! *(bufptr)++ = pg_tolower((unsigned char) *(inptr)++); \ ! } while (0); /* outer loop through fields */ while (*cp != '\0') *************** *** 749,771 **** /* Record start of current field */ if (nf >= maxfields) return DTERR_BAD_FORMAT; ! field[nf] = lp; /* leading digit? then date or time */ if (isdigit((unsigned char) *cp)) { ! *lp++ = *cp++; while (isdigit((unsigned char) *cp)) ! *lp++ = *cp++; /* time field? */ if (*cp == ':') { ftype[nf] = DTK_TIME; ! *lp++ = *cp++; while (isdigit((unsigned char) *cp) || (*cp == ':') || (*cp == '.')) ! *lp++ = *cp++; } /* date field? allow embedded text month */ else if (*cp == '-' || *cp == '/' || *cp == '.') --- 774,796 ---- /* Record start of current field */ if (nf >= maxfields) return DTERR_BAD_FORMAT; ! field[nf] = bufp; /* leading digit? then date or time */ if (isdigit((unsigned char) *cp)) { ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp)) ! APPEND_CHAR(bufp, cp, bufend); /* time field? */ if (*cp == ':') { ftype[nf] = DTK_TIME; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || (*cp == ':') || (*cp == '.')) ! APPEND_CHAR(bufp, cp, bufend); } /* date field? allow embedded text month */ else if (*cp == '-' || *cp == '/' || *cp == '.') *************** *** 773,785 **** /* save delimiting character to use later */ char delim = *cp; ! *lp++ = *cp++; /* second field is all digits? then no embedded text month */ if (isdigit((unsigned char) *cp)) { ftype[nf] = ((delim == '.') ? DTK_NUMBER : DTK_DATE); while (isdigit((unsigned char) *cp)) ! *lp++ = *cp++; /* * insist that the delimiters match to get a --- 798,810 ---- /* save delimiting character to use later */ char delim = *cp; ! APPEND_CHAR(bufp, cp, bufend); /* second field is all digits? then no embedded text month */ if (isdigit((unsigned char) *cp)) { ftype[nf] = ((delim == '.') ? DTK_NUMBER : DTK_DATE); while (isdigit((unsigned char) *cp)) ! APPEND_CHAR(bufp, cp, bufend); /* * insist that the delimiters match to get a *************** *** 788,803 **** if (*cp == delim) { ftype[nf] = DTK_DATE; ! *lp++ = *cp++; while (isdigit((unsigned char) *cp) || *cp == delim) ! *lp++ = *cp++; } } else { ftype[nf] = DTK_DATE; while (isalnum((unsigned char) *cp) || *cp == delim) ! *lp++ = pg_tolower((unsigned char) *cp++); } } --- 813,828 ---- if (*cp == delim) { ftype[nf] = DTK_DATE; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || *cp == delim) ! APPEND_LOWER_CHAR(bufp, cp, bufend); } } else { ftype[nf] = DTK_DATE; while (isalnum((unsigned char) *cp) || *cp == delim) ! APPEND_LOWER_CHAR(bufp, cp, bufend); } } *************** *** 811,819 **** /* Leading decimal point? Then fractional seconds... */ else if (*cp == '.') { ! *lp++ = *cp++; while (isdigit((unsigned char) *cp)) ! *lp++ = *cp++; ftype[nf] = DTK_NUMBER; } --- 836,844 ---- /* Leading decimal point? Then fractional seconds... */ else if (*cp == '.') { ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp)) ! APPEND_CHAR(bufp, cp, bufend); ftype[nf] = DTK_NUMBER; } *************** *** 825,833 **** else if (isalpha((unsigned char) *cp)) { ftype[nf] = DTK_STRING; ! *lp++ = pg_tolower((unsigned char) *cp++); while (isalpha((unsigned char) *cp)) ! *lp++ = pg_tolower((unsigned char) *cp++); /* * Full date string with leading text month? Could also be a --- 850,858 ---- else if (isalpha((unsigned char) *cp)) { ftype[nf] = DTK_STRING; ! APPEND_LOWER_CHAR(bufp, cp, bufend); while (isalpha((unsigned char) *cp)) ! APPEND_LOWER_CHAR(bufp, cp, bufend); /* * Full date string with leading text month? Could also be a *************** *** 838,852 **** char delim = *cp; ftype[nf] = DTK_DATE; ! *lp++ = *cp++; while (isdigit((unsigned char) *cp) || *cp == delim) ! *lp++ = *cp++; } } /* sign? then special or numeric timezone */ else if (*cp == '+' || *cp == '-') { ! *lp++ = *cp++; /* soak up leading whitespace */ while (isspace((unsigned char) *cp)) cp++; --- 863,877 ---- char delim = *cp; ftype[nf] = DTK_DATE; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || *cp == delim) ! APPEND_CHAR(bufp, cp, bufend); } } /* sign? then special or numeric timezone */ else if (*cp == '+' || *cp == '-') { ! APPEND_CHAR(bufp, cp, bufend); /* soak up leading whitespace */ while (isspace((unsigned char) *cp)) cp++; *************** *** 854,871 **** if (isdigit((unsigned char) *cp)) { ftype[nf] = DTK_TZ; ! *lp++ = *cp++; while (isdigit((unsigned char) *cp) || *cp == ':' || *cp == '.') ! *lp++ = *cp++; } /* special? */ else if (isalpha((unsigned char) *cp)) { ftype[nf] = DTK_SPECIAL; ! *lp++ = pg_tolower((unsigned char) *cp++); while (isalpha((unsigned char) *cp)) ! *lp++ = pg_tolower((unsigned char) *cp++); } /* otherwise something wrong... */ else --- 879,896 ---- if (isdigit((unsigned char) *cp)) { ftype[nf] = DTK_TZ; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || *cp == ':' || *cp == '.') ! APPEND_CHAR(bufp, cp, bufend); } /* special? */ else if (isalpha((unsigned char) *cp)) { ftype[nf] = DTK_SPECIAL; ! APPEND_LOWER_CHAR(bufp, cp, bufend); while (isalpha((unsigned char) *cp)) ! APPEND_LOWER_CHAR(bufp, cp, bufend); } /* otherwise something wrong... */ else *************** *** 882,888 **** return DTERR_BAD_FORMAT; /* force in a delimiter after each field */ ! *lp++ = '\0'; nf++; } --- 907,913 ---- return DTERR_BAD_FORMAT; /* force in a delimiter after each field */ ! *bufp++ = '\0'; nf++; } Index: src/backend/utils/adt/nabstime.c =================================================================== RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/nabstime.c,v retrieving revision 1.131 diff -c -r1.131 nabstime.c *** src/backend/utils/adt/nabstime.c 24 May 2005 02:09:45 -0000 1.131 --- src/backend/utils/adt/nabstime.c 24 May 2005 06:24:54 -0000 *************** *** 306,320 **** *tm = &date; int dterr; char *field[MAXDATEFIELDS]; ! char lowstr[MAXDATELEN + 1]; int dtype; int nf, ftype[MAXDATEFIELDS]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) --- 306,318 ---- *tm = &date; int dterr; char *field[MAXDATEFIELDS]; ! char workbuf[MAXDATELEN + 1]; int dtype; int nf, ftype[MAXDATEFIELDS]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) *************** *** 711,722 **** char *field[MAXDATEFIELDS]; int nf, ftype[MAXDATEFIELDS]; ! char lowstr[MAXDATELEN + 1]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec); if (dterr != 0) --- 709,718 ---- char *field[MAXDATEFIELDS]; int nf, ftype[MAXDATEFIELDS]; ! char workbuf[MAXDATELEN + 1]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec); if (dterr != 0) Index: src/backend/utils/adt/timestamp.c =================================================================== RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/timestamp.c,v retrieving revision 1.123 diff -c -r1.123 timestamp.c *** src/backend/utils/adt/timestamp.c 24 May 2005 02:09:45 -0000 1.123 --- src/backend/utils/adt/timestamp.c 24 May 2005 06:24:54 -0000 *************** *** 77,88 **** int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char lowstr[MAXDATELEN + MAXDATEFIELDS]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) --- 77,86 ---- int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char workbuf[MAXDATELEN + MAXDATEFIELDS]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) *************** *** 317,328 **** int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char lowstr[MAXDATELEN + MAXDATEFIELDS]; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) --- 315,324 ---- int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char workbuf[MAXDATELEN + MAXDATEFIELDS]; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), ! field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz); if (dterr != 0) *************** *** 493,499 **** int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char lowstr[MAXDATELEN + MAXDATEFIELDS]; tm->tm_year = 0; tm->tm_mon = 0; --- 489,495 ---- int dterr; char *field[MAXDATEFIELDS]; int ftype[MAXDATEFIELDS]; ! char workbuf[256]; tm->tm_year = 0; tm->tm_mon = 0; *************** *** 503,512 **** tm->tm_sec = 0; fsec = 0; ! if (strlen(str) >= sizeof(lowstr)) ! dterr = DTERR_BAD_FORMAT; ! else ! dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec); if (dterr != 0) --- 499,506 ---- tm->tm_sec = 0; fsec = 0; ! dterr = ParseDateTime(str, workbuf, sizeof(workbuf), field, ! ftype, MAXDATEFIELDS, &nf); if (dterr == 0) dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec); if (dterr != 0) Index: src/include/utils/datetime.h =================================================================== RCS file: /var/lib/cvs/pgsql/src/include/utils/datetime.h,v retrieving revision 1.53 diff -c -r1.53 datetime.h *** src/include/utils/datetime.h 24 May 2005 04:03:01 -0000 1.53 --- src/include/utils/datetime.h 24 May 2005 06:24:54 -0000 *************** *** 276,282 **** extern void j2date(int jd, int *year, int *month, int *day); extern int date2j(int year, int month, int day); ! extern int ParseDateTime(const char *timestr, char *lowstr, char **field, int *ftype, int maxfields, int *numfields); extern int DecodeDateTime(char **field, int *ftype, --- 276,282 ---- extern void j2date(int jd, int *year, int *month, int *day); extern int date2j(int year, int month, int day); ! extern int ParseDateTime(const char *timestr, char *workbuf, size_t buflen, char **field, int *ftype, int maxfields, int *numfields); extern int DecodeDateTime(char **field, int *ftype, Index: src/test/regress/expected/interval.out =================================================================== RCS file: /var/lib/cvs/pgsql/src/test/regress/expected/interval.out,v retrieving revision 1.10 diff -c -r1.10 interval.out *** src/test/regress/expected/interval.out 7 Apr 2005 01:51:40 -0000 1.10 --- src/test/regress/expected/interval.out 24 May 2005 06:24:54 -0000 *************** *** 221,223 **** --- 221,230 ---- @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs (1 row) + -- test long interval input + select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval; + interval + -------------------------------------------- + @ 4541 years 4 mons 4 days 17 mins 31 secs + (1 row) + Index: src/test/regress/sql/interval.sql =================================================================== RCS file: /var/lib/cvs/pgsql/src/test/regress/sql/interval.sql,v retrieving revision 1.6 diff -c -r1.6 interval.sql *** src/test/regress/sql/interval.sql 7 Apr 2005 01:51:41 -0000 1.6 --- src/test/regress/sql/interval.sql 24 May 2005 06:24:54 -0000 *************** *** 66,68 **** --- 66,71 ---- -- updating pg_aggregate.agginitval select avg(f1) from interval_tbl; + + -- test long interval input + select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval;
pgsql-bugs by date: