Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Call IO.popen instead of backticks #6036

Merged
merged 4 commits into from
Oct 4, 2017

Conversation

nobu
Copy link
Contributor

@nobu nobu commented Sep 17, 2017

IO.popen with the command in an array doesn't need command line
quotes, and is safer.

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

What was the end-user problem that led to this PR?

Back-ticks with command line quoting is dangerous.

What was your diagnosis of the problem?

Back-ticks is not for complex purpose.

What is your fix for the problem, implemented in this PR?

Use IO.popen and redirection option instead.

Why did you choose this fix out of the possible options?

IO.popen and options of spawn have been introduced for such purpose.

IO.popen with the command in an array doesn't need command line
quotes, and is safer.
@ghost
Copy link

ghost commented Sep 17, 2017

Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack.

For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide

@olleolleolle
Copy link
Member

@nobu Thanks for this change!

The CI build fails on a single style check: seems like %i-literals ( %i[child out]) need to be lists of symbols instead ([:child, :out]).

@colby-swandale
Copy link
Member

I've created #6039 which sets Rubocop to target ruby version 2.3 which runs the rubocop parser correctly for this PR. If you can please wait for that to be merged.

@nobu nobu force-pushed the feature/sh-by-popen branch 2 times, most recently from 2f4fb5b to ab73431 Compare September 19, 2017 02:27
@nobu nobu force-pushed the feature/sh-by-popen branch from ab73431 to ef2471a Compare September 19, 2017 04:47
@indirect
Copy link
Member

indirect commented Oct 3, 2017

@bundlerbot r+

@bundlerbot
Copy link
Collaborator

📌 Commit ef2471a has been approved by indirect

@bundlerbot
Copy link
Collaborator

⌛ Testing commit ef2471a with merge 89d3cdb...

bundlerbot added a commit that referenced this pull request Oct 3, 2017
Call IO.popen instead of backticks

IO.popen with the command in an array doesn't need command line
quotes, and is safer.

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

### What was the end-user problem that led to this PR?

Back-ticks with command line quoting is dangerous.

### What was your diagnosis of the problem?

Back-ticks is not for complex purpose.

### What is your fix for the problem, implemented in this PR?

Use `IO.popen` and redirection option instead.

### Why did you choose this fix out of the possible options?

`IO.popen` and options of `spawn` have been introduced for such purpose.
@bundlerbot
Copy link
Collaborator

💔 Test failed - status-travis

@segiddins
Copy link
Member

@bundlerbot retry

@bundlerbot
Copy link
Collaborator

⌛ Testing commit ef2471a with merge 4a53b53...

bundlerbot added a commit that referenced this pull request Oct 4, 2017
Call IO.popen instead of backticks

IO.popen with the command in an array doesn't need command line
quotes, and is safer.

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

### What was the end-user problem that led to this PR?

Back-ticks with command line quoting is dangerous.

### What was your diagnosis of the problem?

Back-ticks is not for complex purpose.

### What is your fix for the problem, implemented in this PR?

Use `IO.popen` and redirection option instead.

### Why did you choose this fix out of the possible options?

`IO.popen` and options of `spawn` have been introduced for such purpose.
@bundlerbot
Copy link
Collaborator

☀️ Test successful - status-travis
Approved by: indirect
Pushing 4a53b53 to master...

@bundlerbot bundlerbot merged commit ef2471a into rubygems:master Oct 4, 2017
bundlerbot added a commit that referenced this pull request Aug 11, 2018
Fix SystemStackError (stack level too deep) in GemHelper#sh_with_code

Hello. I found a method which raises SystemStackError by chance.

### What was the end-user problem that led to this PR?

The method always calls itself recursively like as follows:

```
irb(main):006:0> require './lib/bundler/gem_helper'
=> true
irb(main):007:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
Traceback (most recent call last):
       16: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       15: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       14: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       13: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       12: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       11: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       10: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        9: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        8: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        7: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        6: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        5: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        4: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        3: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        2: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        1: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
SystemStackError (stack level too deep)
```

### What was your diagnosis of the problem?

I think that it can be supposed that `sh_with_status` should be called rather than `sh_with_code` because Process::Status#exitstatus is called later on.
(related to #6036)

After this changes, the result of calling the method is as follows:

```
irb(main):011:0> load './lib/bundler/gem_helper.rb'
=> true
irb(main):012:0>
irb(main):013:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
=> ["bin\nbundler.gemspec\nCHANGELOG.md\nCODE_OF_CONDUCT.md\ndoc\nexe\nGemfile\nGemfile.lock\nlib\nLICENSE.md\nman\nRakefile\nREADME.md\nspec\ntask\n", 0]
```

I am not sure if I could remove the method since I cannot have confidence that the method is not used anymore. So I simply fixed it.
bundlerbot added a commit that referenced this pull request Aug 12, 2018
Fix SystemStackError (stack level too deep) in GemHelper#sh_with_code

Hello. I found a method which raises SystemStackError by chance.

### What was the end-user problem that led to this PR?

The method always calls itself recursively like as follows:

```
irb(main):006:0> require './lib/bundler/gem_helper'
=> true
irb(main):007:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
Traceback (most recent call last):
       16: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       15: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       14: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       13: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       12: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       11: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       10: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        9: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        8: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        7: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        6: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        5: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        4: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        3: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        2: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        1: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
SystemStackError (stack level too deep)
```

### What was your diagnosis of the problem?

I think that it can be supposed that `sh_with_status` should be called rather than `sh_with_code` because Process::Status#exitstatus is called later on.
(related to #6036)

After this changes, the result of calling the method is as follows:

```
irb(main):011:0> load './lib/bundler/gem_helper.rb'
=> true
irb(main):012:0>
irb(main):013:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
=> ["bin\nbundler.gemspec\nCHANGELOG.md\nCODE_OF_CONDUCT.md\ndoc\nexe\nGemfile\nGemfile.lock\nlib\nLICENSE.md\nman\nRakefile\nREADME.md\nspec\ntask\n", 0]
```

I am not sure if I could remove the method since I cannot have confidence that the method is not used anymore. So I simply fixed it.
bundlerbot added a commit that referenced this pull request Aug 12, 2018
Fix SystemStackError (stack level too deep) in GemHelper#sh_with_code

Hello. I found a method which raises SystemStackError by chance.

### What was the end-user problem that led to this PR?

The method always calls itself recursively like as follows:

```
irb(main):006:0> require './lib/bundler/gem_helper'
=> true
irb(main):007:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
Traceback (most recent call last):
       16: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       15: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       14: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       13: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       12: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       11: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       10: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        9: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        8: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        7: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        6: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        5: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        4: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        3: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        2: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        1: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
SystemStackError (stack level too deep)
```

### What was your diagnosis of the problem?

I think that it can be supposed that `sh_with_status` should be called rather than `sh_with_code` because Process::Status#exitstatus is called later on.
(related to #6036)

After this changes, the result of calling the method is as follows:

```
irb(main):011:0> load './lib/bundler/gem_helper.rb'
=> true
irb(main):012:0>
irb(main):013:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
=> ["bin\nbundler.gemspec\nCHANGELOG.md\nCODE_OF_CONDUCT.md\ndoc\nexe\nGemfile\nGemfile.lock\nlib\nLICENSE.md\nman\nRakefile\nREADME.md\nspec\ntask\n", 0]
```

I am not sure if I could remove the method since I cannot have confidence that the method is not used anymore. So I simply fixed it.
bundlerbot added a commit that referenced this pull request Aug 15, 2018
Fix SystemStackError (stack level too deep) in GemHelper#sh_with_code

Hello. I found a method which raises SystemStackError by chance.

### What was the end-user problem that led to this PR?

The method always calls itself recursively like as follows:

```
irb(main):006:0> require './lib/bundler/gem_helper'
=> true
irb(main):007:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
Traceback (most recent call last):
       16: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       15: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       14: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       13: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       12: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       11: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       10: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        9: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        8: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        7: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        6: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        5: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        4: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        3: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        2: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        1: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
SystemStackError (stack level too deep)
```

### What was your diagnosis of the problem?

I think that it can be supposed that `sh_with_status` should be called rather than `sh_with_code` because Process::Status#exitstatus is called later on.
(related to #6036)

After this changes, the result of calling the method is as follows:

```
irb(main):011:0> load './lib/bundler/gem_helper.rb'
=> true
irb(main):012:0>
irb(main):013:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
=> ["bin\nbundler.gemspec\nCHANGELOG.md\nCODE_OF_CONDUCT.md\ndoc\nexe\nGemfile\nGemfile.lock\nlib\nLICENSE.md\nman\nRakefile\nREADME.md\nspec\ntask\n", 0]
```

I am not sure if I could remove the method since I cannot have confidence that the method is not used anymore. So I simply fixed it.
bundlerbot added a commit that referenced this pull request Aug 15, 2018
Fix SystemStackError (stack level too deep) in GemHelper#sh_with_code

Hello. I found a method which raises SystemStackError by chance.

### What was the end-user problem that led to this PR?

The method always calls itself recursively like as follows:

```
irb(main):006:0> require './lib/bundler/gem_helper'
=> true
irb(main):007:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
Traceback (most recent call last):
       16: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       15: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       14: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       13: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       12: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       11: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
       10: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        9: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        8: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        7: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        6: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        5: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        4: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        3: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        2: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
        1: from /home/hibariya/src/github.com/bundler/bundler/lib/bundler/gem_helper.rb:191:in `sh_with_code'
SystemStackError (stack level too deep)
```

### What was your diagnosis of the problem?

I think that it can be supposed that `sh_with_status` should be called rather than `sh_with_code` because Process::Status#exitstatus is called later on.
(related to #6036)

After this changes, the result of calling the method is as follows:

```
irb(main):011:0> load './lib/bundler/gem_helper.rb'
=> true
irb(main):012:0>
irb(main):013:0> Bundler::GemHelper.new.send(:sh_with_code, 'ls')
=> ["bin\nbundler.gemspec\nCHANGELOG.md\nCODE_OF_CONDUCT.md\ndoc\nexe\nGemfile\nGemfile.lock\nlib\nLICENSE.md\nman\nRakefile\nREADME.md\nspec\ntask\n", 0]
```

I am not sure if I could remove the method since I cannot have confidence that the method is not used anymore. So I simply fixed it.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants