From 80c595ba3d215de9d1effce47651b85c1354dcfb Mon Sep 17 00:00:00 2001 From: Jerry Aldrich Date: Mon, 17 Jun 2019 21:39:37 -0800 Subject: [PATCH 1/2] nginx_conf: Fix commented/empty file parsing This fixes `nginx_conf.params` when: - Given an empty file - Given a file with only comments - Given a file that has an include for a file that: - Is empty - Has all lines commented out This also fixes a test where a missing file is actually empty Signed-off-by: Jerry Aldrich --- lib/inspec/resources/nginx_conf.rb | 5 +++ test/helpers/mock_loader.rb | 3 +- .../mock/cmd/find-nginx-confd-multiple-conf | 2 ++ .../mock/files/nginx_confd_comments_only.conf | 33 +++++++++++++++++++ test/unit/mock/files/nginx_confd_empty.conf | 0 test/unit/resources/nginx_conf_test.rb | 16 ++++++++- 6 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 test/unit/mock/files/nginx_confd_comments_only.conf create mode 100644 test/unit/mock/files/nginx_confd_empty.conf diff --git a/lib/inspec/resources/nginx_conf.rb b/lib/inspec/resources/nginx_conf.rb index 8e77206cb..56a3a86d7 100644 --- a/lib/inspec/resources/nginx_conf.rb +++ b/lib/inspec/resources/nginx_conf.rb @@ -63,6 +63,11 @@ module Inspec::Resources def parse_nginx(path) return nil if inspec.os.windows? content = read_content(path) + + # Don't attempt to parse file if it contains only comments or is empty + # https://regexper.com/#%2F%5E%5Cs*%23%7C%5E%24%2F + return {} if content.split("\n").reject { |l| l =~ /^\s*#|^$/ } == [] + data = NginxConfig.parse(content) resolve_references(data, File.dirname(path)) rescue StandardError => _ diff --git a/test/helpers/mock_loader.rb b/test/helpers/mock_loader.rb index 051076a3f..701c9d822 100644 --- a/test/helpers/mock_loader.rb +++ b/test/helpers/mock_loader.rb @@ -118,6 +118,8 @@ class MockLoader "/etc/nginx/nginx.conf" => mockfile.call("nginx.conf"), "/etc/nginx/proxy.conf" => mockfile.call("nginx_proxy.conf"), "/etc/nginx/conf/mime.types" => mockfile.call("nginx_mime.types"), + "/etc/nginx/conf.d/comments_only.conf" => mockfile.call("nginx_confd_comments_only.conf"), + "/etc/nginx/conf.d/empty.conf" => mockfile.call("nginx_confd_empty.conf"), "/etc/nginx/conf.d/foobar.conf" => mockfile.call("nginx_confd_foobar.conf"), "/etc/nginx/conf.d/multiple.conf" => mockfile.call("nginx_confd_multiple.conf"), "/etc/nginx/quotes.d/example.conf" => mockfile.call("nginx_quotesd_example.conf"), @@ -156,7 +158,6 @@ class MockLoader "/fakepath/fakefile" => emptyfile.call, "C:/fakepath/fakefile" => emptyfile.call, "/etc/cron.d/crondotd" => mockfile.call("crondotd"), - "/missing_file" => emptyfile.call, } # create all mock commands diff --git a/test/unit/mock/cmd/find-nginx-confd-multiple-conf b/test/unit/mock/cmd/find-nginx-confd-multiple-conf index 7e4570c40..d0bc82193 100644 --- a/test/unit/mock/cmd/find-nginx-confd-multiple-conf +++ b/test/unit/mock/cmd/find-nginx-confd-multiple-conf @@ -1,2 +1,4 @@ +/etc/nginx/conf.d/comments_only.conf +/etc/nginx/conf.d/empty.conf /etc/nginx/conf.d/foobar.conf /etc/nginx/conf.d/multiple.conf diff --git a/test/unit/mock/files/nginx_confd_comments_only.conf b/test/unit/mock/files/nginx_confd_comments_only.conf new file mode 100644 index 000000000..644de5f1b --- /dev/null +++ b/test/unit/mock/files/nginx_confd_comments_only.conf @@ -0,0 +1,33 @@ +# This file is empty save for comments + +# +# HTTPS server configuration +# + +#server { +# listen 443 ssl http2 default_server; +# listen [::]:443 ssl; +# server_name _; +# root /usr/share/nginx/html; +# +# ssl_certificate cert.pem; +# ssl_certificate_key cert.key; +# ssl_session_cache shared:SSL:1m; +# ssl_session_timeout 10m; +# ssl_ciphers HIGH:!aNULL:!MD5; +# ssl_prefer_server_ciphers on; +# +# # Load configuration files for the default server block. +# include /etc/nginx/default.d/*.conf; +# +# location / { +# } +# +# error_page 404 /404.html; +# location = /40x.html { +# } +# +# error_page 500 502 503 504 /50x.html; +# location = /50x.html { +# } +#} diff --git a/test/unit/mock/files/nginx_confd_empty.conf b/test/unit/mock/files/nginx_confd_empty.conf new file mode 100644 index 000000000..e69de29bb diff --git a/test/unit/resources/nginx_conf_test.rb b/test/unit/resources/nginx_conf_test.rb index 3a3fb7629..274775f01 100644 --- a/test/unit/resources/nginx_conf_test.rb +++ b/test/unit/resources/nginx_conf_test.rb @@ -10,7 +10,19 @@ describe "Inspec::Resources::NginxConf" do let(:nginx_conf) { MockLoader.new(:ubuntu1404).load_resource("nginx_conf") } it "doesnt fail with a missing file" do - nginx_conf = MockLoader.new(:ubuntu1404).load_resource("nginx_conf", "/missing_file") + # This path is not mocked because we cannot mock File.exist? + # ...As far as I know + nginx_conf = MockLoader.new(:ubuntu1404).load_resource("nginx_conf", "/this/path/does/not/exist") + _(nginx_conf.params).must_equal({}) + end + + it "does not fail with an empty file" do + nginx_conf = MockLoader.new(:ubuntu1404).load_resource("nginx_conf", "/etc/nginx/conf.d/empty.conf") + _(nginx_conf.params).must_equal({}) + end + + it "does not fail with a file that all lines are commented out" do + nginx_conf = MockLoader.new(:ubuntu1404).load_resource("nginx_conf", "/etc/nginx/conf.d/comments_only.conf") _(nginx_conf.params).must_equal({}) end @@ -26,6 +38,8 @@ describe "Inspec::Resources::NginxConf" do /etc/nginx/nginx.conf /etc/nginx/conf/mime.types /etc/nginx/proxy.conf + /etc/nginx/conf.d/comments_only.conf + /etc/nginx/conf.d/empty.conf /etc/nginx/conf.d/foobar.conf /etc/nginx/conf.d/multiple.conf /etc/nginx/quotes.d/example.conf From 2d72cd5905a213782d319a4163a7dae30fdea569 Mon Sep 17 00:00:00 2001 From: Jerry Aldrich Date: Tue, 18 Jun 2019 08:05:57 -0800 Subject: [PATCH 2/2] Modify line to be more idiomatic. Thanks @clintoncwolfe! Signed-off-by: Jerry Aldrich --- lib/inspec/resources/nginx_conf.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/inspec/resources/nginx_conf.rb b/lib/inspec/resources/nginx_conf.rb index 56a3a86d7..3a4480b67 100644 --- a/lib/inspec/resources/nginx_conf.rb +++ b/lib/inspec/resources/nginx_conf.rb @@ -66,7 +66,7 @@ module Inspec::Resources # Don't attempt to parse file if it contains only comments or is empty # https://regexper.com/#%2F%5E%5Cs*%23%7C%5E%24%2F - return {} if content.split("\n").reject { |l| l =~ /^\s*#|^$/ } == [] + return {} if content.lines.reject { |l| l =~ /^\s*#|^$/ }.empty? data = NginxConfig.parse(content) resolve_references(data, File.dirname(path))