Skip to content

Commit e933a63

Browse files
Reverse the parse direction
Parse the rule from the head to prevent the parser from finding aliases of the intended argument in the rule's heead. String#slice! searches from the head of a string, so if the parser is using it to iteratively search for strings ordered progressively further from the back of a rule, each iteration is a new opportunity to find an aliased substring in its head and completely moot the point of generating an ordered parse list in the first place. Additionally: - Untie the logical knots that were required for reverse-parsing - Don't use Enumerable#each to do absolutely everything by side effect when an iterator could just return the constructed value directly - Don't pre-define variables we don't need yet just so they can be reassigned - Freshen up pointlessly terse variable names that only exist in an ephemeral scope anyway, this is already hard enough to follow - Stop our value regex from capturing a second, nested group if we only need the outer to avoid any later need to transpose the result - Fix any remaining variables with misleading names (e.g., valrev is no longer "values, reversed") or errant comments that no longer accurately describe the operation of the code
1 parent 63107de commit e933a63

File tree

1 file changed

+18
-17
lines changed

1 file changed

+18
-17
lines changed

lib/puppet/provider/firewall/iptables.rb

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -511,8 +511,6 @@ def self.instances
511511
end
512512

513513
def self.rule_to_hash(line, table, counter)
514-
hash = {}
515-
keys = []
516514
values = ' ' + line
517515

518516
####################
@@ -646,35 +644,38 @@ def self.rule_to_hash(line, table, counter)
646644
# MAIN PARSE
647645
############
648646

649-
# Here we iterate across our values to generate an array of keys
650-
parser_list.reverse_each do |k|
651-
resource_map_key = resource_map[k]
652-
[resource_map_key].flatten.each do |opt|
653-
if values.slice!(%r{\s#{opt}})
654-
keys << k
647+
# Iterate through the sorted list of resource matchers we found, and
648+
# pull them out of the rule text so only their matching values remain
649+
keys = parser_list.each_with_object([]) do |resource_key, found_keys|
650+
[resource_map[resource_key]].flatten.each do |matcher|
651+
if values.slice!(%r{\s#{matcher}})
652+
found_keys << resource_key
655653
break
656654
end
657655
end
658656
end
659657

660-
valrev = values.scan(%r{("([^"\\]|\\.)*"|\S+)}).transpose[0].reverse
658+
# Slurp up the resource values
659+
found_values = values.scan(%r{("(?:[^"\\]|\\.)*"|\S+)}).flatten
661660

662-
if keys.length != valrev.length
663-
warning "Skipping unparsable iptables rule: keys (#{keys.length}) and values (#{valrev.length}) count mismatch on line: #{line}"
661+
if keys.length != found_values.length
662+
warning "Skipping unparsable iptables rule: keys (#{keys.length}) and values (#{found_values.length}) count mismatch on line: #{line}"
664663
return
665664
end
666665

667666
# Here we generate the main hash by scanning arguments off the values
668667
# string, handling any quoted characters present in the value, and then
669668
# zipping the values with the array of keys.
670-
keys.zip(valrev) do |f, v|
671-
hash[f] = if %r{^".*"$}.match?(v)
672-
v.sub(%r{^"(.*)"$}, '\1').gsub(%r{\\(\\|'|")}, '\1')
673-
else
674-
v.dup
675-
end
669+
found_values = found_values.map do |value|
670+
if %r{^".*"$}.match?(value)
671+
value.sub(%r{^"(.*)"$}, '\1').gsub(%r{\\(\\|'|")}, '\1')
672+
else
673+
value
674+
end
676675
end
677676

677+
hash = keys.zip(found_values).to_h
678+
678679
#####################
679680
# POST PARSE CLUDGING
680681
#####################

0 commit comments

Comments
 (0)