OWASP / railsgoat

A vulnerable version of Rails that follows the OWASP Top 10
railsgoat.cktricky.com
MIT License
867 stars 667 forks source link

Few Fixes #339

Closed hardlyhuman closed 6 years ago

hardlyhuman commented 6 years ago

Gemfile is updated to ensure successful installation of phantomJS for running the tests.

While running the test cases, command_injection_spec is going to pending, I have fixed it which ensures successful run all the time.

A justification for this PR

  1. To run tests, phantomJS is needed and there is a need for a gem installation. Hence I have added and sent a PR of the updated Gemfile to make developers life easier.

  2. Coming to the change in command_injection_spec, this particular test is not succeeding all the time. This particular issue is fixed with the proposed changes

cktricky commented 6 years ago

Hey @SriHarshaGajavalli 👋 - Thanks for submitting a PR. Can you provide some context as to the changes, their purpose, etc.?

hardlyhuman commented 6 years ago

Hello @cktricky - I have updated the description

jasnow commented 6 years ago

@SriHarshaGajavalli - How does this PR impact the phantomJS installation instructions in the README file?

jasnow commented 6 years ago

Why do you want the poltergeist gem twice in the Gemfile file?

hardlyhuman commented 6 years ago

@jasnow poltergeist gem is already existing but it is failing to get installing sometime. Hence I have added. But it is because of some issues on the local machine. However, I have removed it now.

Coming to the change command_injection_spec, everytime a test is running, it is creating a file and the test is going to pending status instead of failing (which is a successful run)

hardlyhuman commented 6 years ago

It isn't failing. Instead, it's successful execution leads to the creation of a file which in turn results in the test case status pending

jasnow commented 6 years ago
It isn't failing. Instead, it's successful execution leads to the 
creation of a file which in turn results in the test case status pending

With or without commenting out line 17?

hardlyhuman commented 6 years ago

With commenting, test case executes successfully (which means actually failed). If we uncomment the line, the test case is going to pending status.

I guess there could be bit of confusion with the concept itself

jasnow commented 6 years ago

@SriHarshaGajavalli - As you mention above:

Coming to the change in command_injection_spec, this particular test is not succeeding all the time. This particular issue is fixed with the proposed changes

Do you know why the command_injection_spec is succeeding sometimes and not others?

hardlyhuman commented 6 years ago

@jasnow : Because of the above mentioned reason, it never succeeded for me

jasnow commented 6 years ago

Could not reproduce the problem so rejecting the fix.

jasnow commented 6 years ago

@SriHarshaGajavalli - You are welcome to reopen this issue when you have more data to share.