diff --git a/lib/ruby_smb.rb b/lib/ruby_smb.rb index 971b860ff..b8ff83601 100644 --- a/lib/ruby_smb.rb +++ b/lib/ruby_smb.rb @@ -1,4 +1,5 @@ require 'bindata' +require 'ruby_smb/compatibility' require 'net/ntlm' require 'net/ntlm/client' require 'openssl' diff --git a/lib/ruby_smb/compatibility.rb b/lib/ruby_smb/compatibility.rb new file mode 100644 index 000000000..bd6ccca1e --- /dev/null +++ b/lib/ruby_smb/compatibility.rb @@ -0,0 +1,37 @@ +module RubySMB + # Compatibility shims for the gems RubySMB builds on. + module Compatibility + end +end + +# BinData rejects a field name when a method of that name is already defined on +# the struct class. The check in BinData::DSLMixin::DSLFieldValidator uses +# Module#method_defined? without the second argument, so it also matches methods +# inherited from ancestors such as Object, Kernel and Hash. +# +# That is too broad in practice. When a test suite loads a mocking library that +# adds helper methods to Object (rspec-mocks defines #stub on BasicObject when +# `config.mock_with :rspec` is used, mocha defines #stubs, and so on) before +# ruby_smb is required, perfectly valid BinData definitions start raising +# SyntaxError: field 'stub' shadows an existing method +# even though no struct field actually shadows a method that the struct itself +# relies on. See https://github.com/rapid7/ruby_smb/issues/261. +# +# Field accessors are installed per instance with define_singleton_method, so an +# inherited method named like a field never gets in the way at runtime; only a +# method defined directly on the struct class is a genuine collision. Passing +# `false` to method_defined? restricts the check to methods defined directly on +# the class, which is the behaviour the guard is actually meant to enforce. +# Names that clash with Hash/BinData internals remain covered by the separate +# BinData::Struct::RESERVED list, so this does not loosen that protection. +if defined?(BinData::DSLMixin::DSLFieldValidator) + module BinData + module DSLMixin + class DSLFieldValidator + def name_shadows_method?(name) + @the_class.method_defined?(name, false) + end + end + end + end +end diff --git a/spec/lib/ruby_smb/compatibility_spec.rb b/spec/lib/ruby_smb/compatibility_spec.rb new file mode 100644 index 000000000..51c1b5af4 --- /dev/null +++ b/spec/lib/ruby_smb/compatibility_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +RSpec.describe 'BinData field name compatibility (issue #261)' do + # Regression coverage for + # https://github.com/rapid7/ruby_smb/issues/261 + # + # Loading a mocking library (rspec-mocks via `config.mock_with :rspec`) adds a + # #stub helper to Object before ruby_smb is required. BinData's field-name + # guard used method_defined? without restricting it to the struct class, so it + # picked up that inherited method and refused otherwise-valid field names such + # as :stub (RubySMB::Dcerpc::Request actually has a :stub field). + + context 'when an inherited method shares a name with a BinData field' do + before do + # Simulate what rspec-mocks/mocha do: define a helper method on Object so + # that every class, including a fresh BinData struct, inherits it. + Object.send(:define_method, :inherited_helper_261) { :from_object } + end + + after do + Object.send(:remove_method, :inherited_helper_261) + end + + it 'allows the field to be defined without raising' do + expect(Object.method_defined?(:inherited_helper_261)).to be true + + expect do + Class.new(BinData::Record) do + uint8 :inherited_helper_261 + end + end.not_to raise_error + end + end + + context 'when a field name shadows a method defined directly on the struct' do + it 'still raises, so the genuine guard is preserved' do + expect do + Class.new(BinData::Record) do + def direct_method_261; end + uint8 :direct_method_261 + end + end.to raise_error(/shadows an existing method/) + end + end + + context 'when a field name collides with a BinData reserved name' do + it 'still raises, so the RESERVED guard is preserved' do + expect do + Class.new(BinData::Record) do + uint8 :keys # Hash#keys, part of BinData::Struct::RESERVED + end + end.to raise_error(/reserved name/) + end + end +end