Skip to content
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

KST: Fix too small input file for expr_io_ternary test #118

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Apr 11, 2024

The test requires 1 + 4 + 8 = 13 bytes, but term_strz.bin used as input contains only 12 bytes. Strange, but Ruby test was successfully passed -- it seems Ruby runtime has a bug in substream implementation. According to that test:
https://github.com/kaitai-io/kaitai_struct_ruby_runtime/blob/ecf67519a990296bb1e095363ef2dc414e600502/spec/subio_spec.rb#L70-L76
is it normal for Ruby to be able to read from a stream more that it contains successfully?

This commit replaces too small term_strz.bin file with if_struct.bin which size is 13 bytes.

The test was introduced in https://github.com/kaitai-io/kaitai_struct_tests/commits/dceaee2ebf9eb05177e6f5d0effd1b1b907be9ac

cc @GreyCat

The test requires 1 + 4 + 8 = 13 bytes, but term_strz.bin contains only 12 bytes.
Strange, but Ruby test was successfully passed.

This commit replaces too small `term_strz.bin` file with `if_struct.bin` which size is 13 bytes.
@generalmimon
Copy link
Member

Strange, but Ruby test was successfully passed -- it seems Ruby runtime has a bug in substream implementation.

Looks like it, that's why I added the {Eof,Eos}ExceptionSized tests in 99f9229.

@generalmimon
Copy link
Member

@Mingun:

According to that test:
https://github.com/kaitai-io/kaitai_struct_ruby_runtime/blob/ecf67519a990296bb1e095363ef2dc414e600502/spec/subio_spec.rb#L70-L76
is it normal for Ruby to be able to read from a stream more that it contains successfully?

Well, it is, just as it is for StringIO or any other IO from the Ruby stdlib. If you look closely at the test you're linking to, it asserts that Kaitai::Struct::SubIO behaves the same as the Ruby built-in class StringIO in various cases.

Note that Kaitai::Struct::SubIO is not a class that any code outside the runtime library should directly interact with. It is only used internally in the substream method lib/kaitai/struct/struct.rb:529-533:

  def substream(n)
    sub = Stream.new(SubIO.new(@_io, @_io.pos, n))
    @_io.seek(@_io.pos + n)
    sub
  end

The problem is not with the internal Kaitai::Struct::SubIO#read method (as long it works the same as the native read methods would, it's fine), but that the substream method or the Kaitai::Struct::SubIO constructor lack any checks that we're not creating a substream that exceeds the parent stream size.

@Mingun
Copy link
Contributor Author

Mingun commented May 1, 2024

@GreyCat , @generalmimon, what the status?

Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mingun LGTM, thanks!

@generalmimon generalmimon merged commit 390e0ee into kaitai-io:master Jul 9, 2024
@Mingun Mingun deleted the fix-expr_io_ternary branch July 9, 2024 10:48
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.

2 participants