From 3b40a5d83db90534b3cb61f4dc25547f501e4775 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 3 Dec 2013 22:23:11 -0200 Subject: Improve a couple exception messages related to variants and mime types Avoid one-liner conditionals when they are too big. Avoid concatenating strings to build error messages. Improve messages a bit. --- actionpack/lib/abstract_controller/collector.rb | 18 ++++++++++-------- .../lib/action_controller/metal/mime_responds.rb | 6 ++++-- .../lib/action_dispatch/http/mime_negotiation.rb | 10 +++++----- 3 files changed, 19 insertions(+), 15 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/collector.rb b/actionpack/lib/abstract_controller/collector.rb index f7a309c26c..ddd56b354a 100644 --- a/actionpack/lib/abstract_controller/collector.rb +++ b/actionpack/lib/abstract_controller/collector.rb @@ -23,15 +23,17 @@ module AbstractController protected def method_missing(symbol, &block) - mime_const = symbol.upcase - - raise NoMethodError, "To respond to a custom format, register it as a MIME type first:" + - "http://guides.rubyonrails.org/action_controller_overview.html#restful-downloads." + - "If you meant to respond to a variant like :tablet or :phone, not a custom format," + - "be sure to nest your variant response within a format response: format.html" + - "{ |html| html.tablet { ..." unless Mime.const_defined?(mime_const) + const_name = symbol.upcase + + unless Mime.const_defined?(const_name) + raise NoMethodError, "To respond to a custom format, register it as a MIME type first: " \ + "http://guides.rubyonrails.org/action_controller_overview.html#restful-downloads. " \ + "If you meant to respond to a variant like :tablet or :phone, not a custom format, " \ + "be sure to nest your variant response within a format response: " \ + "format.html { |html| html.tablet { ... } }" + end - mime_constant = Mime.const_get(mime_const) + mime_constant = Mime.const_get(const_name) if Mime::SET.include?(mime_constant) AbstractController::Collector.generate_method_for_mime(mime_constant) diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 0d28409aaa..54d3be68f0 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -348,8 +348,10 @@ module ActionController #:nodoc: # 2. :action - overwrites the default render action used after an # unsuccessful html +post+ request. def respond_with(*resources, &block) - raise "In order to use respond_with, first you need to declare the formats your " \ - "controller responds to in the class level" if self.class.mimes_for_respond_to.empty? + if self.class.mimes_for_respond_to.empty? + raise "In order to use respond_with, first you need to declare the " \ + "formats your controller responds to in the class level." + end if collector = retrieve_collector_from_mimes(&block) options = resources.size == 1 ? {} : resources.extract_options! diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 41e6727315..346598b6de 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -66,15 +66,15 @@ module ActionDispatch end end - # Sets the \variant for template + # Sets the \variant for template. def variant=(variant) if variant.is_a? Symbol @variant = variant else - raise ArgumentError, "request.variant must be set to a Symbol, not a #{variant.class}. For security reasons," + - "never directly set the variant to a user-provided value, like params[:variant].to_sym." + - "Check user-provided value against a whitelist first, then set the variant:"+ - "request.variant = :tablet if params[:some_param] == 'tablet'" + raise ArgumentError, "request.variant must be set to a Symbol, not a #{variant.class}. " \ + "For security reasons, never directly set the variant to a user-provided value, " \ + "like params[:variant].to_sym. Check user-provided value against a whitelist first, " \ + "then set the variant: request.variant = :tablet if params[:variant] == 'tablet'" end end -- cgit v1.2.3