Skip to content

Fix args passed to create_tsig_opts#68

Merged
alexdalitz merged 1 commit into
alexdalitz:masterfrom
elibus:fix-null
Feb 15, 2015
Merged

Fix args passed to create_tsig_opts#68
alexdalitz merged 1 commit into
alexdalitz:masterfrom
elibus:fix-null

Conversation

@elibus
Copy link
Copy Markdown
Contributor

@elibus elibus commented Feb 14, 2015

Wrong argument is passed to the create_tsig_options in the Resolver.get_tsig function.
The function checks if args[0] is an Array and then it passes just args.
Fixing this makes AXFR w/ TSIG work again.

This is a sample working code.

require 'dnsruby'
res = Dnsruby::Resolver.new

server = '127.0.0.1'
zone = 'example.com'

KEY_NAME = "dns-server.example.com"
KEY = "wI6XiocuMR8X/DySzKVbp2SdzZZeXCsQLjEs6HRlnkY=";

zt = Dnsruby::ZoneTransfer.new
zt.server = server
zt.transfer_type = Dnsruby::Types.AXFR
zt.tsig= KEY_NAME, KEY

zoneref = zt.transfer(zone)

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.14%) to 76.81% when pulling 0f00a2e on elibus:fix-null into efcfe59 on alexdalitz:master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.14%) to 76.81% when pulling 0f00a2e on elibus:fix-null into efcfe59 on alexdalitz:master.

@keithrbennett
Copy link
Copy Markdown
Contributor

Firstly, thanks for doing all this, and investing the time in making this a better gem.

Is it possible to reduce your script to a code fragment that can be a test method in test/tc_resolver.rb, preferably testing as narrow a scope of code as possible (e.g. not the whole AXFR operation, if that's possible), and without access to a server? To test just that test file from the command line you can do:

rake test TEST=test/tc_resolver.rb

Since the other tests in that file take a while to run, I disable them by wrapping them in:

=begin
# all the other tests
=end

We would want to ensure that not only does it pass after the modification, but also that it fails before it.

Looking at create_tsig_options, it looks like there's a misleading arg test and comment; it appears to permit an arg array of size 1, but in the code it looks like it does nothing with such an array. I suspect that this is not the intended action. So although the test does not fail, it may not be behaving correctly either. The correct behavior might be to use the splat on args0 to expand the elements into separate function arguments, rather than to pass the array as itself. (And, to fix create_tsig_options so that it prohibits an array size of 1 and is documented as such.)

alexdalitz added a commit that referenced this pull request Feb 15, 2015
Fix args passed to create_tsig_opts
@alexdalitz alexdalitz merged commit d2680e9 into alexdalitz:master Feb 15, 2015
@elibus
Copy link
Copy Markdown
Contributor Author

elibus commented Feb 15, 2015

@keithrbennett Hi, just opened a new pr #69 . Tried to work as you suggested and fixed a small bug.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants