From 2f6f6f3fd2425a978c5507651d042cd600bc6cd5 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 15 Jun 2022 13:44:27 +0200
Subject: [PATCH 1/6] Make headers and request_headers arrays

They now both only accept arrays of non-empty strings. The default is
set to an empty array, which behaves as if it's not set.
---
 manifests/vhost.pp                 | 19 ++++++++-----------
 spec/defines/vhost_spec.rb         |  2 +-
 templates/vhost/_header.erb        |  8 +++-----
 templates/vhost/_requestheader.erb |  8 +++-----
 4 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/manifests/vhost.pp b/manifests/vhost.pp
index ad0c312146..80dfd7d16c 100644
--- a/manifests/vhost.pp
+++ b/manifests/vhost.pp
@@ -1853,8 +1853,8 @@
   Optional[Variant[Array[String],String]] $redirectmatch_status                       = undef,
   Optional[Variant[Array[String],String]] $redirectmatch_regexp                       = undef,
   Optional[Variant[Array[String],String]] $redirectmatch_dest                         = undef,
-  Optional[String] $headers                                                           = undef,
-  Optional[Array[String]] $request_headers                                            = undef,
+  Array[String[1]] $headers                                                           = [],
+  Array[String[1]] $request_headers                                                   = [],
   Optional[Array[String]] $filters                                                    = undef,
   Optional[Array] $rewrites                                                           = undef,
   Optional[String] $rewrite_base                                                      = undef,
@@ -2259,13 +2259,6 @@
     }
   }
 
-  # Check if mod_headers is required to process $headers/$request_headers
-  if $headers or $request_headers {
-    if ! defined(Class['apache::mod::headers']) {
-      include apache::mod::headers
-    }
-  }
-
   # Check if mod_filter is required to process $filters
   if $filters {
     if ! defined(Class['apache::mod::filter']) {
@@ -2534,7 +2527,9 @@
 
   # Template uses:
   # - $headers
-  if $headers and ! empty($headers) {
+  if ! empty($headers) and $ensure == 'present' {
+    include apache::mod::headers
+
     concat::fragment { "${name}-header":
       target  => "${priority_real}${filename}.conf",
       order   => 140,
@@ -2544,7 +2539,9 @@
 
   # Template uses:
   # - $request_headers
-  if $request_headers and ! empty($request_headers) {
+  if ! empty($request_headers) and $ensure == 'present' {
+    include apache::mod::headers
+
     concat::fragment { "${name}-requestheader":
       target  => "${priority_real}${filename}.conf",
       order   => 150,
diff --git a/spec/defines/vhost_spec.rb b/spec/defines/vhost_spec.rb
index f79fb4065e..f455fd5f75 100644
--- a/spec/defines/vhost_spec.rb
+++ b/spec/defines/vhost_spec.rb
@@ -385,7 +385,7 @@
               'redirectmatch_status'        => ['404'],
               'redirectmatch_regexp'        => ['\.git$'],
               'redirectmatch_dest'          => ['http://www.example.com'],
-              'headers'                     => 'Set X-Robots-Tag "noindex, noarchive, nosnippet"',
+              'headers'                     => ['Set X-Robots-Tag "noindex, noarchive, nosnippet"'],
               'request_headers'             => ['append MirrorID "mirror 12"'],
               'rewrites'                    => [
                 {
diff --git a/templates/vhost/_header.erb b/templates/vhost/_header.erb
index c0f68c8257..54f84cac5a 100644
--- a/templates/vhost/_header.erb
+++ b/templates/vhost/_header.erb
@@ -1,10 +1,8 @@
-<% if @headers and ! @headers.empty? -%>
+<% unless @headers.empty? -%>
 
   ## Header rules
-  ## as per http://httpd.apache.org/docs/2.2/mod/mod_headers.html#header
-  <%- Array(@headers).each do |header_statement| -%>
-    <%- if header_statement != '' -%>
+  ## as per http://httpd.apache.org/docs/2.4/mod/mod_headers.html#header
+  <%- @headers.each do |header_statement| -%>
   Header <%= header_statement %>
-    <%- end -%>
   <%- end -%>
 <% end -%>
diff --git a/templates/vhost/_requestheader.erb b/templates/vhost/_requestheader.erb
index 9f175052b5..56ee68b299 100644
--- a/templates/vhost/_requestheader.erb
+++ b/templates/vhost/_requestheader.erb
@@ -1,10 +1,8 @@
-<% if @request_headers and ! @request_headers.empty? -%>
+<% unless @request_headers.empty? -%>
 
   ## Request header rules
-  ## as per http://httpd.apache.org/docs/2.2/mod/mod_headers.html#requestheader
-  <%- Array(@request_headers).each do |request_statement| -%>
-    <%- if request_statement != '' -%>
+  ## as per http://httpd.apache.org/docs/2.4/mod/mod_headers.html#requestheader
+  <%- @request_headers.each do |request_statement| -%>
   RequestHeader <%= request_statement %>
-    <%- end -%>
   <%- end -%>
 <% end -%>

From d00aabb0c196962c3cf078394f7b5ff7d9fe9104 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 15 Jun 2022 14:06:03 +0200
Subject: [PATCH 2/6] Stricter data type on apache::vhost::filters

This was previously filtered out but it's best if the information simply
never is entered in the first place.

The include is also simplified and only applied if the vhost is present.
---
 manifests/vhost.pp           | 13 ++++---------
 templates/vhost/_filters.erb |  6 ++----
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/manifests/vhost.pp b/manifests/vhost.pp
index 80dfd7d16c..531bed1584 100644
--- a/manifests/vhost.pp
+++ b/manifests/vhost.pp
@@ -1855,7 +1855,7 @@
   Optional[Variant[Array[String],String]] $redirectmatch_dest                         = undef,
   Array[String[1]] $headers                                                           = [],
   Array[String[1]] $request_headers                                                   = [],
-  Optional[Array[String]] $filters                                                    = undef,
+  Array[String[1]] $filters                                                           = [],
   Optional[Array] $rewrites                                                           = undef,
   Optional[String] $rewrite_base                                                      = undef,
   Optional[Variant[Array[String],String]] $rewrite_rule                               = undef,
@@ -2259,13 +2259,6 @@
     }
   }
 
-  # Check if mod_filter is required to process $filters
-  if $filters {
-    if ! defined(Class['apache::mod::filter']) {
-      include apache::mod::filter
-    }
-  }
-
   # Check if mod_env is required and not yet loaded.
   # create an expression to simplify the conditional check
   $use_env_mod = $setenv and ! empty($setenv)
@@ -2906,7 +2899,9 @@
 
   # Template uses:
   # - $filters
-  if $filters and ! empty($filters) {
+  if ! empty($filters) and $ensure == 'present' {
+    include apache::mod::filter
+
     concat::fragment { "${name}-filters":
       target  => "${priority_real}${filename}.conf",
       order   => 330,
diff --git a/templates/vhost/_filters.erb b/templates/vhost/_filters.erb
index b862597349..bb36fc3914 100644
--- a/templates/vhost/_filters.erb
+++ b/templates/vhost/_filters.erb
@@ -1,10 +1,8 @@
-<% if @filters and ! @filters.empty? -%>
+<% unless @filters.empty? -%>
 
   ## Filter module rules
   ## as per http://httpd.apache.org/docs/2.2/mod/mod_filter.html
-  <%- Array(@filters).each do |filter| -%>
-    <%- if filter != '' -%>
+  <%- @filters.each do |filter| -%>
   <%= filter %>
-    <%- end -%>
   <%- end -%>
 <% end -%>

From 7b22dae24ff4a04f9258362f90bf6a2151c24a23 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 15 Jun 2022 14:07:35 +0200
Subject: [PATCH 3/6] Stricter data types on apache::vhost::rewrite*

Ideally a Struct would be used instead of Hash for $rewrites, but this
is a closer description of the real data type.
---
 manifests/vhost.pp           | 26 +++++++-------------------
 spec/defines/vhost_spec.rb   |  2 +-
 templates/vhost/_rewrite.erb |  6 ++----
 3 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/manifests/vhost.pp b/manifests/vhost.pp
index 531bed1584..79aa2c3feb 100644
--- a/manifests/vhost.pp
+++ b/manifests/vhost.pp
@@ -1856,10 +1856,10 @@
   Array[String[1]] $headers                                                           = [],
   Array[String[1]] $request_headers                                                   = [],
   Array[String[1]] $filters                                                           = [],
-  Optional[Array] $rewrites                                                           = undef,
-  Optional[String] $rewrite_base                                                      = undef,
-  Optional[Variant[Array[String],String]] $rewrite_rule                               = undef,
-  Optional[Variant[Array[String],String]] $rewrite_cond                               = undef,
+  Array[Hash] $rewrites                                                               = [],
+  Optional[String[1]] $rewrite_base                                                   = undef,
+  Optional[String[1]] $rewrite_rule                                                   = undef,
+  Array[String[1]] $rewrite_cond                                                      = [],
   Boolean $rewrite_inherit                                                            = false,
   Variant[Array[String],String] $setenv                                               = [],
   Variant[Array[String],String] $setenvif                                             = [],
@@ -1997,13 +1997,6 @@
 
   $apache_name = $apache::apache_name
 
-  if $rewrites {
-    unless empty($rewrites) {
-      $rewrites_flattened = delete_undef_values(flatten([$rewrites]))
-      assert_type(Array[Hash], $rewrites_flattened)
-    }
-  }
-
   # Input validation begins
 
   if $access_log_file and $access_log_pipe {
@@ -2225,13 +2218,6 @@
     }
   }
 
-  # Load mod_rewrite if needed and not yet loaded
-  if $rewrites or $rewrite_cond {
-    if ! defined(Class['apache::mod::rewrite']) {
-      include apache::mod::rewrite
-    }
-  }
-
   # Load mod_alias if needed and not yet loaded
   if ($scriptalias or $scriptaliases != [])
   or ($aliases and $aliases != [])
@@ -2603,7 +2589,9 @@
   # - $rewrite_rule
   # - $rewrite_cond
   # - $rewrite_map
-  if $rewrites or $rewrite_rule {
+  if (! empty($rewrites) or $rewrite_rule) and $ensure == 'present' {
+    include apache::mod::rewrite
+
     concat::fragment { "${name}-rewrite":
       target  => "${priority_real}${filename}.conf",
       order   => 190,
diff --git a/spec/defines/vhost_spec.rb b/spec/defines/vhost_spec.rb
index f455fd5f75..3a408977cc 100644
--- a/spec/defines/vhost_spec.rb
+++ b/spec/defines/vhost_spec.rb
@@ -403,7 +403,7 @@
               ],
               'rewrite_base'                => '/',
               'rewrite_rule'                => '^index\.html$ welcome.html',
-              'rewrite_cond'                => '%{HTTP_USER_AGENT} ^MSIE',
+              'rewrite_cond'                => ['%{HTTP_USER_AGENT} ^MSIE'],
               'rewrite_inherit'             => true,
               'setenv'                      => ['FOO=/bin/true'],
               'setenvif'                    => 'Request_URI "\.gif$" object_is_image=gif',
diff --git a/templates/vhost/_rewrite.erb b/templates/vhost/_rewrite.erb
index 282733757c..f4ac5a3174 100644
--- a/templates/vhost/_rewrite.erb
+++ b/templates/vhost/_rewrite.erb
@@ -8,7 +8,7 @@
   RewriteBase <%= @rewrite_base %>
   <%- end -%>
 
-  <%- [@rewrites].flatten.compact.each do |rewrite_details| -%>
+  <%- @rewrites.each do |rewrite_details| -%>
     <%- if rewrite_details['comment'] -%>
   #<%= rewrite_details['comment'] %>
     <%- end -%>
@@ -44,10 +44,8 @@
   <%- if @rewrite_base -%>
   RewriteBase <%= @rewrite_base %>
   <%- end -%>
-  <%- if @rewrite_cond -%>
-    <%- Array(@rewrite_cond).each do |cond| -%>
+    <%- @rewrite_cond.each do |cond| -%>
   RewriteCond <%= cond %>
-    <%- end -%>
   <%- end -%>
   RewriteRule <%= @rewrite_rule %>
 <%- end -%>

From 56c9e1e9021570b7d6ba4a28a41b07bc987a3a5b Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 15 Jun 2022 14:15:54 +0200
Subject: [PATCH 4/6] Stricter data type on apache::vhost::access_logs

This always required an array of hashes and closer matches the actual
behavior.

It also fixes the check on the template use by checking the correct
variable. bb96180f62a1897a9f81dfb6a821f2f10c9bdfd3 broke this.

Fixes: bb96180f62a1897a9f81dfb6a821f2f10c9bdfd3
---
 manifests/vhost.pp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/manifests/vhost.pp b/manifests/vhost.pp
index 79aa2c3feb..6f503f12d6 100644
--- a/manifests/vhost.pp
+++ b/manifests/vhost.pp
@@ -1800,7 +1800,7 @@
   Variant[Boolean,String] $access_log_syslog                                          = false,
   Variant[Boolean,String] $access_log_format                                          = false,
   Variant[Boolean,String] $access_log_env_var                                         = false,
-  Optional[Array] $access_logs                                                        = undef,
+  Optional[Array[Hash]] $access_logs                                                  = undef,
   Boolean $use_servername_for_filenames                                               = false,
   Boolean $use_port_for_filenames                                                     = false,
   Optional[Variant[Array[Hash],Hash,String]] $aliases                                 = undef,
@@ -2134,6 +2134,8 @@
     }]
   } elsif $access_logs {
     $_access_logs = $access_logs
+  } else {
+    $_access_logs = []
   }
 
   if $error_log_file {
@@ -2459,13 +2461,12 @@
   }
 
   # Template uses:
-  # - $access_log
+  # - $_access_logs
   # - $_access_log_env_var
   # - $access_log_destination
   # - $_access_log_format
   # - $_access_log_env_var
-  # - $access_logs
-  if $access_log or $access_logs {
+  if !empty($_access_logs) {
     concat::fragment { "${name}-access_log":
       target  => "${priority_real}${filename}.conf",
       order   => 100,

From bcd35a7657a5a1ed2509a2482875b6853c53600c Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 15 Jun 2022 14:22:39 +0200
Subject: [PATCH 5/6] Stricter data type on apache::vhost::modsec_disable_ips

---
 manifests/vhost.pp            | 4 ++--
 templates/vhost/_security.erb | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/manifests/vhost.pp b/manifests/vhost.pp
index 6f503f12d6..e4fa1b2177 100644
--- a/manifests/vhost.pp
+++ b/manifests/vhost.pp
@@ -1954,7 +1954,7 @@
   Optional[String] $add_default_charset                                               = undef,
   Boolean $modsec_disable_vhost                                                       = false,
   Optional[Variant[Hash, Array]] $modsec_disable_ids                                  = undef,
-  Optional[Array[String]] $modsec_disable_ips                                         = undef,
+  Array[String[1]] $modsec_disable_ips                                                = [],
   Optional[Variant[Hash, Array]] $modsec_disable_msgs                                 = undef,
   Optional[Variant[Hash, Array]] $modsec_disable_tags                                 = undef,
   Optional[String] $modsec_body_limit                                                 = undef,
@@ -2878,7 +2878,7 @@
   # - $modsec_disable_tags
   # - $modsec_body_limit
   # - $modsec_audit_log_destination
-  if $modsec_disable_vhost or $modsec_disable_ids or $modsec_disable_ips or $modsec_disable_msgs or $modsec_disable_tags or $modsec_audit_log_destination {
+  if $modsec_disable_vhost or $modsec_disable_ids or !empty($modsec_disable_ips) or $modsec_disable_msgs or $modsec_disable_tags or $modsec_audit_log_destination {
     concat::fragment { "${name}-security":
       target  => "${priority_real}${filename}.conf",
       order   => 320,
diff --git a/templates/vhost/_security.erb b/templates/vhost/_security.erb
index dc35c78af1..a7e02e6c59 100644
--- a/templates/vhost/_security.erb
+++ b/templates/vhost/_security.erb
@@ -14,9 +14,8 @@
   </LocationMatch>
 <%   end -%>
 <% end -%>
-<% ips = Array(@modsec_disable_ips).join(',') %>
-<% if ips != '' %>
-  SecRule REMOTE_ADDR "<%= ips %>" "nolog,allow,id:1234123455"
+<% unless @modsec_disable_ips.empty? %>
+  SecRule REMOTE_ADDR "<%= @modsec_disable_ips.join(',') %>" "nolog,allow,id:1234123455"
   SecAction  "phase:2,pass,nolog,id:1234123456"
 <% end -%>
 <% if @_modsec_disable_msgs.is_a?(Hash) -%>

From 8c515509cca881411c3f927d9cd6117a3cead092 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Wed, 15 Jun 2022 14:30:07 +0200
Subject: [PATCH 6/6] Stricter data type on apache::vhost::jk_mounts

This also pulls in mod_jk if needed.
---
 manifests/vhost.pp             | 6 ++++--
 templates/vhost/_jk_mounts.erb | 8 +++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/manifests/vhost.pp b/manifests/vhost.pp
index e4fa1b2177..a92331f68d 100644
--- a/manifests/vhost.pp
+++ b/manifests/vhost.pp
@@ -1958,7 +1958,7 @@
   Optional[Variant[Hash, Array]] $modsec_disable_msgs                                 = undef,
   Optional[Variant[Hash, Array]] $modsec_disable_tags                                 = undef,
   Optional[String] $modsec_body_limit                                                 = undef,
-  Optional[Array[Hash]] $jk_mounts                                                    = undef,
+  Array[Hash] $jk_mounts                                                              = [],
   Boolean $auth_kerb                                                                  = false,
   Enum['on', 'off'] $krb_method_negotiate                                             = 'on',
   Enum['on', 'off'] $krb_method_k5passwd                                              = 'on',
@@ -2900,7 +2900,9 @@
 
   # Template uses:
   # - $jk_mounts
-  if $jk_mounts and ! empty($jk_mounts) {
+  if !empty($jk_mounts) and $ensure == 'present' {
+    include apache::mod::jk
+
     concat::fragment { "${name}-jk_mounts":
       target  => "${priority_real}${filename}.conf",
       order   => 340,
diff --git a/templates/vhost/_jk_mounts.erb b/templates/vhost/_jk_mounts.erb
index 8cb1d116bb..3bd5a721a6 100644
--- a/templates/vhost/_jk_mounts.erb
+++ b/templates/vhost/_jk_mounts.erb
@@ -1,12 +1,10 @@
-<% if @jk_mounts and not @jk_mounts.empty? -%>
+<% unless @jk_mounts.empty? -%>
 
   <%- @jk_mounts.each do |jk| -%>
-    <%- if jk.is_a?(Hash) -%>
-      <%- if jk.has_key?('mount') and jk.has_key?('worker') -%>
+    <%- if jk.has_key?('mount') and jk.has_key?('worker') -%>
   JkMount   <%= jk['mount'] %> <%= jk['worker'] %>
-      <%- elsif jk.has_key?('unmount') and jk.has_key?('worker') -%>
+    <%- elsif jk.has_key?('unmount') and jk.has_key?('worker') -%>
   JkUnMount <%= jk['unmount'] %> <%= jk['worker'] %>
-      <%- end -%>
     <%- end -%>
   <%- end -%>
 <% end -%>