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

Entering a literal null byte causes a crash and subsequent session history-load errors #1789

Closed
owst opened this Issue Oct 9, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@owst
Contributor

owst commented Oct 9, 2018

On Ubuntu 16.04 x64, with xterm and Pry version 0.11.3 on Ruby 2.4.4 I accidentally entered a literal null byte when typing a string value in pry (by hitting Ctrl-space) - pry immediately crashed, and when I reload it, refuses to load the history as it contains a null byte:

(I held ctrl while first typing the space below, so it appeared as if I hadn't added one)

$ pry             
[1] pry(main)> x = 'foo bar'
~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/history.rb:117:in `<<': string contains null byte (ArgumentError)
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/history.rb:117:in `push_to_readline'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/history.rb:48:in `call'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/history.rb:48:in `push'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_instance.rb:267:in `handle_line'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_instance.rb:243:in `block (2 levels) in eval'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_instance.rb:242:in `catch'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_instance.rb:242:in `block in eval'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_instance.rb:241:in `catch'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_instance.rb:241:in `eval'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/repl.rb:77:in `block in repl'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/repl.rb:67:in `loop'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/repl.rb:67:in `repl'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/repl.rb:38:in `block in start'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/input_lock.rb:61:in `__with_ownership'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/input_lock.rb:79:in `with_ownership'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/repl.rb:38:in `start'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/repl.rb:13:in `start'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/pry_class.rb:192:in `start'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-byebug-3.6.0/lib/pry-byebug/pry_ext.rb:11:in `start_with_pry_byebug'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/lib/pry/cli.rb:116:in `start'
from ~/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/pry-0.11.3/bin/pry:12:in `<top (required)>'
from ~/.rbenv/versions/2.4.4/bin/pry:23:in `load'
from ~/.rbenv/versions/2.4.4/bin/pry:23:in `<main>'

and now, pry apparently refuses to load my history:

$ pry
History file not loaded: string contains null byte
[1] pry(main)>     

On master as of today (ef80932bc66ca8a03f970d24280a77af30293489) pry crashes rather than reporting that it can't load history:

$ bundle exec bin/pry
bundler: failed to load command: bin/pry (bin/pry)
ArgumentError: string contains null byte
  ~/code/pry/lib/pry/history.rb:117:in `<<'
  ~/code/pry/lib/pry/history.rb:117:in `push_to_readline'
  ~/code/pry/lib/pry/history.rb:37:in `call'
  ~/code/pry/lib/pry/history.rb:37:in `block in load'
  ~/code/pry/lib/pry/history.rb:108:in `block in read_from_file'
  ~/code/pry/lib/pry/history.rb:108:in `foreach'
  ~/code/pry/lib/pry/history.rb:108:in `read_from_file'
  ~/code/pry/lib/pry/history.rb:36:in `call'
  ~/code/pry/lib/pry/history.rb:36:in `load'
  ~/code/pry/lib/pry/pry_class.rb:243:in `load_history'
  ~/code/pry/lib/pry/pry_class.rb:147:in `final_session_setup'
  ~/code/pry/lib/pry/cli.rb:84:in `parse_options'
  bin/pry:11:in `<top (required)>'

owst added a commit to bambooengineering/pry that referenced this issue Oct 9, 2018

Gracefully handle (ignore) null bytes in history lines
Fixes pry#1789.

Readline is unable to add lines to its history that contain a null byte;
we should therefore avoid saving such lines to the history file, and
ignore any such lines that are already present in the file.
@kyrylo

This comment has been minimized.

Member

kyrylo commented Oct 10, 2018

Can you please help me reproduce this issue? I am trying:

% echo $'foo\0bar' >> ~/.pry_history
% pry
[1] pry(main)>

I am on OSX, so I Ctrl-Space doesn't work for me.

@owst

This comment has been minimized.

Contributor

owst commented Oct 10, 2018

Interesting - on my MacOS laptop, with iTerm2 (3.2.3) I can reproduce using your example:

~ pry
[1] pry(main)>
~ echo $'foo\0bar' >> ~/.pry_history
~ pry
History file not loaded: string contains null byte
[1] pry(main)>
~ tail -n-1 ~/.pry_history | xxd -
00000000: 666f 6f00 6261 720a                      foo.bar.
~ pry --version
Pry version 0.11.3 on Ruby 2.4.4

Just to check: are you running on the branch of my PR?

@kyrylo

This comment has been minimized.

Member

kyrylo commented Oct 10, 2018

I'm running pry 0.11.3 on iTerm2 3.1.6, will try to upgrade iTerm.

~% echo $'foo\0bar' >> ~/.pry_history
~% pry
zsh: command not found: pry
~% gem i pry
Fetching: pry-0.11.3.gem (100%)
Successfully installed pry-0.11.3
1 gem installed
~% pry
zsh: correct 'pry' to 'pr' [nyae]? n
[1] pry(main)>
~% cat ~/.pry_history | xxd -
00000000: 666f 6f00 6261 720a                      foo.bar.
~% pry --version
Pry version 0.11.3 on Ruby 2.4.2
@kyrylo

This comment has been minimized.

Member

kyrylo commented Oct 10, 2018

Upgraded iTerm2 to 3.2.3. Still nothing.

What's your Readline version? I'm on 7.0:

[1] pry(main)> Readline::VERSION
=> "7.0"
@owst

This comment has been minimized.

Contributor

owst commented Oct 10, 2018

[...]
~% cat ~/.pry_history | xxd -
00000000: 666f 6f00 6261 720a                      foo.bar.
[...]

So the NULL byte is there - but you're not seeing the error. Which version of Readline do you have?

I have:

$ pry
History file not loaded: string contains null byte
[1] pry(main)> Readline::VERSION
=> "7.0"
@kyrylo

This comment has been minimized.

Member

kyrylo commented Oct 10, 2018

I don't know if this is of any importance but I'm on High Sierra. 😁

@owst

This comment has been minimized.

Contributor

owst commented Oct 10, 2018

It's a change in Ruby 2.4.2 => Ruby 2.4.4 (I missed that you were on a different version of Ruby!)): specifically, 2.4.4 includes this change.

After that change, hist_push (i.e. Readline::HISTORY#<<) uses OutputStringValue, which was changed to use StringValueCStr, which is

#define StringValueCStr(v) rb_string_value_cstr(&(v))

Finally, rb_string_value_cstr raises if the underlying C-string contains a NULL byte

@owst

This comment has been minimized.

Contributor

owst commented Oct 10, 2018

Just to confirm, if I use Ruby 2.4.2, I also don't see the warning about a NULL byte - but, as expected due to the way C strings are handled, my history entry is truncated at the first NULL byte:

> ruby --version
ruby 2.4.2p198 (2017-09-14 revision 59899) [x86_64-darwin17]
> tail -n-1 ~/.pry_history | xxd -
00000000: 666f 6f00 6261 720a                      foo.bar.
> pry # Then hit up to see last history entry...
[1] pry(main)> foo
@kyrylo

This comment has been minimized.

Member

kyrylo commented Oct 12, 2018

Thanks for digging into this. I did notice the Ruby version mismatch we have but neglected it (thought it was just a TEENY version difference, so no problems here).

@kyrylo kyrylo closed this in #1790 Oct 12, 2018

kyrylo added a commit that referenced this issue Oct 12, 2018

kyrylo added a commit that referenced this issue Oct 15, 2018

kyrylo added a commit that referenced this issue Oct 17, 2018

kyrylo added a commit that referenced this issue Oct 19, 2018

kyrylo added a commit that referenced this issue Oct 19, 2018

kyrylo added a commit that referenced this issue Oct 19, 2018

kyrylo added a commit that referenced this issue Oct 20, 2018

kyrylo added a commit that referenced this issue Oct 21, 2018

kyrylo added a commit that referenced this issue Oct 28, 2018

kyrylo added a commit that referenced this issue Oct 28, 2018

kyrylo added a commit that referenced this issue Nov 1, 2018

kyrylo added a commit that referenced this issue Nov 3, 2018

kyrylo added a commit that referenced this issue Nov 3, 2018

kyrylo added a commit that referenced this issue Nov 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment