httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dgau...@hyperreal.org
Subject cvs commit: apachen/src/modules/standard mod_imap.c
Date Wed, 07 Jan 1998 22:23:35 GMT
dgaudet     98/01/07 14:23:34

  Modified:    src/modules/standard mod_imap.c
  Log:
  This is a bit large, but that's deliberate because I took the opportunity
  to do the crap that we've been wanting done to mod_imap.
  
  - liberal use of const to help find stack assignments
  
  - remove all constant sized char arrays except input[]; replaced by pool
      string functions or by pointers into tokens inside the input[]
      array
  
  - in particular, the use of read_quoted() had a stack overrun potential.
      Eliminated.
  
  - These changes can chew memory when generating a menu.  I don't care,
      I'd rather have them do that than have them overrun the stack.  It
      shouldn't chew more than approx the size of the map file though.
  
  - better error handling
  
  Reviewed by:	Jim Jagielski, Martin Kraemer
  
  Revision  Changes    Path
  1.37      +165 -153  apachen/src/modules/standard/mod_imap.c
  
  Index: mod_imap.c
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/modules/standard/mod_imap.c,v
  retrieving revision 1.36
  retrieving revision 1.37
  diff -u -r1.36 -r1.37
  --- mod_imap.c	1998/01/07 16:46:50	1.36
  +++ mod_imap.c	1998/01/07 22:23:33	1.37
  @@ -97,8 +97,6 @@
   #include "util_script.h"
   
   #define IMAP_MAGIC_TYPE "application/x-httpd-imap"
  -#define LARGEBUF 500
  -#define SMALLBUF 256
   #define MAXVERTS 100
   #define X 0
   #define Y 1
  @@ -159,7 +157,7 @@
       {NULL}
   };
   
  -static int pointinrect(double point[2], double coords[MAXVERTS][2])
  +static int pointinrect(const double point[2], const double coords[MAXVERTS][2])
   {
       double max[2], min[2];
       if (coords[0][X] > coords[1][X]) {
  @@ -184,7 +182,7 @@
               (point[Y] >= min[1] && point[Y] <= max[1]));
   }
   
  -static int pointincircle(double point[2], double coords[MAXVERTS][2])
  +static int pointincircle(const double point[2], const double coords[MAXVERTS][2])
   {
       double radius1, radius2;
   
  @@ -197,11 +195,12 @@
       return (radius2 <= radius1);
   }
   
  -static int pointinpoly(double point[2], double pgon[MAXVERTS][2])
  +static int pointinpoly(const double point[2], const double pgon[MAXVERTS][2])
   {
       int i, numverts, inside_flag, xflag0;
       int crossings;
  -    double *p, *stop;
  +    double *p;
  +    const double *stop;
       double tx, ty, y;
   
       for (i = 0; pgon[i][X] != -1 && i < MAXVERTS; i++);
  @@ -271,7 +270,7 @@
   }
   
   
  -static int is_closer(double point[2], double coords[MAXVERTS][2], double *closest)
  +static int is_closer(const double point[2], const double coords[MAXVERTS][2], double *closest)
   {
       double dist_squared = ((point[X] - coords[0][X]) * (point[X] - coords[0][X]))
       + ((point[Y] - coords[0][Y]) * (point[Y] - coords[0][Y]));
  @@ -289,7 +288,7 @@
   
   }
   
  -static double get_x_coord(char *args)
  +static double get_x_coord(const char *args)
   {
       char *endptr;               /* we want it non-null */
       double x_coord = -1;        /* -1 is returned if no coordinate is given */
  @@ -308,7 +307,7 @@
       return (-1);                /* else if no conversion was made, or if no args was given
*/
   }
   
  -static double get_y_coord(char *args)
  +static double get_y_coord(const char *args)
   {
       char *endptr;               /* we want it non-null */
       char *start_of_y = NULL;
  @@ -336,107 +335,98 @@
   }
   
   
  -static int read_quoted(char *string, char *quoted_part)
  +/* See if string has a "quoted part", and if so set *quoted_part to
  + * the first character of the quoted part, then hammer a \0 onto the
  + * trailing quote, and set *string to point at the first character
  + * past the second quote.
  + *
  + * Otherwise set *quoted_part to NULL, and leave *string alone.
  + */
  +static void read_quoted(char **string, char **quoted_part)
   {
  -    char *starting_pos = string;
  +    char *strp = *string;
   
  -    while (isspace(*string))
  -        string++;               /* go along string until non-whitespace */
  +    /* assume there's no quoted part */
  +    *quoted_part = NULL;
   
  -    if (*string == '"') {       /* if that character is a double quote */
  +    while (isspace(*strp))
  +        strp++;               	/* go along string until non-whitespace */
   
  -        string++;               /* step over it */
  +    if (*strp == '"') {       	/* if that character is a double quote */
  +        strp++;               	/* step over it */
  +	*quoted_part = strp;  	/* note where the quoted part begins */
   
  -        while (*string && *string != '"') {
  -            *quoted_part++ = *string++;         /* copy the quoted portion */
  +        while (*strp && *strp != '"') {
  +	    ++strp;		/* skip the quoted portion */
           }
   
  -        *quoted_part = '\0';    /* end the string with a SNUL */
  +        *strp = '\0';    	/* end the string with a NUL */
   
  -        string++;               /* step over the last double quote */
  +        strp++;               	/* step over the last double quote */
  +	*string = strp;
       }
  -
  -    return (string - starting_pos);     /* return the total characters read */
   }
   
   /*
  - * url needs to point to a string with at least SMALLBUF memory allocated
  + * returns the mapped URL or NULL.
    */
  -static void imap_url(request_rec *r, char *base, char *value, char *url)
  +static char *imap_url(request_rec *r, const char *base, const char *value)
   {
   /* translates a value into a URL. */
       int slen, clen;
       char *string_pos = NULL;
  +    const char *string_pos_const = NULL;
       char *directory = NULL;
       char *referer = NULL;
  -    char my_base[SMALLBUF] =
  -    {'\0'};
  +    char *my_base;
   
       if (!strcasecmp(value, "map") || !strcasecmp(value, "menu")) {
  -        if (r->server->port == DEFAULT_PORT) {
  -            ap_snprintf(url, SMALLBUF,
  -                        "http://%s%s", r->server->server_hostname, r->uri);
  -        }
  -        else {
  -            ap_snprintf(url, SMALLBUF, "http://%s:%d%s", r->server->server_hostname,
  -                        r->server->port, r->uri);
  -        }
  -        return;
  +	return construct_url(r->pool, r->uri, r->server);
       }
   
       if (!strcasecmp(value, "nocontent") || !strcasecmp(value, "error")) {
  -        strncpy(url, value, SMALLBUF - 1);
  -        url[SMALLBUF - 1] = '\0';
  -        return;                 /* these are handled elsewhere, so just copy them */
  +        return pstrdup(r->pool, value);                 /* these are handled elsewhere,
so just copy them */
       }
   
       if (!strcasecmp(value, "referer")) {
           referer = table_get(r->headers_in, "Referer");
           if (referer && *referer) {
  -            strncpy(url, referer, SMALLBUF - 1);
  -            url[SMALLBUF - 1] = '\0';
  -            return;
  +	    return pstrdup(r->pool, referer);
           }
           else {
  -            *value = '\0';      /* if 'referer' but no referring page, null the value */
  +	    /* XXX:  This used to do *value = '\0'; ... which is totally bogus
  +	     * because it hammers the passed in value, which can be a string constant,
  +	     * or part of a config, or whatever.  Total garbage.  This works around
  +	     * that without changing the rest of this code much
  +	     */
  +            value = "";      /* if 'referer' but no referring page, null the value */
           }
       }
   
  -    string_pos = value;
  -    while (isalpha(*string_pos))
  -        string_pos++;           /* go along the URL from the map until a non-letter */
  -    if (*string_pos == ':') {
  -        strncpy(url, value, SMALLBUF - 1);      /* if letters and then a colon (like http:)
*/
  -        url[SMALLBUF - 1] = '\0';
  -        return;                 /* it's an absolute URL, so use it! */
  +    string_pos_const = value;
  +    while (isalpha(*string_pos_const))
  +	string_pos_const++;           /* go along the URL from the map until a non-letter */
  +    if (*string_pos_const == ':') {
  +	/* if letters and then a colon (like http:) */
  +	/* it's an absolute URL, so use it! */
  +	return pstrdup(r->pool, value);
       }
   
       if (!base || !*base) {
           if (value && *value) {
  -            strncpy(url, value, SMALLBUF - 1);  /* no base: use what is given */
  -            url[SMALLBUF - 1] = '\0';
  +	    return pstrdup(r->pool, value); /* no base: use what is given */
           }
  -        else {
  -            if (r->server->port == DEFAULT_PORT) {
  -                ap_snprintf(url, SMALLBUF, "http://%s/", r->server->server_hostname);
  -            }
  -            if (r->server->port != DEFAULT_PORT) {
  -                ap_snprintf(url, SMALLBUF, "http://%s:%d/",
  -                            r->server->server_hostname, r->server->port);
  -            }                   /* no base, no value: pick a simple default */
  -        }
  -        return;
  +	/* no base, no value: pick a simple default */
  +	return construct_url(r->pool, "/", r->server);
       }
   
       /* must be a relative URL to be combined with base */
  -    strncpy(my_base, base, sizeof(my_base) - 1);
  -    my_base[sizeof(my_base) - 1] = '\0';
  -    if (strchr(my_base, '/') == NULL && (!strncmp(value, "../", 3) || !strcmp(value,
".."))) {
  -        url[0] = '\0';
  +    if (strchr(base, '/') == NULL && (!strncmp(value, "../", 3) || !strcmp(value,
".."))) {
           aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r->server,
                       "invalid base directive in map file: %s", r->uri);
  -        return;
  +        return NULL;
       }
  +    my_base = pstrdup(r->pool, base);
       string_pos = my_base;
       while (*string_pos) {
           if (*string_pos == '/' && *(string_pos + 1) == '/') {
  @@ -486,10 +476,9 @@
               value += 2;         /* jump over the '..' that we found in the value */
           }
           else if (directory) {
  -            url[0] = '\0';
               aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r->server,
                           "invalid directory name in map file: %s", r->uri);
  -            return;
  +            return NULL;
           }
   
           if (!strncmp(value, "/../", 4) || !strcmp(value, "/.."))
  @@ -500,12 +489,9 @@
       }                           /* by this point, value does not start with '..' */
   
       if (value && *value) {
  -        ap_snprintf(url, SMALLBUF, "%s%s", my_base, value);
  -    }
  -    else {
  -        ap_snprintf(url, SMALLBUF, "%s", my_base);
  +	return pstrcat(r->pool, my_base, value, NULL);
       }
  -    return;
  +    return my_base;
   }
   
   static int imap_reply(request_rec *r, char *redirect)
  @@ -613,46 +599,29 @@
   
   static int imap_handler(request_rec *r)
   {
  -    char input[LARGEBUF] =
  -    {'\0'};
  -    /* size of input can not be lowered without changing hard-coded
  -     * checks
  -     */
  -    char href_text[SMALLBUF] =
  -    {'\0'};
  -    char base[SMALLBUF] =
  -    {'\0'};
  -    char redirect[SMALLBUF] =
  -    {'\0'};
  -    char directive[SMALLBUF] =
  -    {'\0'};
  -    char value[SMALLBUF] =
  -    {'\0'};
  -    char mapdflt[SMALLBUF] =
  -    {'\0'};
  -    char closest[SMALLBUF] =
  -    {'\0'};
  +    char input[MAX_STRING_LEN];
  +    char *directive;
  +    char *value;
  +    char *href_text;
  +    char *base;
  +    char *redirect;
  +    char *mapdflt;
  +    char *closest = NULL;
       double closest_yet = -1;
   
  -    double testpoint[2] =
  -    {-1, -1};
  -    double pointarray[MAXVERTS + 1][2] =
  -    {
  -        {-1, -1}};
  -    int vertex = 0;
  +    double testpoint[2];
  +    double pointarray[MAXVERTS + 1][2];
  +    int vertex;
   
  -    char *string_pos = NULL;
  -    int chars_read = 0;
  +    char *string_pos;
       int showmenu = 0;
   
       imap_conf_rec *icr = get_module_config(r->per_dir_config, &imap_module);
   
  -    char *imap_menu = icr->imap_menu ?
  -    icr->imap_menu : IMAP_MENU_DEFAULT;
  -    char *imap_default = icr->imap_default ?
  -    icr->imap_default : IMAP_DEFAULT_DEFAULT;
  -    char *imap_base = icr->imap_base ?
  -    icr->imap_base : IMAP_BASE_DEFAULT;
  +    char *imap_menu = icr->imap_menu ? icr->imap_menu : IMAP_MENU_DEFAULT;
  +    char *imap_default = icr->imap_default
  +			    ?  icr->imap_default : IMAP_DEFAULT_DEFAULT;
  +    char *imap_base = icr->imap_base ? icr->imap_base : IMAP_BASE_DEFAULT;
   
       configfile_t *imap; 
   
  @@ -664,8 +633,12 @@
       if (!imap)
           return NOT_FOUND;
   
  -    imap_url(r, NULL, imap_base, base);         /* set base according to default */
  -    imap_url(r, NULL, imap_default, mapdflt);   /* and default to global default */
  +    base = imap_url(r, NULL, imap_base);         /* set base according to default */
  +    if (!base)
  +	return HTTP_INTERNAL_SERVER_ERROR;
  +    mapdflt = imap_url(r, NULL, imap_default);   /* and default to global default */
  +    if (!mapdflt)
  +	return HTTP_INTERNAL_SERVER_ERROR;
   
       testpoint[X] = get_x_coord(r->args);
       testpoint[Y] = get_y_coord(r->args);
  @@ -684,15 +657,7 @@
           menu_header(r, imap_menu);
       }
   
  -    while (!cfg_getline(input, LARGEBUF, imap)) {
  -        string_pos = input;     /* always start at the beginning of line */
  -
  -        directive[0] = '\0';
  -        value[0] = '\0';
  -        href_text[0] = '\0';
  -        redirect[0] = '\0';
  -        chars_read = 0;         /* clear these before using */
  -
  +    while (!cfg_getline(input, sizeof(input), imap)) {
           if (!input[0]) {
               if (showmenu) {
                   menu_blank(r, imap_menu);
  @@ -707,35 +672,56 @@
               continue;
           }                       /* blank lines and comments are ignored if we aren't printing
a menu */
   
  -
  -        if (sscanf(input, "%255s %255s", directive, value) != 2) {
  -            continue;           /* make sure we read two fields */
  -        }
  -        /* Now skip what we just read... we can't use ANSIism %n */
  -        while (!(isspace(*string_pos)))         /* past directive */
  -            string_pos++;
  -        while (isspace(*string_pos))    /* and whitespace */
  -            string_pos++;
  -        while (!(isspace(*string_pos)))         /* and value... have to watch it */
  -            string_pos++;       /* can have punctuation and stuff */
  +	/* find the first two space delimited fields, recall that
  +	 * cfg_getline has removed leading/trailing whitespace and
  +	 * compressed the other whitespace down to one space a piece
  +	 *
  +	 * note that we're tokenizing as we go... if we were to use the
  +	 * getword() class of functions we would end up allocating extra
  +	 * memory for every line of the map file
  +	 */
  +        string_pos = input;
  +	if (!*string_pos)		/* need at least two fields */
  +	    goto need_2_fields;
  +
  +	directive = string_pos;
  +	while (*string_pos && *string_pos != ' ')	/* past directive */
  +	    ++string_pos;
  +	if (!*string_pos)		/* need at least two fields */
  +	    goto need_2_fields;
  +	*string_pos++ = '\0';
  +
  +	if (!*string_pos)		/* need at least two fields */
  +	    goto need_2_fields;
  +	value = string_pos;
  +	while (*string_pos && *string_pos != ' ')	/* past value */
  +	    ++string_pos;
  +	if (*string_pos == ' ') {
  +	    *string_pos++ = '\0';
  +	}
  +	else {
  +	    /* end of input, don't advance past it */
  +	    *string_pos = '\0';
  +	}
   
           if (!strncasecmp(directive, "base", 4)) {       /* base, base_uri */
  -            imap_url(r, NULL, value, base);
  +            base = imap_url(r, NULL, value);
  +	    if (!base)
  +		goto menu_bail;
               continue;           /* base is never printed to a menu */
           }
   
  -        chars_read = read_quoted(string_pos, href_text);
  -        string_pos += chars_read;       /* read the quoted href text if present */
  +        read_quoted(&string_pos, &href_text);
   
           if (!strcasecmp(directive, "default")) {        /* default */
  -            imap_url(r, NULL, value, mapdflt);
  +            mapdflt = imap_url(r, NULL, value);
  +	    if (!mapdflt)
  +		goto menu_bail;
               if (showmenu) {     /* print the default if there's a menu */
  -                if (!*href_text) {      /* if we didn't find a "href text" */
  -                    strncpy(href_text, mapdflt, sizeof(href_text) - 1);         /* use
the href itself as text */
  -                    href_text[sizeof(href_text) - 1] = '\0';
  -                }
  -                imap_url(r, base, mapdflt, redirect);
  -                menu_default(r, imap_menu, redirect, href_text);
  +                redirect = imap_url(r, base, mapdflt);
  +		if (!redirect)
  +		    goto menu_bail;
  +                menu_default(r, imap_menu, redirect, href_text ? href_text : mapdflt);
               }
               continue;
           }
  @@ -762,13 +748,13 @@
           pointarray[vertex][X] = -1;     /* signals the end of vertices */
   
           if (showmenu) {
  -            read_quoted(string_pos, href_text);         /* href text could be here instead
*/
  -            if (!*href_text) {  /* if we didn't find a "href text" */
  -                strncpy(href_text, value, sizeof(href_text) - 1);       /* use the href
itself in the menu */
  -                href_text[sizeof(href_text) - 1] = '\0';
  -            }
  -            imap_url(r, base, value, redirect);
  -            menu_directive(r, imap_menu, redirect, href_text);
  +	    if (!href_text) {
  +		read_quoted(&string_pos, &href_text);         /* href text could be here instead
*/
  +	    }
  +            redirect = imap_url(r, base, value);
  +	    if (!redirect)
  +		goto menu_bail;
  +            menu_directive(r, imap_menu, redirect, href_text ? href_text : value);
               continue;
           }
           /* note that we don't make it past here if we are making a menu */
  @@ -781,7 +767,9 @@
   
               if (pointinpoly(testpoint, pointarray)) {
   		cfg_closefile(imap);
  -                imap_url(r, base, value, redirect);
  +                redirect = imap_url(r, base, value);
  +		if (!redirect)
  +		    return HTTP_INTERNAL_SERVER_ERROR;
                   return (imap_reply(r, redirect));
               }
               continue;
  @@ -791,7 +779,9 @@
   
               if (pointincircle(testpoint, pointarray)) {
   		cfg_closefile(imap);
  -                imap_url(r, base, value, redirect);
  +                redirect = imap_url(r, base, value);
  +		if (!redirect)
  +		    return HTTP_INTERNAL_SERVER_ERROR;
                   return (imap_reply(r, redirect));
               }
               continue;
  @@ -801,7 +791,9 @@
   
               if (pointinrect(testpoint, pointarray)) {
   		cfg_closefile(imap);
  -                imap_url(r, base, value, redirect);
  +                redirect = imap_url(r, base, value);
  +		if (!redirect)
  +		    return HTTP_INTERNAL_SERVER_ERROR;
                   return (imap_reply(r, redirect));
               }
               continue;
  @@ -810,8 +802,7 @@
           if (!strcasecmp(directive, "point")) {  /* point */
   
               if (is_closer(testpoint, pointarray, &closest_yet)) {
  -                strncpy(closest, value, sizeof(closest) - 1);   /* if the closest point
yet save it */
  -                closest[sizeof(closest) - 1] = '\0';
  +		closest = pstrdup(r->pool, value);
               }
   
               continue;
  @@ -826,17 +817,38 @@
           return OK;
       }
   
  -    if (*closest) {             /* if a 'point' directive has been seen */
  -        imap_url(r, base, closest, redirect);
  +    if (closest) {             /* if a 'point' directive has been seen */
  +        redirect = imap_url(r, base, closest);
  +	if (!redirect)
  +	    return HTTP_INTERNAL_SERVER_ERROR;
           return (imap_reply(r, redirect));
       }
   
  -    if (*mapdflt) {             /* a default should be defined, even if only 'nocontent'
*/
  -        imap_url(r, base, mapdflt, redirect);
  +    if (mapdflt) {             /* a default should be defined, even if only 'nocontent'
*/
  +        redirect = imap_url(r, base, mapdflt);
  +	if (!redirect)
  +	    return HTTP_INTERNAL_SERVER_ERROR;
           return (imap_reply(r, redirect));
       }
   
       return SERVER_ERROR;        /* If we make it this far, we failed. They lose! */
  +
  +need_2_fields:
  +    aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r->server,
  +		"map file %s, line %d syntax error: requires at least two fields",
  +		r->uri, imap->line_number);
  +    /* fall through */
  +menu_bail:
  +    cfg_closefile(imap);
  +    if (showmenu) {
  +	/* There's not much else we can do ... we've already sent the headers
  +	 * to the client.
  +	 */
  +	rputs("\n\n[an internal server error occured]\n", r);
  +	menu_footer(r);
  +	return OK;
  +    }
  +    return HTTP_INTERNAL_SERVER_ERROR;
   }
   
   
  
  
  

Mime
View raw message