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
Raise routing errors for invalid or empty page ids #178
Conversation
@@ -31,7 +31,13 @@ def clean_path | |||
end | |||
|
|||
def clean_id | |||
@page_id.tr("^#{VALID_CHARACTERS}", '') | |||
@page_id.tr("^#{VALID_CHARACTERS}", '').tap do |id| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Thanks for the pr @prydonius. Sorry for the delay. Would you mind updating to handle the style issues Hound commented on? Thanks! |
7c28ee4
to
d24cf18
Compare
before { get :show, :id => 'exists' } | ||
context "using default configuration" do | ||
describe "on GET to /pages/exists" do | ||
before { get :show, :id => "exists" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the new Ruby 1.9 hash syntax.
No worries, @dgalarza! Rebased and fixed the style issues. |
@@ -31,7 +31,13 @@ def clean_path | |||
end | |||
|
|||
def clean_id | |||
@page_id.tr("^#{VALID_CHARACTERS}", '') | |||
@page_id.tr("^#{VALID_CHARACTERS}", "").tap do |id| | |||
unless id.length > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about if id.blank?
Thanks for the update @prydonius. Do you feel comfortable rebasing and squashing to prep for a merge. |
aec0915
to
9929234
Compare
@dgalarza is this ready to be merged in now? |
Raise routing errors for invalid or empty page ids
👍 thanks for the contribution @prydonius |
Visiting page_ids made entirely up of invalid characters gets sanitised into an empty string, so it looks for a template called "pages/". This raises a missing template error (because it doesn't match the regex checking that it isn't a missing partial).
This commit adds a check to see if the page_id is empty after sanitisation, and raises and error if it is -- which is caught and turned into a routing error.