MEDIUM: config: don't check config validity when there are fatal errors
Overall we do have an issue with the severity of a number of errors. Most fatal errors are reported with ERR_FATAL (which prevents startup) and not ERR_ABORT (which stops parsing ASAP), but check_config_validity() is still called on ERR_FATAL, and will most of the time report bogus errors. This is what caused smp_resolve_args() to be called on a number of unparsable ACLs, and it also is what reports incorrect ordering or unresolvable section names when certain entries could not be properly parsed. This patch stops this domino effect by simply aborting before trying to further check and resolve the configuration when it's already know that there are fatal errors. A concrete example comes from this config : userlist users : user foo insecure-password bar listen foo bind :1234 mode htttp timeout client 10S timeout server 10s timeout connect 10s stats uri /stats stats http-request auth unless { http_auth(users) } http-request redirect location /index.html if { path / } It contains a colon after the userlist name, a typo in the client timeout value, another one in "mode http" which cause some other configuration elements not to be properly handled. Previously it would confusingly report : [ALERT] 108/114851 (20224) : parsing [err-report.cfg:1] : 'userlist' cannot handle unexpected argument ':'. [ALERT] 108/114851 (20224) : parsing [err-report.cfg:6] : unknown proxy mode 'htttp'. [ALERT] 108/114851 (20224) : parsing [err-report.cfg:7] : unexpected character 'S' in 'timeout client' [ALERT] 108/114851 (20224) : Error(s) found in configuration file : err-report.cfg [ALERT] 108/114851 (20224) : parsing [err-report.cfg:11] : unable to find userlist 'users' referenced in arg 1 of ACL keyword 'http_auth' in proxy 'foo'. [WARNING] 108/114851 (20224) : config : missing timeouts for proxy 'foo'. | While not properly invalid, you will certainly encounter various problems | with such a configuration. To fix this, please ensure that all following | timeouts are set to a non-zero value: 'client', 'connect', 'server'. [WARNING] 108/114851 (20224) : config : 'stats' statement ignored for proxy 'foo' as it requires HTTP mode. [WARNING] 108/114851 (20224) : config : 'http-request' rules ignored for proxy 'foo' as they require HTTP mode. [ALERT] 108/114851 (20224) : Fatal errors found in configuration. The "requires HTTP mode" errors are just pollution resulting from the improper spelling of this mode earlier. The unresolved reference to the userlist is caused by the extra colon on the declaration, and the warning regarding the missing timeouts is caused by the wrong character. Now it more accurately reports : [ALERT] 108/114900 (20225) : parsing [err-report.cfg:1] : 'userlist' cannot handle unexpected argument ':'. [ALERT] 108/114900 (20225) : parsing [err-report.cfg:6] : unknown proxy mode 'htttp'. [ALERT] 108/114900 (20225) : parsing [err-report.cfg:7] : unexpected character 'S' in 'timeout client' [ALERT] 108/114900 (20225) : Error(s) found in configuration file : err-report.cfg [ALERT] 108/114900 (20225) : Fatal errors found in configuration. Despite not really a fix, this patch should be backported at least to 1.7, possibly even 1.6, and 1.5 since it hardens the config parser against certain bad situations like the recently reported use-after-free and the last null dereference.
This commit is contained in:
parent
bcfe23a7ec
commit
b83dc3d2ef
|
@ -1035,6 +1035,15 @@ static void init(int argc, char **argv)
|
||||||
exit(1);
|
exit(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* do not try to resolve arguments nor to spot inconsistencies when
|
||||||
|
* the configuration contains fatal errors caused by files not found
|
||||||
|
* or failed memory allocations.
|
||||||
|
*/
|
||||||
|
if (err_code & (ERR_ABORT|ERR_FATAL)) {
|
||||||
|
Alert("Fatal errors found in configuration.\n");
|
||||||
|
exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
pattern_finalize_config();
|
pattern_finalize_config();
|
||||||
|
|
||||||
err_code |= check_config_validity();
|
err_code |= check_config_validity();
|
||||||
|
|
Loading…
Reference in New Issue