handle error messages during building and processing of the configuration.

add missing return statements, wrap some lines, remove unused vars.
move syntax error reporting and exit(1) back to the right place (to be
    fixed in a future pass; the exit() is inappropriate for parsing
    .htaccess files).


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@85023 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 91 insertions, 58 deletions

diff --git a/include/http_config.h b/include/http_config.h
index e2b7f70f61..a72f60a666 100644
--- a/include/http_config.h
+++ b/include/http_config.h
@@ -305,8 +305,9 @@ API_EXPORT(const char *) ap_find_module_name(module *m);
 API_EXPORT(module *) ap_find_linked_module(const char *name);
 
 /* for implementing subconfigs and customized config files */
-API_EXPORT(ap_directive_t *) ap_build_config(cmd_parms *parms, ap_directive_t *current);
-API_EXPORT(const char *)ap_walk_config(ap_directive_t *conftree, cmd_parms *parms, void *config, int container);
+API_EXPORT(const char *) ap_build_config(cmd_parms *parms,
+					 ap_directive_t **conftree);
+API_EXPORT(const char *) ap_walk_config(ap_directive_t *conftree, cmd_parms *parms, void *config, int container);
 
 /* ap_check_cmd_context() definitions: */
 API_EXPORT(const char *) ap_check_cmd_context(cmd_parms *cmd, unsigned forbidden);
diff --git a/modules/http/http_core.c b/modules/http/http_core.c
index 63f55a324b..541658a76c 100644
--- a/modules/http/http_core.c
+++ b/modules/http/http_core.c
@@ -1437,8 +1437,10 @@ static const char *dirsection(cmd_parms *cmd, void *dummy, const char *arg)
     old_end_token = cmd->end_token;
     cmd->end_token = thiscmd->cmd_data ? end_directorymatch_section : end_directory_section;
 
-    ap_walk_config(NULL, cmd, new_dir_conf, 1);
+    errmsg = ap_walk_config(NULL, cmd, new_dir_conf, 1);
     cmd->end_token = old_end_token;
+    if (errmsg != NULL)
+	return errmsg;
 
     conf = (core_dir_config *)ap_get_module_config(new_dir_conf, &core_module);
     conf->r = r;
@@ -1496,8 +1498,10 @@ static const char *urlsection(cmd_parms *cmd, void *dummy, const char *arg)
     cmd->end_token = thiscmd->cmd_data ? end_locationmatch_section
                                        : end_location_section;
 
-    ap_walk_config(NULL, cmd, new_url_conf, 1);
-    cmd->end_token = old_end_token; 
+    errmsg = ap_walk_config(NULL, cmd, new_url_conf, 1);
+    cmd->end_token = old_end_token;
+    if (errmsg != NULL)
+	return errmsg;
 
     conf = (core_dir_config *)ap_get_module_config(new_url_conf, &core_module);
     conf->d = ap_pstrdup(cmd->pool, cmd->path);	/* No mangling, please */
@@ -1563,8 +1567,10 @@ static const char *filesection(cmd_parms *cmd, core_dir_config *c,
     old_end_token = cmd->end_token;
     cmd->end_token = thiscmd->cmd_data ? end_filesmatch_section : end_files_section;
 
-    ap_walk_config(NULL, cmd, new_file_conf, 1);
+    errmsg = ap_walk_config(NULL, cmd, new_file_conf, 1);
     cmd->end_token = old_end_token;
+    if (errmsg != NULL)
+	return errmsg;
 
     conf = (core_dir_config *)ap_get_module_config(new_file_conf,
 						   &core_module);
@@ -1598,10 +1604,8 @@ static const char *end_ifmod(cmd_parms *cmd, void *dummy)
 static const char *start_ifmod(cmd_parms *cmd, void *dummy, char *arg)
 {
     char *endp = strrchr(arg, '>');
-    char l[MAX_STRING_LEN];
     int not = (arg[0] == '!');
     module *found;
-    int nest = 1;
 
     if (endp == NULL) {
 	return unclosed_directive(cmd);
@@ -1616,9 +1620,10 @@ static const char *start_ifmod(cmd_parms *cmd, void *dummy, char *arg)
     found = ap_find_linked_module(arg);
 
     if ((!not && found) || (not && !found)) {
-        ap_walk_config(NULL, cmd, cmd->server->lookup_defaults, 1);
-        return NULL;
+        return ap_walk_config(NULL, cmd, cmd->server->lookup_defaults, 1);
     }
+
+    return NULL;
 }
 
 API_EXPORT(int) ap_exists_config_define(char *name)
@@ -1643,10 +1648,8 @@ static const char *end_ifdefine(cmd_parms *cmd, void *dummy)
 static const char *start_ifdefine(cmd_parms *cmd, void *dummy, char *arg)
 {
     char *endp;
-    char l[MAX_STRING_LEN];
     int defined;
     int not = 0;
-    int nest = 1;
 
     endp = strrchr(arg, '>');
     if (endp == NULL) {
@@ -1663,9 +1666,10 @@ static const char *start_ifdefine(cmd_parms *cmd, void *dummy, char *arg)
     defined = ap_exists_config_define(arg);
 
     if ((!not && defined) || (not && !defined)) {
-        ap_walk_config(NULL, cmd, dummy, 1);
-        return NULL;
+        return ap_walk_config(NULL, cmd, dummy, 1);
     }
+
+    return NULL;
 }
 
 /* httpd.conf commands... beginning with the <VirtualHost> business */
@@ -1713,13 +1717,10 @@ static const char *virtualhost_section(cmd_parms *cmd, void *dummy, char *arg)
     cmd->end_token = end_virtualhost_section;
     cmd->server = s;
 
-    ap_walk_config(NULL, cmd, s->lookup_defaults, 1);
+    errmsg = ap_walk_config(NULL, cmd, s->lookup_defaults, 1);
     cmd->end_token = old_end_token;
     cmd->server = main_server;
 
-    if (errmsg == end_virtualhost_section) {
-	return NULL;
-    }
     return errmsg;
 }
 
diff --git a/server/config.c b/server/config.c
index 0f899bc74d..0d9f2022b0 100644
--- a/server/config.c
+++ b/server/config.c
@@ -831,13 +831,15 @@ CORE_EXPORT(void *) ap_set_config_vectors(cmd_parms *parms, void *config, module
     return mconfig;
 }
 
-CORE_EXPORT(void) ap_build_config_sub(cmd_parms *parms, const char *l, ap_directive_t **current, ap_directive_t **curr_parent)
+static const char * ap_build_config_sub(cmd_parms *parms, const char *l,
+					ap_directive_t **current,
+					ap_directive_t **curr_parent)
 {
     const char *args, *cmd_name;
     ap_directive_t *newdir;
 
     if ((l[0] == '#') || (!l[0]))
-	return;
+	return NULL;
 
 #if RESOLVE_ENV_PER_TOKEN
     args = l;
@@ -846,7 +848,7 @@ CORE_EXPORT(void) ap_build_config_sub(cmd_parms *parms, const char *l, ap_direct
 #endif
     cmd_name = ap_getword_conf(parms->temp_pool, &args);
     if (*cmd_name == '\0')
-	return;
+	return NULL;
 
     newdir = ap_pcalloc(parms->pool, sizeof(ap_directive_t));
     newdir->line_num = parms->config_file->line_number;
@@ -872,9 +874,12 @@ CORE_EXPORT(void) ap_build_config_sub(cmd_parms *parms, const char *l, ap_direct
     else {
         *current = ap_add_node(curr_parent, *current, newdir, 0);
     }
+
+    return NULL;
 }
 
-API_EXPORT(const char *)ap_walk_config_sub(ap_directive_t *current, cmd_parms *parms, void *config)
+static const char *ap_walk_config_sub(ap_directive_t *current,
+				      cmd_parms *parms, void *config)
 {
     void *oldconfig;
     const command_rec *cmd;
@@ -899,17 +904,12 @@ API_EXPORT(const char *)ap_walk_config_sub(ap_directive_t *current, cmd_parms *p
     } while (retval && !strcmp(retval, DECLINE_CMD));
     parms->context = oldconfig;
 
-    if (retval) {
-	ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
-                     "Syntax error on line %d of %s:",
-		     parms->config_file->line_number, parms->config_file->name);
-	ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL, 
-                     "%s", retval);
-	exit(1);
-    }
+    return retval;
 }
 
-API_EXPORT(const char *)ap_walk_config(ap_directive_t *conftree, cmd_parms *parms, void *config, int container)
+API_EXPORT(const char *) ap_walk_config(ap_directive_t *conftree,
+					cmd_parms *parms, void *config,
+					int container)
 {
     static ap_directive_t *current;
 
@@ -921,8 +921,12 @@ API_EXPORT(const char *)ap_walk_config(ap_directive_t *conftree, cmd_parms *parm
     }
 
     while (current != NULL) {
+	const char *errmsg;
+
         /* actually parse the command and execute the correct function */
-        ap_walk_config_sub(current, parms, config);
+        errmsg = ap_walk_config_sub(current, parms, config);
+	if (errmsg != NULL)
+	    return errmsg;
 
         if (current->next == NULL) {
             current = current->parent;
@@ -932,23 +936,33 @@ API_EXPORT(const char *)ap_walk_config(ap_directive_t *conftree, cmd_parms *parm
             continue;
         }
     }
+
+    return NULL;
 }
 
 
-API_EXPORT(ap_directive_t *) ap_build_config(cmd_parms *parms, ap_directive_t *current)
+API_EXPORT(const char *) ap_build_config(cmd_parms *parms,
+					 ap_directive_t **conftree)
 {
+    ap_directive_t *current = NULL;
     ap_directive_t *curr_parent = NULL;
-    ap_directive_t *cfg_root = NULL;
     char l[MAX_STRING_LEN];
 
+    *conftree = NULL;
+
     while (!(ap_cfg_getline(l, MAX_STRING_LEN, parms->config_file))) {
-	ap_build_config_sub(parms, l, &current, &curr_parent);
-        if (cfg_root == NULL && current != NULL) {
-            cfg_root = current;
+	const char *errmsg;
+
+	errmsg = ap_build_config_sub(parms, l, &current, &curr_parent);
+	if (errmsg != NULL)
+	    return errmsg;
+
+        if (*conftree == NULL && current != NULL) {
+            *conftree = current;
         }
     }
 
-    return cfg_root;
+    return NULL;
 }
 
 /*
@@ -1063,6 +1077,7 @@ static void process_command_config(server_rec *s, ap_array_header_t *arr, ap_poo
     const char *errmsg;
     cmd_parms parms;
     arr_elts_param_t arr_parms;
+    ap_directive_t *conftree;
 
     arr_parms.curr_idx = 0;
     arr_parms.array = arr;
@@ -1075,9 +1090,10 @@ static void process_command_config(server_rec *s, ap_array_header_t *arr, ap_poo
     parms.config_file = ap_pcfg_open_custom(p, "-c/-C directives",
                                          &arr_parms, NULL,
                                          arr_elts_getstr, arr_elts_close);
-/*
-    errmsg = ap_build_config(&parms, s->lookup_defaults);
 
+    errmsg = ap_build_config(&parms, &conftree);
+    if (errmsg == NULL)
+	errmsg = ap_walk_config(conftree, &parms, s->lookup_defaults, 0);
     if (errmsg) {
         ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
                      "Syntax error in -C/-c directive:\n%s", errmsg);
@@ -1085,14 +1101,14 @@ static void process_command_config(server_rec *s, ap_array_header_t *arr, ap_poo
     }
 
     ap_cfg_closefile(parms.config_file);
-*/
 }
 
 void ap_process_resource_config(server_rec *s, const char *fname, ap_pool_t *p, ap_pool_t *ptemp)
 {
     cmd_parms parms;
     ap_finfo_t finfo;
-    ap_directive_t *current = NULL;
+    ap_directive_t *conftree;
+    const char *errmsg;
 
     fname = ap_server_root_relative(p, fname);
 
@@ -1118,8 +1134,19 @@ void ap_process_resource_config(server_rec *s, const char *fname, ap_pool_t *p,
 	exit(1);
     }
 
-    current = ap_build_config(&parms, NULL);
-    ap_walk_config(current, &parms, s->lookup_defaults, 0);
+    errmsg = ap_build_config(&parms, &conftree);
+    if (errmsg == NULL)
+	errmsg = ap_walk_config(conftree, &parms, s->lookup_defaults, 0);
+
+    if (errmsg != NULL) {
+	/* ### wrong line number. need to pull from ap_directive_t */
+	ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
+                     "Syntax error on line %d of %s:",
+		     parms.config_file->line_number, parms.config_file->name);
+	ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL, 
+                     "%s", errmsg);
+	exit(1);
+    }
 
     ap_cfg_closefile(parms.config_file);
 }
@@ -1130,13 +1157,11 @@ int ap_parse_htaccess(void **result, request_rec *r, int override,
 {
     configfile_t *f = NULL;
     cmd_parms parms;
-    const char *errmsg = NULL;
     char *filename = NULL;
     const struct htaccess_result *cache;
     struct htaccess_result *new;
     void *dc = NULL;
     ap_status_t status;
-    ap_directive_t *htaccess_tree = NULL;
 
 /* firstly, search cache */
     for (cache = r->htaccess; cache != NULL; cache = cache->next)
@@ -1161,13 +1186,16 @@ int ap_parse_htaccess(void **result, request_rec *r, int override,
         status = ap_pcfg_openfile(&f, r->pool, filename);
 
         if (status == APR_SUCCESS) {
+	    const char *errmsg;
+	    ap_directive_t *conftree;
 
             dc = ap_create_per_dir_config(r->pool);
 
             parms.config_file = f;
 
-            htaccess_tree = ap_build_config(&parms, NULL);
-            ap_walk_config(htaccess_tree, &parms, dc, 0);
+            errmsg = ap_build_config(&parms, &conftree);
+	    if (errmsg == NULL)
+		errmsg = ap_walk_config(conftree, &parms, dc, 0);
 
             ap_cfg_closefile(f);
 
@@ -1178,16 +1206,19 @@ int ap_parse_htaccess(void **result, request_rec *r, int override,
             }
             *result = dc;
             break;
-        }
-        else if (status != APR_ENOENT && status != APR_ENOTDIR) {
-            ap_log_rerror(APLOG_MARK, APLOG_CRIT, errno, r,
-                          "%s pcfg_openfile: unable to check htaccess file, "
-                          "ensure it is readable",
-                          filename);
-            ap_table_setn(r->notes, "error-notes",
-                          "Server unable to read htaccess file, denying "
-                          "access to be safe");
-            return HTTP_FORBIDDEN;
+        } else {
+	    ap_status_t cerr = ap_canonical_error(status);
+
+	    if (cerr != APR_ENOENT && cerr != APR_ENOTDIR) {
+		ap_log_rerror(APLOG_MARK, APLOG_CRIT, status, r,
+			      "%s pcfg_openfile: unable to check htaccess file, "
+			      "ensure it is readable",
+			      filename);
+		ap_table_setn(r->notes, "error-notes",
+			      "Server unable to read htaccess file, denying "
+			      "access to be safe");
+		return HTTP_FORBIDDEN;
+	    }
         }
     }