There must be cases were we want to Optionally include configuration files. IncludeOptional check for the folder to exists and then parses the configuration files found in it (if any) but return an error if the directory does not exists yet. After apr_dir_open() fails in process_resource_config_fnmatch and process_resource_config_nofnmatch, we can check for the "optional" arg and return error only if not optional.
Created attachment 32472 [details] Dont error when IncludeOptional refers to a non-existent directory
Created attachment 35467 [details] httpd-trunk-core_include_optional_fix.patch Hi Alberto, I have another version of the patch since I've tested it and stuff like the following seems not to be skipped: IncludeOptional notexisstent Still need to test it carefully but your review would surely help! Luca
So I just noticed that https://httpd.apache.org/docs/2.4/mod/core.html#includeoptional says: "This directive allows inclusion of other configuration files from within the server configuration files. It works identically to the Include directive, with the exception that if wildcards do not match any file or directory, the IncludeOptional directive will be silently ignored instead of causing an error." I was brought to this task since there is an outstanding bug in Debian for mod_security in stretch: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=878920 Basically the maintainer of libapache2-mod_security added the following: # Include OWASP ModSecurity CRS rules if installed IncludeOptional /usr/share/modsecurity-crs/owasp-crs.load This causes an error but something like the following does not: # Include OWASP ModSecurity CRS rules if installed IncludeOptional /usr/share/modsecurity-crs/owasp-crs*.load I can see the value of this difference but is there any strong motivation to error out on the former and not on the latter?
My patch has been working for almost 3 years in clearlinux https://github.com/clearlinux-pkgs/httpd/blob/master/0002-do-not-crash-when-IncludeOptional-dir-is-not-existent.patch I was new at the project so I just wrote and forget about it (my bad) I guess is time to chase the apache developers to review and include it in master. I currently do not know how to contact them (mailing list I guess) if you find the way CC me :)
(In reply to Alberto Murillo Silva from comment #4) > My patch has been working for almost 3 years in clearlinux > https://github.com/clearlinux-pkgs/httpd/blob/master/0002-do-not-crash-when- > IncludeOptional-dir-is-not-existent.patch > > I was new at the project so I just wrote and forget about it (my bad) I > guess is time to chase the apache developers to review and include it in > master. > > I currently do not know how to contact them (mailing list I guess) if you > find the way CC me :) What are the use cases that you tested to make sure that it was working? I am trying to understand an example that I can test and verify, not trying to discard your work :) You can surely contact the dev using the https://httpd.apache.org/lists.html#http-dev mailing list, but I guess that bugzilla is fine since some of them are reading it daily (like me :). The patch that I proposed work even if you include a non existent file, but it might be a big change in behavior that is not worth it. Anyhow, thanks for the work done! Luca
Luca you are right. In ClearLinux: IncludeOptional /not/existing/dir/*.conf WORKS IncludeOptional /not/existing/file.conf FAILS I guess both options should work. Our use case is: # Virtual hosts IncludeOptional /usr/share/defaults/httpd/conf.d/*.conf IncludeOptional /usr/share/defaults/httpd/conf.modules.d/*.conf IncludeOptional /etc/httpd/conf.d/*.conf IncludeOptional /etc/httpd/conf.modules.d/*.conf where /etc/httpd sometimes doesn't exist but the service successfully starts with the default page.
Hi Alberto, I just sent an email to the dev@ mailing list to see what the community thinks about this task. Let's see what are the answers :) If you want to follow you can either register to dev@ (https://httpd.apache.org/lists.html#http-dev) or check https://lists.apache.org/list.html?dev@httpd.apache.org. Luca
Committed svn.apache.org/r1814968 to trunk, if the patch is accepted I'll also propose a backport to 2.4.x soon!
Backported to 2.4.x with https://svn.apache.org/r1818964, it will be part of the next httpd release. Thanks Alberto for the help, please re-open if anything is missing!