Permalink
Browse files

Don't allow false-y arguments to redirects

It's too easy for an accidentally-nil value to result in Rubysh.<(val)
changing from a file redirect to a descriptor capture.
  • Loading branch information...
1 parent c9db374 commit 314d0ec46b0f3f1301f96d41650e5c78f24147be @gdb committed Jan 10, 2014
Showing with 42 additions and 23 deletions.
  1. +29 −23 lib/rubysh.rb
  2. +13 −0 test/unit/lib/rubysh.rb
View
@@ -128,46 +128,32 @@ def self.stderr
FD.new(2)
end
- def self.>(target=nil, opts=nil)
- # Might want to DRY this logic up at some point. Right now seems
- # like it'd just sacrifice clarity though.
- if !opts && target.kind_of?(Hash)
- opts = target
- target = nil
- end
+ def self.>(target=(t=true; nil), opts=(o=true; nil))
+ target, opts = handle_redirect_args(target, t, opts, o)
target ||= :stdout
Redirect.new(1, '>', target, opts)
end
- def self.>>(target=nil, opts=nil)
- if !opts && target.kind_of?(Hash)
- opts = target
- target = nil
- end
+ def self.>>(target=(t=true; nil), opts=(o=true; nil))
+ target, opts = handle_redirect_args(target, t, opts, o)
target ||= :stdout
Redirect.new(1, '>>', target, opts)
end
- def self.<(target=nil, opts=nil)
- if !opts && target.kind_of?(Hash)
- opts = target
- target = nil
- end
+ def self.<(target=(t=true; nil), opts=(o=true; nil))
+ target, opts = handle_redirect_args(target, t, opts, o)
target ||= :stdin
Redirect.new(0, '<', target, opts)
end
# Hack to implement <<<
- def self.<<(fd=nil, opts=nil)
- if !opts && fd.kind_of?(Hash)
- opts = fd
- fd = nil
- end
-
+ def self.<<(fd=(f=true; nil), opts=(o=true; nil))
+ fd, opts = handle_redirect_args(fd, f, opts, o)
fd ||= FD.new(0)
+
TripleLessThan::Shell.new(fd, opts)
end
@@ -189,4 +175,24 @@ def self.assert(fact, msg, hard=false)
log.error(formatted)
raise msg if hard
end
+
+ def self.handle_redirect_args(target, target_omitted, opts, opts_omitted)
+ if opts_omitted && target.kind_of?(Hash)
+ # Shift over if user provided target as a hash but omitted opts.
+ opts = target
+ opts_omitted = target_omitted
+
+ target = nil
+ target_omitted = true
+ end
+
+ # User provided a false-y value for target. This probably
+ # indicates a bug in the user's code, where a variable is
+ # accidentally nil.
+ if !target_omitted && !target
+ raise Rubysh::Error::BaseError.new("You provided #{target.inspect} as your redirect target. This probably indicates a bug in your code. Either omit the target argument or provide a non-false-y value for it.")
+ end
+
+ return target, opts
+ end
end
@@ -69,6 +69,19 @@ class RubyshTest < UnitTest
assert_equal([Rubysh::Redirect.new(Rubysh::FD.new(2), '>', Rubysh::FD.new(1))], subprocess.directives)
end
end
+
+ # Rubysh('ls', '/tmp', Rubysh.stderr > Rubysh.stdout)
+ describe 'with a redirection' do
+ it 'raises if the argument is nil' do
+ assert_raises(Rubysh::Error::BaseError) {Rubysh.<(nil)}
+ assert_raises(Rubysh::Error::BaseError) {Rubysh.<(nil, :foo => :bar)}
+ end
+
+ it 'allows omitting target' do
+ Rubysh.<()
+ Rubysh.<(:foo => :bar)
+ end
+ end
end
end
end

0 comments on commit 314d0ec

Please sign in to comment.