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

diff --git a/include/http_config.h b/include/http_config.h
index e2b7f70f61deecb9734fd24f9c7fda4f4a929228..a72f60a6669d65ee9499e8529f8b34916b740dd1 100644 (file)
--- 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 63f55a324b22dcc5a9204773a3869d04de103643..541658a76c3a2ca0e699477b7cf52153bffc8c1d 100644 (file)
--- 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 0f899bc74d0f3409b503294fda057d7d04e44dd1..0d9f2022b084ef27a9dffc0384e98783b167d3d0 100644 (file)
--- 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;
+           }
         }
     }