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.

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

Make sure requested pattern is valid #23

Merged
merged 2 commits into from
Feb 21, 2014
Merged

Conversation

jasonlong
Copy link
Owner

As a security precaution, this checks that the requested generator is valid before attempting to call it.

/cc @gjtorikian @gregose @andrew

@parkr
Copy link
Contributor

parkr commented Feb 20, 2014

Looks great. 👍

Not quite sure why you rescue exceptions and abort – won't the exceptions end the process properly? Just curious.

@jasonlong
Copy link
Owner Author

I saw that pattern elsewhere while looking for Ruby examples. My Ruby-fu isn't the strongest in a lot of places – would it be better to just remove that block and let it abort normally?

@jasonlong
Copy link
Owner Author

Removed the rescue block and merging.

jasonlong pushed a commit that referenced this pull request Feb 21, 2014
Make sure requested pattern is valid
@jasonlong jasonlong merged commit 0ee9a9f into master Feb 21, 2014
@jasonlong jasonlong deleted the generator-validation branch February 21, 2014 14:22
@@ -81,9 +81,9 @@ def generate_background

def generate_pattern
if opts[:generator]
begin
if PATTERNS.include?(opts[:generator].to_sym)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to nit, but could we moved PATTERNS to a frozen array of strings instead of symbols? These symbols are only used as strings and this way we can avoid .to_sym on the passed (potentially user controlled) options. to_sym could lead to a DoS: http://www.ruby-doc.org/core-2.1.0/doc/security_rdoc.html#label-Symbols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants