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;
}
