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 <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 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;
+ }
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Reply via email to