New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

registry_key fails when item has a '.' in name #1281

Closed
pburkholder opened this Issue Nov 6, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@pburkholder
Contributor

pburkholder commented Nov 6, 2016

Description

Checks like this where the item, 'explorer.exe' has a dot ('.') in the name, always results in a NoMethodError:

      describe registry_key({
        hive: 'HKLM',
        key:  'Software\Policies\Microsoft\Internet Explorer\Main\FeatureControl\FEATURE_RESTRICT_ACTIVEXINSTALL',
      }) do
        its('explorer.exe') { should eq 1 }
      end

results in

 1) Registry Key HKLM\oftware\Policies\Microsoft\Internet Explorer\Main\FeatureControl\FEATURE_RESTRICT_ACTIVEXINSTALL explorer.exe
     Failure/Error: inner_subject.send(attr)

     NoMethodError:
       undefined method `exe' for nil:NilClass

InSpec and Platform Version

1.4.1

Replication Case

Tried this in two 2012R2 instances...

PS C:\Users\pburkholder> inspec shell
Welcome to the interactive InSpec Shell
To find out how to use it, type: ☺☻help☺☻
inspec> describe registry_key({
inspec>     hive: 'HKLM',
inspec>     key:  'Software\Policies\Microsoft\Internet Explorer\Main\FeatureControl\FEATURE_RESTRICT_ACTIVEXINSTALL',

inspec> }) do
inspec>   its('explorer.exe') { should eq 1 }
inspec> end

  Registry Key
     ✖  undefined method `exe' for nil:NilClass

Summary: 0 successful, 1 failures, 0 skipped

Try the above example...

Possible Solutions

If you have already ideas how to solve the issue, add them here.

Stacktrace

Please include the stacktrace output or link to a gist of it, if there is one.

@username-is-already-taken2

This comment has been minimized.

Show comment
Hide comment
@username-is-already-taken2

username-is-already-taken2 Nov 8, 2016

Contributor

Hi there

I thought I would take a punt last night and see if I could find where the problem lies (I'm still find my way with ruby/pry)

the registry_key resource looks to be OK IMHO, the values looks to gathered correctly and the def registry_property_value does evaluate correctly if it has the right property :)

but at this point the property is now an array ["explorer", "exe"]
digging a bit further it looks like its being done here

C:\opscode\chefdk\embedded\lib\ruby\gems\2.3.0\gems\rspec-its-1.2.0\lib\rspec\its.rb

  def its(attribute, *options, &block)
      its_caller = caller.select {|file_line| file_line !~ %r(/lib/rspec/its) }
      describe(attribute.to_s, :caller => its_caller) do
        let(:__its_subject) do
          if Array === attribute
            if Hash === subject
              attribute.inject(subject) {|inner, attr| inner[attr] }
            else
              subject[*attribute]
            end
          else
            attribute_chain = attribute.to_s.split('.')
            attribute_chain.inject(subject) do |inner_subject, attr|
              inner_subject.send(attr)
            end
          end
        end

infact looking at the comments (thank you Mr Developer ;) it would seem they are catering for phone numbers

    # The attribute can be a `Symbol` or a `String`. Given a `String`
    # with dots, the result is as though you concatenated that `String`
    # onto the subject in an expression.

Now for the hard bit working on a fix :)

should we look to evaluate attribute with a regex, I was thinking maybe look to see if the string is made up with just numbers and . if not just send back the string as a whole? I thought that was better then maintaining a load of extensions

Let me know what you think

Best Regards

Gary

Contributor

username-is-already-taken2 commented Nov 8, 2016

Hi there

I thought I would take a punt last night and see if I could find where the problem lies (I'm still find my way with ruby/pry)

the registry_key resource looks to be OK IMHO, the values looks to gathered correctly and the def registry_property_value does evaluate correctly if it has the right property :)

but at this point the property is now an array ["explorer", "exe"]
digging a bit further it looks like its being done here

C:\opscode\chefdk\embedded\lib\ruby\gems\2.3.0\gems\rspec-its-1.2.0\lib\rspec\its.rb

  def its(attribute, *options, &block)
      its_caller = caller.select {|file_line| file_line !~ %r(/lib/rspec/its) }
      describe(attribute.to_s, :caller => its_caller) do
        let(:__its_subject) do
          if Array === attribute
            if Hash === subject
              attribute.inject(subject) {|inner, attr| inner[attr] }
            else
              subject[*attribute]
            end
          else
            attribute_chain = attribute.to_s.split('.')
            attribute_chain.inject(subject) do |inner_subject, attr|
              inner_subject.send(attr)
            end
          end
        end

infact looking at the comments (thank you Mr Developer ;) it would seem they are catering for phone numbers

    # The attribute can be a `Symbol` or a `String`. Given a `String`
    # with dots, the result is as though you concatenated that `String`
    # onto the subject in an expression.

Now for the hard bit working on a fix :)

should we look to evaluate attribute with a regex, I was thinking maybe look to see if the string is made up with just numbers and . if not just send back the string as a whole? I thought that was better then maintaining a load of extensions

Let me know what you think

Best Regards

Gary

@username-is-already-taken2

This comment has been minimized.

Show comment
Hide comment
@username-is-already-taken2

username-is-already-taken2 Nov 8, 2016

Contributor

Me again, I have a workaround for the time being, calling the have_property_value directly

describe registry_key ('HKLM\SOFTWARE\Policies\Microsoft\Internet Explorer\Main\FeatureControl\FEATURE_RESTRICT_ACTIVEXINSTALL') do
  it { should have_property_value('explorer.exe', :string, '1') }
end

Hope that helps

Best Regards

Gary

Contributor

username-is-already-taken2 commented Nov 8, 2016

Me again, I have a workaround for the time being, calling the have_property_value directly

describe registry_key ('HKLM\SOFTWARE\Policies\Microsoft\Internet Explorer\Main\FeatureControl\FEATURE_RESTRICT_ACTIVEXINSTALL') do
  it { should have_property_value('explorer.exe', :string, '1') }
end

Hope that helps

Best Regards

Gary

@chris-rock

This comment has been minimized.

Show comment
Hide comment
@chris-rock

chris-rock Apr 6, 2017

Member

This is related to #875. RSpec its implementation evaluates the content and tries to call a method. The solution from @username-is-already-taken2 is indeed the best one since it circumvents RSpec by calling our methods directly. I am going to add a test for that.

Member

chris-rock commented Apr 6, 2017

This is related to #875. RSpec its implementation evaluates the content and tries to call a method. The solution from @username-is-already-taken2 is indeed the best one since it circumvents RSpec by calling our methods directly. I am going to add a test for that.

chris-rock added a commit that referenced this issue Apr 6, 2017

add documentation for registry_key backslashes and #1281
Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

chris-rock added a commit that referenced this issue Apr 7, 2017

add documentation for registry_key backslashes and #1281
Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

@adamleff adamleff self-assigned this Sep 19, 2017

@arlimus arlimus removed the in progress label Sep 19, 2017

@username-is-already-taken2

This comment has been minimized.

Show comment
Hide comment
@username-is-already-taken2

username-is-already-taken2 Sep 25, 2017

Contributor

@adamleff - Hi Adam, I tried taking a punt on this one early on so I've got keen interest to see how this was resolved. Thanks for the PR :) If I'm honest I don't really understand what you did :) I can't see how you ended up knowing (or why) defining a missing method function fixes it. Would you be willing to explain?

Contributor

username-is-already-taken2 commented Sep 25, 2017

@adamleff - Hi Adam, I tried taking a punt on this one early on so I've got keen interest to see how this was resolved. Thanks for the PR :) If I'm honest I don't really understand what you did :) I can't see how you ended up knowing (or why) defining a missing method function fixes it. Would you be willing to explain?

@adamleff

This comment has been minimized.

Show comment
Hide comment
@adamleff

adamleff Sep 26, 2017

Contributor

@username-is-already-taken2 Absolutely! Sorry if I explain anything you already know. :)

The its syntax takes the contents of the argument to its and then tries to call that method chain on the subject, which in this case, is an InSpec resource instance.

So, if I had the following test:

describe my_resource do
  its('foo') { should cmp 'bar' }
end

... it would create an instance of my_resource, and then try and call foo on it (i.e. my_resource_instance.foo... and then pass that to the test comparison.

If the test was written like this:

its('foo.adam') { should cmp 'bar' }

... it would first call foo on my_resource, and then try to call adam on whatever is returned from foo.

Resources such as registry_key can't know all the different methods it would need to respond to, unlike a file resource for example, we use method_missing to catch all methods send to the registry_key resource and then look up that key in the data hash for the resource. However, because . is used to chain methods, you can't have methods with dots in them. And because registry key data can have dots in the keys, you can't use a string argument to its.

So the workaround is to use the Array syntax:

its(['foo', 'bar']) { should cmp 123 }

However, when method_missing receives more than one argument, the first being :[] to indicate an array was passed in, and then the contents of the array. Let's look at an example:

Here's my resource:

class MethodMissingTest < Inspec.resource(1)
  name 'method_missing_test'

  def method_missing(*args)
    puts args.inspect
    true
  end
end

Here's my control:

control 'foo' do
  title 'foo'
  impact 1.0
  describe method_missing_test do
    its(['foo', 'bar.with.dots']) { should be true }
  end
end

... and here's the puts output:

[:[], "foo", "bar.with.dots"]

The registry_key method_missing was written like this:

def method_missing(args)

... which will only accept one argument. I changed it to:

def method_missing(*args)

... and then added the logic to do the right thing if we received a :[] as the first argument.

I hope that helps!

Contributor

adamleff commented Sep 26, 2017

@username-is-already-taken2 Absolutely! Sorry if I explain anything you already know. :)

The its syntax takes the contents of the argument to its and then tries to call that method chain on the subject, which in this case, is an InSpec resource instance.

So, if I had the following test:

describe my_resource do
  its('foo') { should cmp 'bar' }
end

... it would create an instance of my_resource, and then try and call foo on it (i.e. my_resource_instance.foo... and then pass that to the test comparison.

If the test was written like this:

its('foo.adam') { should cmp 'bar' }

... it would first call foo on my_resource, and then try to call adam on whatever is returned from foo.

Resources such as registry_key can't know all the different methods it would need to respond to, unlike a file resource for example, we use method_missing to catch all methods send to the registry_key resource and then look up that key in the data hash for the resource. However, because . is used to chain methods, you can't have methods with dots in them. And because registry key data can have dots in the keys, you can't use a string argument to its.

So the workaround is to use the Array syntax:

its(['foo', 'bar']) { should cmp 123 }

However, when method_missing receives more than one argument, the first being :[] to indicate an array was passed in, and then the contents of the array. Let's look at an example:

Here's my resource:

class MethodMissingTest < Inspec.resource(1)
  name 'method_missing_test'

  def method_missing(*args)
    puts args.inspect
    true
  end
end

Here's my control:

control 'foo' do
  title 'foo'
  impact 1.0
  describe method_missing_test do
    its(['foo', 'bar.with.dots']) { should be true }
  end
end

... and here's the puts output:

[:[], "foo", "bar.with.dots"]

The registry_key method_missing was written like this:

def method_missing(args)

... which will only accept one argument. I changed it to:

def method_missing(*args)

... and then added the logic to do the right thing if we received a :[] as the first argument.

I hope that helps!

@username-is-already-taken2

This comment has been minimized.

Show comment
Hide comment
@username-is-already-taken2

username-is-already-taken2 Oct 9, 2017

Contributor

@adamleff - I'm sorry for not replying sooner.

I very much appreciate your response, thank you for taking to the time to explain this. I've shared your repsonse around internally so you have helped a few of us here understand ruby a bit better :)

See you at the summit tomorrow

Contributor

username-is-already-taken2 commented Oct 9, 2017

@adamleff - I'm sorry for not replying sooner.

I very much appreciate your response, thank you for taking to the time to explain this. I've shared your repsonse around internally so you have helped a few of us here understand ruby a bit better :)

See you at the summit tomorrow

@adamleff

This comment has been minimized.

Show comment
Hide comment
@adamleff

adamleff Oct 9, 2017

Contributor

My pleasure, @username-is-already-taken2! See you tomorrow!

Contributor

adamleff commented Oct 9, 2017

My pleasure, @username-is-already-taken2! See you tomorrow!

james-stocks added a commit that referenced this issue Jul 17, 2018

Generate describe code for an array of strings
Context:
When testing a Windows registry key with a period character in it e.g. `explorer.exe` it is not possible to use `its("explorer.exe")` because the period would be interpreted as method chaining.
In this case, you must instead use `its(["explorer", "exe"])`
See #1281

This commit fixes `to_ruby`in `Inspec::Describe` so that it produces an array in the generated Inspec code instead of a string.

Signed-off-by: James Stocks <jstocks@chef.io>

jquick added a commit that referenced this issue Jul 19, 2018

Generate describe code for an array of strings (#3227)
Context:
When testing a Windows registry key with a period character in it e.g. `explorer.exe` it is not possible to use `its("explorer.exe")` because the period would be interpreted as method chaining.
In this case, you must instead use `its(["explorer", "exe"])`
See #1281

This commit fixes `to_ruby`in `Inspec::Describe` so that it produces an array in the generated Inspec code instead of a string.

Signed-off-by: James Stocks <jstocks@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment