Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

14463 Convert Fixnum into strings. #101

Closed
wants to merge 4 commits into from

3 participants

@mrwacky42

Avert errors like this:
Parameter dport failed: Munging failed for value 1194 in class dport: can’t convert Fixnum into String

As it happens Socket.getservbyname only wants strings.

Also, the string_to_port method now tells Socket which protocol to use (vs the previous default of only asking about TCP). I cannot think of any services that use different ports for TCP vs UDP, but here is a fix anyway.

mrwacky42 added some commits
@mrwacky42 mrwacky42 14463 Convert Fixnum into strings.
Avert errors like this:
Parameter dport failed: Munging failed for value 1194 in class dport: can’t convert Fixnum into String

Also, pass along the protocol so Socket can make well informed decisions.
2421574
@mrwacky42 mrwacky42 Fix to pass unit tests.
* Add default protocol to fix the test for converting a string 'ssh' to a port
number was failing like so:
  1) Puppet::Type::Firewall dport should convert a port name for dport to its number
     Failure/Error: @resource[port] = 'ssh'
     Puppet::Error:
       Parameter dport failed: Munging failed for value "ssh" in class dport: no such service ssh/proto
     # ./lib/puppet/type/../../puppet/util/firewall.rb:84:in `getservbyname'
     # ./lib/puppet/type/../../puppet/util/firewall.rb:84:in `string_to_port'
     # ./lib/puppet/type/firewall.rb:164:in `unsafe_munge'
     # ./spec/unit/puppet/type/firewall_spec.rb:161

* Always convert the response .to_s
b4c00b6
@jbraeuer

Nice to see this issue gets fixed!

lib/puppet/util/firewall.rb
((8 lines not shown))
#
# If the string already contains a port number or perhaps a range of ports
# in the format 22:1000 for example, it simply returns the string and does
# nothing.
- def string_to_port(value)
+ def string_to_port(value, proto)
+ proto = proto.to_s
+ unless proto =~ /^tcp|udp$/
@dcarley Collaborator
dcarley added a note

/^(tcp|udp)$/ ?

Right you are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mrwacky42

Hmm.
All the Travis builds succeeded, except one that timed out..
https://travis-ci.org/puppetlabs/puppetlabs-firewall/jobs/3404783

@mrwacky42 mrwacky42 14463 Add a UDP lookup test as well.
Mostly just to get Travis to try again.
761f505
@dcarley dcarley closed this pull request from a commit
@dcarley dcarley Merge branch '14463-port_fixnums_to_strings'
Fixes #101 pull request.
b0a5122
@dcarley dcarley closed this in b0a5122
@dcarley
Collaborator

Cheers. Rebased both pairs of commits together for clarity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 27, 2012
  1. @mrwacky42

    14463 Convert Fixnum into strings.

    mrwacky42 authored
    Avert errors like this:
    Parameter dport failed: Munging failed for value 1194 in class dport: can’t convert Fixnum into String
    
    Also, pass along the protocol so Socket can make well informed decisions.
  2. @mrwacky42

    Fix to pass unit tests.

    mrwacky42 authored
    * Add default protocol to fix the test for converting a string 'ssh' to a port
    number was failing like so:
      1) Puppet::Type::Firewall dport should convert a port name for dport to its number
         Failure/Error: @resource[port] = 'ssh'
         Puppet::Error:
           Parameter dport failed: Munging failed for value "ssh" in class dport: no such service ssh/proto
         # ./lib/puppet/type/../../puppet/util/firewall.rb:84:in `getservbyname'
         # ./lib/puppet/type/../../puppet/util/firewall.rb:84:in `string_to_port'
         # ./lib/puppet/type/firewall.rb:164:in `unsafe_munge'
         # ./spec/unit/puppet/type/firewall_spec.rb:161
    
    * Always convert the response .to_s
Commits on Nov 28, 2012
  1. @mrwacky42

    Use grown up regex.

    mrwacky42 authored
  2. @mrwacky42

    14463 Add a UDP lookup test as well.

    mrwacky42 authored
    Mostly just to get Travis to try again.
This page is out of date. Refresh to see the latest.
View
6 lib/puppet/type/firewall.rb
@@ -131,7 +131,7 @@
EOS
munge do |value|
- @resource.string_to_port(value)
+ @resource.string_to_port(value, :proto)
end
def is_to_s(value)
@@ -161,7 +161,7 @@ def should_to_s(value)
EOS
munge do |value|
- @resource.string_to_port(value)
+ @resource.string_to_port(value, :proto)
end
def is_to_s(value)
@@ -191,7 +191,7 @@ def should_to_s(value)
EOS
munge do |value|
- @resource.string_to_port(value)
+ @resource.string_to_port(value, :proto)
end
def is_to_s(value)
View
15 lib/puppet/util/firewall.rb
@@ -65,21 +65,26 @@ def log_level_name_to_number(value)
end
end
- # This method takes a string and attempts to convert it to a port number
- # if valid.
+ # This method takes a string and a protocol and attempts to convert
+ # it to a port number if valid.
#
# If the string already contains a port number or perhaps a range of ports
# in the format 22:1000 for example, it simply returns the string and does
# nothing.
- def string_to_port(value)
+ def string_to_port(value, proto)
+ proto = proto.to_s
+ unless proto =~ /^(tcp|udp)$/
+ proto = 'tcp'
+ end
+
if value.kind_of?(String)
if value.match(/^\d+(-\d+)?$/)
return value
else
- return Socket.getservbyname(value).to_s
+ return Socket.getservbyname(value, proto).to_s
end
else
- Socket.getservbyname(value)
+ Socket.getservbyname(value.to_s, proto).to_s
end
end
View
5 spec/unit/puppet/type/firewall_spec.rb
@@ -140,6 +140,11 @@
@resource[port].should == ['22','23']
end
+ it "should accept a #{port} as a number" do
+ @resource[port] = 22
+ @resource[port].should == ['22']
+ end
+
it "should accept a #{port} as a hyphen separated range" do
@resource[port] = ['22-1000']
@resource[port].should == ['22-1000']
View
6 spec/unit/puppet/util/firewall_spec.rb
@@ -68,8 +68,10 @@
describe '#string_to_port' do
subject { resource }
- specify { subject.string_to_port('80').should == '80' }
- specify { subject.string_to_port('http').should == '80' }
+ specify { subject.string_to_port('80','tcp').should == '80' }
+ specify { subject.string_to_port(80,'tcp').should == '80' }
+ specify { subject.string_to_port('http','tcp').should == '80' }
+ specify { subject.string_to_port('domain','udp').should == '53' }
end
describe '#to_hex32' do
Something went wrong with that request. Please try again.