I started working on this problem on Tuesday, but was in New York City
yesterday so I was not able to comment on this patch before. I think
the one applied is great. (I added C comments on the use of "errno =
0".
Here is the patch I was working on. It does us a separate libpq
strtol() function, but I question whether it is worth it, or if it is
meaningful when used by FRONTEND applications. Anyway, I am just
throwing it out if it gives others ideas.
---------------------------------------------------------------------------
Michael Fuhr wrote:
> On Wed, Nov 30, 2005 at 02:01:46PM -0500, Tom Lane wrote:
> > Michael Fuhr <mike@fuhr.org> writes:
> > > Any preferences on an approach? The simplest and easiest to verify
> > > would be to raise an error for just this particular case; a TODO
> > > item might be to change how the string is parsed to allow values
> > > larger than LONG_MAX.
> >
> > I think the latter would be a feature enhancement and therefore not
> > good material to back-patch. Just erroring out seems appropriate
> > for now.
>
> Agreed. I'm thinking about rewriting strtol() calls in datetime.c
> to look like this:
>
> errno = 0;
> val = strtol(field[i], &cp, 10);
> if (errno == ERANGE)
> return DTERR_FIELD_OVERFLOW;
>
> Does that look okay? Or would you rather raise an error with ereport()?
>
> > > I see several calls to strtol() that aren't checked for overflow but
> > > that might not be relevant to this problem, so I'm thinking this patch
> > > ought not touch them. Maybe that's another TODO item.
> >
> > If it's possible for them to be given overflowing input, they probably
> > ought to be checked.
>
> I'm looking at all the strtol() calls in datetime.c right now; I
> haven't looked anywhere else yet. Should I bother checking values
> that will be range checked later anyway? Time zone displacements,
> for example?
>
> --
> Michael Fuhr
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/backend/libpq/ip.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/ip.c,v
retrieving revision 1.33
diff -c -c -r1.33 ip.c
*** src/backend/libpq/ip.c 22 Nov 2005 18:17:11 -0000 1.33
--- src/backend/libpq/ip.c 1 Dec 2005 18:40:02 -0000
***************
*** 342,348 ****
long bits;
char *endptr;
! bits = strtol(numbits, &endptr, 10);
if (*numbits == '\0' || *endptr != '\0')
return -1;
--- 342,348 ----
long bits;
char *endptr;
! bits = pg_strtol_range(numbits, &endptr, 10);
if (*numbits == '\0' || *endptr != '\0')
return -1;
Index: src/backend/utils/adt/datetime.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.161
diff -c -c -r1.161 datetime.c
*** src/backend/utils/adt/datetime.c 22 Nov 2005 18:17:22 -0000 1.161
--- src/backend/utils/adt/datetime.c 1 Dec 2005 18:40:03 -0000
***************
*** 1013,1019 ****
if (tzp == NULL)
return DTERR_BAD_FORMAT;
! val = strtol(field[i], &cp, 10);
j2date(val, &tm->tm_year, &tm->tm_mon, &tm->tm_mday);
/* Get the time zone from the end of the string */
--- 1013,1019 ----
if (tzp == NULL)
return DTERR_BAD_FORMAT;
! val = pg_strtol_range(field[i], &cp, 10);
j2date(val, &tm->tm_year, &tm->tm_mon, &tm->tm_mday);
/* Get the time zone from the end of the string */
***************
*** 1158,1164 ****
char *cp;
int val;
! val = strtol(field[i], &cp, 10);
/*
* only a few kinds are allowed to have an embedded
--- 1158,1164 ----
char *cp;
int val;
! val = pg_strtol_range(field[i], &cp, 10);
/*
* only a few kinds are allowed to have an embedded
***************
*** 1915,1921 ****
break;
}
! val = strtol(field[i], &cp, 10);
/*
* only a few kinds are allowed to have an embedded
--- 1915,1921 ----
break;
}
! val = pg_strtol_range(field[i], &cp, 10);
/*
* only a few kinds are allowed to have an embedded
***************
*** 2456,2466 ****
*tmask = DTK_TIME_M;
! tm->tm_hour = strtol(str, &cp, 10);
if (*cp != ':')
return DTERR_BAD_FORMAT;
str = cp + 1;
! tm->tm_min = strtol(str, &cp, 10);
if (*cp == '\0')
{
tm->tm_sec = 0;
--- 2456,2466 ----
*tmask = DTK_TIME_M;
! tm->tm_hour = pg_strtol_range(str, &cp, 10);
if (*cp != ':')
return DTERR_BAD_FORMAT;
str = cp + 1;
! tm->tm_min = pg_strtol_range(str, &cp, 10);
if (*cp == '\0')
{
tm->tm_sec = 0;
***************
*** 2471,2477 ****
else
{
str = cp + 1;
! tm->tm_sec = strtol(str, &cp, 10);
if (*cp == '\0')
*fsec = 0;
else if (*cp == '.')
--- 2471,2477 ----
else
{
str = cp + 1;
! tm->tm_sec = pg_strtol_range(str, &cp, 10);
if (*cp == '\0')
*fsec = 0;
else if (*cp == '.')
***************
*** 2522,2528 ****
*tmask = 0;
! val = strtol(str, &cp, 10);
if (cp == str)
return DTERR_BAD_FORMAT;
--- 2522,2528 ----
*tmask = 0;
! val = pg_strtol_range(str, &cp, 10);
if (cp == str)
return DTERR_BAD_FORMAT;
***************
*** 2809,2819 ****
if (*str != '+' && *str != '-')
return DTERR_BAD_FORMAT;
! hr = strtol(str + 1, &cp, 10);
/* explicit delimiter? */
if (*cp == ':')
! min = strtol(cp + 1, &cp, 10);
/* otherwise, might have run things together... */
else if (*cp == '\0' && strlen(str) > 3)
{
--- 2809,2819 ----
if (*str != '+' && *str != '-')
return DTERR_BAD_FORMAT;
! hr = pg_strtol_range(str + 1, &cp, 10);
/* explicit delimiter? */
if (*cp == ':')
! min = pg_strtol_range(cp + 1, &cp, 10);
/* otherwise, might have run things together... */
else if (*cp == '\0' && strlen(str) > 3)
{
***************
*** 3056,3062 ****
case DTK_DATE:
case DTK_NUMBER:
! val = strtol(field[i], &cp, 10);
if (type == IGNORE_DTF)
type = DTK_SECOND;
--- 3056,3062 ----
case DTK_DATE:
case DTK_NUMBER:
! val = pg_strtol_range(field[i], &cp, 10);
if (type == IGNORE_DTF)
type = DTK_SECOND;
Index: src/backend/utils/adt/numeric.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.88
diff -c -c -r1.88 numeric.c
*** src/backend/utils/adt/numeric.c 22 Nov 2005 18:17:23 -0000 1.88
--- src/backend/utils/adt/numeric.c 1 Dec 2005 18:40:08 -0000
***************
*** 2812,2818 ****
char *endptr;
cp++;
! exponent = strtol(cp, &endptr, 10);
if (endptr == cp)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
--- 2812,2818 ----
char *endptr;
cp++;
! exponent = pg_strtol_range(cp, &endptr, 10);
if (endptr == cp)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
Index: src/include/port.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/port.h,v
retrieving revision 1.84
diff -c -c -r1.84 port.h
*** src/include/port.h 15 Oct 2005 02:49:41 -0000 1.84
--- src/include/port.h 1 Dec 2005 18:40:09 -0000
***************
*** 249,254 ****
--- 249,257 ----
#define closesocket close
#endif /* WIN32 */
+ /* do strtol(), but error out on ERANGE */
+ long pg_strtol_range(const char *nptr, char **endptr, int base);
+
/*
* Default "extern" declarations or macro substitutes for library routines.
* When necessary, these routines are provided by files in src/port/.
Index: src/interfaces/ecpg/pgtypeslib/numeric.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/ecpg/pgtypeslib/numeric.c,v
retrieving revision 1.24
diff -c -c -r1.24 numeric.c
*** src/interfaces/ecpg/pgtypeslib/numeric.c 22 Nov 2005 18:17:32 -0000 1.24
--- src/interfaces/ecpg/pgtypeslib/numeric.c 1 Dec 2005 18:40:11 -0000
***************
*** 218,224 ****
char *endptr;
(*ptr)++;
! exponent = strtol(*ptr, &endptr, 10);
if (endptr == (*ptr))
{
errno = PGTYPES_NUM_BAD_NUMERIC;
--- 218,224 ----
char *endptr;
(*ptr)++;
! exponent = pg_strtol_range(*ptr, &endptr, 10);
if (endptr == (*ptr))
{
errno = PGTYPES_NUM_BAD_NUMERIC;
Index: src/port/exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/exec.c,v
retrieving revision 1.40
diff -c -c -r1.40 exec.c
*** src/port/exec.c 22 Nov 2005 18:17:34 -0000 1.40
--- src/port/exec.c 1 Dec 2005 18:40:11 -0000
***************
*** 592,594 ****
--- 592,618 ----
return -1;
}
+
+
+ /*
+ * This really should be in its own file, but it so small, it hardly
+ * seems worth it.
+ */
+ long
+ pg_strtol_range(const char *nptr, char **endptr, int base)
+ {
+ long ret = strtol(nptr, endptr, base);
+
+ if (errno == ERANGE)
+ {
+ #ifndef FRONTEND
+ elog(ERROR, _("value not in supported range for an integer: \"%s\""), nptr);
+ #else
+ /* Keep strings identical for ease of translation */
+ fprintf(stderr, _("value not in supported range for an integer: \"%s\""), nptr);
+ fputc('\n', stderr);
+ #endif
+ return LONG_MAX;
+ }
+ return ret;
+ }
