Skip to content

Conversation

@mame
Copy link
Member

@mame mame commented Aug 24, 2021

Currently, RubyVM::AST.of attempts to reopen a file based on iseq's file
path. However, it opens a wrong file if the iseq's file path is spoofed.

module Dummy
  binding.irb
end
$ ruby test.rb

From: test.rb @ line 2 :

    1: module Dummy
 => 2:   binding.irb
    3: end

irb(Dummy):001:0> foo
/home/mame/work/ruby/local/lib/ruby/3.1.0/error_highlight/base.rb:412:in `spot_colon2': undefined method `last_lineno' for nil:NilClass (NoMethodError)

      if nd_parent.last_lineno == @node.last_lineno
                  ^^^^^^^^^^^^

Found by @kateinoigakukun

Though this is a fundamental design issue of RubyVM::AST.of,
in any way it would be good to make error_highlight robust for
unexpected exception.

In a long term, this particular issue would be fixed by adding to irb an
option RubyVM::ISeq.keep_script_lines = true (which is not implemented
yet).

mame added 2 commits August 24, 2021 14:43
Currently, RubyVM::AST.of attempts to reopen a file based on iseq's file
path. However, it opens a wrong file if the iseq's file path is spoofed.

```
module Dummy
  binding.irb
end
```

```
$ ruby test.rb

From: test.rb @ line 2 :

    1: module Dummy
 => 2:   binding.irb
    3: end

irb(Dummy):001:0> foo
/home/mame/work/ruby/local/lib/ruby/3.1.0/error_highlight/base.rb:412:in `spot_colon2': undefined method `last_lineno' for nil:NilClass (NoMethodError)

      if nd_parent.last_lineno == @node.last_lineno
                  ^^^^^^^^^^^^
```

Found by @kateinoigakukun

Though this is a fundamental design issue of RubyVM::AST.of,
in any way it would be good to make error_highlight robust for
unexpected exception.

In a long term, this particular issue would be fixed by adding to irb an
option `RubyVM::ISeq.keep_script_lines = true` (which is not implemented
yet).
@mame
Copy link
Member Author

mame commented Jan 4, 2022

This issue was fixed in the side of the interpreter. I just merge the test.

Just for record: Maybe this issue is related to #16.

@mame mame merged commit f3626b9 into ruby:master Jan 4, 2022
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.

1 participant