Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Properly escape user input by using Open3 capture methods #702

Merged
merged 8 commits into from
Oct 23, 2020
Merged

Properly escape user input by using Open3 capture methods #702

merged 8 commits into from
Oct 23, 2020

Conversation

ericfranz
Copy link
Contributor

@ericfranz ericfranz commented Oct 23, 2020 •
edited
Loading

Also remove 'active_users' from developer interface which was broken as of Passenger 6 (and not really used anyways).

@ericfranz
Copy link
Contributor Author

Looks like Open3.capture methods do not like spaces in the arguments.

These all fail

# these fail with unknown option, rsync error: syntax or usage error
o,e,s = Open3.capture3 'rsync', "-r --exclude='.svn' --exclude='.git' --exclude='.gitignore' --filter=':- .gitignore'", 'foo/','bar'
o,e,s = Open3.capture3 'rsync', "-r --exclude='.svn' --exclude='.git' --exclude='.gitignore'", 'foo/','bar'

# these fail with unknown filter rule
o,e,s = Open3.capture3 'rsync', '-r', "--exclude='.svn'", "--exclude='.git'", "--exclude='.gitignore'", "--filter=':- .gitignore'", 'foo/','bar'
o,e,s = Open3.capture3 'rsync', '-r', "--exclude='.svn'", "--exclude='.git'", "--exclude='.gitignore'", "--filter=':-", ".gitignore'", 'foo/','bar'

# this succeeds
o,e,s = Open3.capture3 'rsync', '-r', "--exclude='.svn'", "--exclude='.git'", "--exclude='.gitignore'", 'foo/','bar'

@ericfranz
myjobs: fix rsync by reverting to backticks
35b13e4 
Open3.capture methods don't appear to deal well with arguments with spaces
so revert to backticks with Shellwords.escape for escaping the values

added tests to verify the happy trail as well
@ericfranz
myjobs: fix error reporting in new template form
c633f3d 
in controller, move bulk of the create block into begin/rescue
and add base error message for any problems

check for rsync error and set to path attribute if error message present

display all error messages as bullet list at top of form

set focus to top of error list of no input valid
@ericfranz
Revert "skip broken tests"
c7182fb 
This reverts commit 8d01823.
@ericfranz ericfranz merged commit dbfaaf4 into master Oct 23, 2020
@ericfranz ericfranz deleted the job_composer_validate_user_input branch October 23, 2020 20:18
ericfranz added a commit that referenced this pull request Nov 4, 2020
@ericfranz
Escape user input with Open3 capture (#702)
047f9de 
- use Open3.capture3 for rsync instead of backticks in most cases
- dashboard: remove broken 'active_users' feature from developer dashboard
- myjobs: stick with rsync and Shellwords.escape for rsync command since using filter argument with Open3.capture3 is problematic
- myjobs: fix error reporting on new template page (display all error messages as bullet list at top of form) and fix focus
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johrstrom johrstrom johrstrom left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericfranz @johrstrom