From 0dbfe3a8d52937645d4cd0626b327b3777fffa8d Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Thu, 4 Jun 2026 07:24:16 +0900 Subject: [PATCH] Ignore inherited methods in BinData field-name guard (#261) BinData rejects a field name when a method of that name is already defined on the struct class. The check in DSLFieldValidator used Module#method_defined? without the second argument, so it also matched methods inherited from ancestors such as Object, Kernel and Hash. When a mocking library is loaded before ruby_smb (for example rspec-mocks adds Object#stub under config.mock_with :rspec), valid BinData definitions started raising SyntaxError: field 'stub' shadows an existing method even though RubySMB::Dcerpc::Request legitimately defines a :stub field. Field accessors are installed per instance with define_singleton_method, so an inherited method named like a field never collides at runtime; only a method defined directly on the struct class is a real collision. Passing false to method_defined? restricts the guard to directly-defined methods. Names that clash with Hash/BinData internals stay covered by the separate BinData::Struct::RESERVED list, so that protection is unchanged. The fix is a small compatibility shim required right after bindata, plus a regression spec covering the inherited-method, direct-method and reserved-name cases. Fixes #261 Signed-off-by: Arpit Jain --- lib/ruby_smb.rb | 1 + lib/ruby_smb/compatibility.rb | 37 +++++++++++++++++ spec/lib/ruby_smb/compatibility_spec.rb | 55 +++++++++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 lib/ruby_smb/compatibility.rb create mode 100644 spec/lib/ruby_smb/compatibility_spec.rb 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