Error line reporting in backtraces

[AlexEscalante]

Hello, and such good code you have here!

I have a small problem with some code I am playing with, copied and pasted from your examples, but with an intentional typo:

begin
  s.run(priv, 'puts "here we go"
  x = X.new
  x.foo
  x.bar

  x.xprivileged_operation # FAIL
  ', source: '<user code>')
rescue SecurityError => e  
  puts "privileged_operation failed due security error:\n#{e.to_s}\n"
rescue Exception => e
  puts "backtrace:\n#{e.to_s}\n"
  e.backtrace.each { |l| puts l }
end

Seems to me like the offending line number there is line 6, but it gets reported as 12:

undefined method `xprivileged_operation' for class `X'
...
<user code>:12:in `<module:SandboxBasenamespace96278636>'
<user code>:1:in `<top (required)>'

I see a pattern on my tests: the line counter gets the lines double and inits at 2, not at zero. And then it ignores blank lines and comments lines...

[tario]

Thanks for the report,. Line numbers reported on exception backtrace is an issue hard to fix, Maybe I should add it to the list of known issues on the README and docs

The key problem here is: the code is parsed (this imply removing ALL the comments and significant information about identation, lines, etc...) and emulated using ruby, the information destroyed in the first step of the process can't be regenerated so the line number could and will not match

The only way to fix this issue, is to use a parser that does not destroy that information (I hear suggestions)

For the moment, I will leave this issue open until a solution appears, but I consider it very unlikely

[AlexEscalante]

I was afraid something like that could be the case. I was looking at the code. I think it could be important for a number of scenarios where you receive multiline input from the user and need to report back for errors.

If you think something can be done, give me a pointer and I will give it a shot.

Thanks anyway, Alex

[tario]

There is no hotfix or easy way to implement this feature, I would try to find a way to preserve the file line information when parsing using RubyParser, and if this doesn't work, then search for another gem to parse the ruby code

[tario]

There is a hope, but the solution will requiere complete redesign of ruby emulator (partialruby gem) to use the YARV parser instead of RubyParser (YARV parser keeps line number information), This also will improve performance but there will no longer support for Ruby 1.8 (at this point I think it doesn't matters) since YARV parser it's only available on Ruby 1.9 and later (e.g. Ruby 2.0)

The next big work for this gem will be that refactor (this work will be on partialruby gem)

Mental notes

String with ruby code -> YARV -> Ruby s-exp with line info -> Evalhook filters -> Modified ruby s-exp -> Ruby code generation with line numbers

[tario]

You cannot translate YARV to Ruby s-exp, the only way to do this is reimplement evalhook filters to handle YARV code instead of Ruby sexp, and allow partialruby to emulate YARV code

[tario]

There is no need to convert to YARV to implement this feature, since RubyParser comes with line tracking, we can use the info from it

tree = RubyParser.new.parse "print 'hello'\nprint 'world'\n"
 => s(:block, s(:call, nil, :print, s(:str, "hello")), s(:call, nil, :print, s(:str, "world"))) 
tree[1].line
 => 1 
tree[2].line
 => 2 

The other remaining part of the solution is to use this information and encode it on the generated ruby code, all of this work will be mainly, but probably not limited to, partialruby gem https://github.com/tario/partialruby

@AlexEscalante if you want to help with this, you can write Unit tests defining as you think how the sandbox should handle line numbers and how the line numbers on exception should be returned (I cannot grant write an implementation passing all your tests, but these tests can be useful to guide me on what feature in code are trying to implement with this)

[AlexEscalante]

I read somewhere that line numbers as reported by RubyParser are not accurate:

https://github.com/seattlerb/ruby_parser

* Known Issue: line number values can be slightly off. Parsing LR sucks.

Sadly, I don't know when I will be able to work on this, but as soon as I can I will be happy to help.

[tario]

For the moment this is the best we can get, if the line numbers reported by RubyParser are wrong, we could fork RubyParser to fix any issue about line numbers

Another option is to write a parser from scratch, but this would take to long and will be error prone, I would not