kaitai-io / kaitai_struct

Kaitai Struct: declarative language to generate binary data parsers in C++ / C# / Go / Java / JavaScript / Lua / Nim / Perl / PHP / Python / Ruby
https://kaitai.io
4.04k stars 199 forks source link

Ruby, Lua: missing `require`s not detected by tests due to `require` leaks #1099

Open generalmimon opened 7 months ago

generalmimon commented 7 months ago

I was wondering how it is possible that the EnumImport test passes in Ruby (see https://ci.kaitai.io/ or ci.json:239-243 of the ruby/3.3-linux-x86_64 target).

The enum_import.ksy spec imports two other .ksy specs (enum_0 and enum_deep) and defines two seq fields, each of an enum type defined in one of the imported specs:

meta:
  id: enum_import
  endian: le
  imports:
    - enum_0
    - enum_deep
seq:
  - id: pet_1
    type: u4
    enum: enum_0::animal
  - id: pet_2
    type: u4
    enum: enum_deep::container1::container2::animal

The problem is that the enum_import.rb (still generated by the latest KSC at the time of writing, i.e. 29f7a592) looks like this:

# This is a generated file! Please edit source .ksy file and use kaitai-struct-compiler to rebuild

require 'kaitai/struct/struct'

unless Gem::Version.new(Kaitai::Struct::VERSION) >= Gem::Version.new('0.11')
  raise "Incompatible Kaitai Struct Ruby API: 0.11 or later is required, but you have #{Kaitai::Struct::VERSION}"
end

class EnumImport < Kaitai::Struct::Struct
  def initialize(_io, _parent = nil, _root = self)
    super(_io, _parent, _root)
    _read
  end

  def _read
    @pet_1 = Kaitai::Struct::Stream::resolve_enum(Enum0::ANIMAL, @_io.read_u4le)
    @pet_2 = Kaitai::Struct::Stream::resolve_enum(EnumDeep::Container1::Container2::ANIMAL, @_io.read_u4le)
    self
  end
  attr_reader :pet_1
  attr_reader :pet_2
end

Notice that it refers to classes Enum0 and EnumDeep on these lines:

    @pet_1 = Kaitai::Struct::Stream::resolve_enum(Enum0::ANIMAL, @_io.read_u4le)
    #                                             ^^^^^
    @pet_2 = Kaitai::Struct::Stream::resolve_enum(EnumDeep::Container1::Container2::ANIMAL, @_io.read_u4le)
    #                                             ^^^^^^^^

... but there are no corresponding require statements, so the Ruby interpreter should have no idea what these names are. And yet this test passes in Ruby.

It turns out that the requires in Ruby are "leaky", i.e. they pollute the global namespace. So if any format or test Ruby files running before the above enum_import.rb contain require 'enum_0' and require 'enum_deep' (which is what the EnumImport class needs to run successfully) in the same Ruby interpreter process, the fact that the enum_import.rb file doesn't itself have the requires it needs is not detected by tests.

And if you search for require 'enum_0' and for require 'enum_deep', you can see that both requires have 2 occurrences in test specs.

If you disable the enum_0 in spec/ruby/enum_import_spec.rb:

RSpec.describe 'EnumImport' do
  it 'parses test properly' do
    require 'enum_import'
    # require 'enum_0'
    require 'enum_deep'

... it's not enough (i.e. the EnumImport test will still pass), because enum_0 is also tested directly, so require 'enum_0' is also in spec/ruby/enum_0_spec.rb:

RSpec.describe 'Enum0' do
  it 'parses test properly' do
    # require 'enum_0'

EnumImport will finally fail (as it should have all this time) only after you disable both these requires:

$ ./run-ruby

  ...

  3) EnumImport parses test properly
     Failure/Error: r = EnumImport.from_file('src/enum_0.bin')

     NameError:
       uninitialized constant EnumImport::Enum0
     # ./compiled/ruby/enum_import.rb:16:in `_read'
     # ./compiled/ruby/enum_import.rb:12:in `initialize'
     # C:/temp/kaitai_struct/runtime/ruby/lib/kaitai/struct/struct.rb:26:in `new'
     # C:/temp/kaitai_struct/runtime/ruby/lib/kaitai/struct/struct.rb:26:in `from_file'
     # ./spec/ruby/enum_import_spec.rb:8:in `block (2 levels) in <top (required)>'
generalmimon commented 7 months ago

The same problem occurs in Lua - enum_import.lua also has no require("enum_0") or require("enum_deep") but refers to Enum0 and EnumDeep:

-- This is a generated file! Please edit source .ksy file and use kaitai-struct-compiler to rebuild
--
-- This file is compatible with Lua 5.3

local class = require("class")
require("kaitaistruct")

EnumImport = class.class(KaitaiStruct)

function EnumImport:_init(io, parent, root)
  KaitaiStruct._init(self, io)
  self._parent = parent
  self._root = root or self
  self:_read()
end

function EnumImport:_read()
  self.pet_1 = Enum0.Animal(self._io:read_u4le())
  self.pet_2 = EnumDeep.Container1.Container2.Animal(self._io:read_u4le())
end

Unlike Ruby, test_enum_import.lua only contains require("enum_import") and not require("enum_0") (which is correct since none of the asserts refer to Enum0, see https://github.com/kaitai-io/kaitai_struct_tests/pull/109). But like in Ruby, the require("enum_0") is obviously present in test_enum_0.lua:

-- Autogenerated from KST: please remove this line if doing any edits by hand!

local luaunit = require("luaunit")

-- require("enum_0")

TestEnum0 = {}

function TestEnum0:test_enum_0()

... and as long as it present, EnumImport passes in Lua despite the missing require("enum_0") in compiled/lua/enum_import.lua. Only if you disable it as above, then the EnumImport test finally breaks:

  • 🔴 2 tests broken:
    • Enum0: (...)
    • EnumImport:
    • {"status"=>"passed"}
    • {"status"=>"failed",
    • "failure"=>
    • {"file_name"=>nil,
    • "line_num"=>nil,
    • "message"=>
    • "compiled/lua/enum_import.lua:18: attempt to index a nil value (global 'Enum0')",
    • "trace"=>
    • "stack traceback:\n" +
    • "\tcompiled/lua/enum_import.lua:18: in method '_read'\n" +
    • "\tcompiled/lua/enum_import.lua:14: in local 'init'\n" +
    • "\t../runtime/lua/class.lua:70: in function <../runtime/lua/class.lua:66>\n" +
    • "\t(...tail calls...)\n" +
    • "\tspec/lua/test_enum_import.lua:10: in upvalue 'TestEnumImport.test_enum_import'"}}