From b79bfaadaf5b5c6d3e458c24184a0e846bd8cf55 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Mon, 6 Jul 2015 14:12:00 -0300 Subject: Use URL path extension as format in bad params exception handling --- actionpack/lib/action_dispatch/http/parameters.rb | 19 +++++++++++++++++-- actionpack/test/dispatch/show_exceptions_test.rb | 16 +++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index c9df787351..5c20bc53c2 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -41,9 +41,9 @@ module ActionDispatch # Returns a hash with the \parameters used to form the \path of the request. # Returned hash keys are strings: # - # {'action' => 'my_action', 'controller' => 'my_controller'} + # {'action' => 'my_action', 'controller' => 'my_controller', format => 'html'} def path_parameters - get_header(PARAMETERS_KEY) || {} + get_header(PARAMETERS_KEY) || default_path_parameters end private @@ -66,6 +66,21 @@ module ActionDispatch def params_parsers ActionDispatch::Request.parameter_parsers end + + def default_path_parameters + if format = format_from_path_extension + { 'format' => format } + else + {} + end + end + + def format_from_path_extension + path = @env['action_dispatch.original_path'] + if match = path.match(/\.(\w+)$/) + match.captures.first + end + end end end end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index ffdf775836..14894d4b82 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -8,7 +8,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest case req.path when "/not_found" raise AbstractController::ActionNotFound - when "/bad_params" + when "/bad_params", "/bad_params.json" begin raise StandardError.new rescue @@ -120,4 +120,18 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest assert_response 405 assert_equal "", body end + + test "bad params exception is returned in the correct format" do + @app = ProductionApp + + get "/bad_params", headers: { 'action_dispatch.show_exceptions' => true } + assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] + assert_response 400 + assert_match(/400 error/, body) + + get "/bad_params.json", headers: { 'action_dispatch.show_exceptions' => true } + assert_equal "application/json; charset=utf-8", response.headers["Content-Type"] + assert_response 400 + assert_equal("{\"status\":400,\"error\":\"Bad Request\"}", body) + end end -- cgit v1.2.3 From 83b4e9073f0852afc065ef398bd3ad9b5a6db29c Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Mon, 6 Jul 2015 21:33:19 -0300 Subject: Response when error should be formatted properly in Rails API if local request --- actionpack/lib/action_dispatch/http/parameters.rb | 6 +++--- .../lib/action_dispatch/middleware/debug_exceptions.rb | 17 +++++++++++++++-- actionpack/test/controller/mime/respond_to_test.rb | 10 ++++++---- actionpack/test/dispatch/debug_exceptions_test.rb | 18 ++++++++++++++++++ actionpack/test/dispatch/routing_test.rb | 1 + 5 files changed, 43 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 5c20bc53c2..4084d78f49 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -69,15 +69,15 @@ module ActionDispatch def default_path_parameters if format = format_from_path_extension - { 'format' => format } + { format: format } else {} end end def format_from_path_extension - path = @env['action_dispatch.original_path'] - if match = path.match(/\.(\w+)$/) + path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] + if match = path && path.match(/\.(\w+)$/) match.captures.first end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 66bb74b9c5..972410d806 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -38,9 +38,10 @@ module ActionDispatch end end - def initialize(app, routes_app = nil) + def initialize(app, routes_app = nil, api_only = false) @app = app @routes_app = routes_app + @api_only = api_only end def call(env) @@ -90,7 +91,19 @@ module ActionDispatch ) file = "rescues/#{wrapper.rescue_template}" - if request.xhr? + if @api_only + body = { + :status => wrapper.status_code, + :error => Rack::Utils::HTTP_STATUS_CODES.fetch(wrapper.status_code, Rack::Utils::HTTP_STATUS_CODES[500]), + :exception => wrapper.exception.inspect, + :traces => traces + } + if content_type = request.formats.first + to_format = "to_#{content_type.to_sym}" + body = body.public_send(to_format) + end + format = "application/json" + elsif request.xhr? body = template.render(template: file, layout: false, formats: [:text]) format = "text/plain" else diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index c025c7fa00..76e2d3ff43 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -661,10 +661,6 @@ class RespondToControllerTest < ActionController::TestCase end def test_variant_inline_syntax - get :variant_inline_syntax, format: :js - assert_equal "text/javascript", @response.content_type - assert_equal "js", @response.body - get :variant_inline_syntax assert_equal "text/html", @response.content_type assert_equal "none", @response.body @@ -674,6 +670,12 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "phone", @response.body end + def test_variant_inline_syntax_with_format + get :variant_inline_syntax, format: :js + assert_equal "text/javascript", @response.content_type + assert_equal "js", @response.body + end + def test_variant_inline_syntax_without_block get :variant_inline_syntax_without_block, params: { v: :phone } assert_equal "text/html", @response.content_type diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 30772bd9ed..48eff700ce 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -75,6 +75,13 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end + class BoomerAPI < Boomer + def call(env) + env['action_dispatch.show_detailed_exceptions'] = @detailed + raise "puke!" + end + end + RoutesApp = Struct.new(:routes).new(SharedTestRoutes) ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp) DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp) @@ -155,6 +162,17 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/ActionController::ParameterMissing/, body) end + test "rescue with json on API request" do + @app = ActionDispatch::DebugExceptions.new(BoomerAPI.new(true), RoutesApp, true) + + get "/index.json", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_no_match(/
/, body) + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + end + test "rescue with text error for xhr request" do @app = DevelopmentApp xhr_request_env = {'action_dispatch.show_exceptions' => true, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest'} diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 8972f3e74d..1b711e1bf8 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3272,6 +3272,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest expected_params = { controller: 'downloads', action: 'show', + format: 'tar', id: '1' } -- cgit v1.2.3 From 05d89410bf97d0778e78558db3c9fed275f8a614 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Tue, 7 Jul 2015 12:43:49 -0300 Subject: Fix some edge cases in AD::DebugExceptions in rails api apps --- .../action_dispatch/middleware/debug_exceptions.rb | 105 +++++++++++++-------- actionpack/test/dispatch/debug_exceptions_test.rb | 73 +++++++++++--- 2 files changed, 126 insertions(+), 52 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 972410d806..c81f52ec88 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -67,55 +67,78 @@ module ActionDispatch log_error(request, wrapper) if request.get_header('action_dispatch.show_detailed_exceptions') - traces = wrapper.traces - - trace_to_show = 'Application Trace' - if traces[trace_to_show].empty? && wrapper.rescue_template != 'routing_error' - trace_to_show = 'Full Trace' - end - - if source_to_show = traces[trace_to_show].first - source_to_show_id = source_to_show[:id] - end - - template = DebugView.new([RESCUES_TEMPLATE_PATH], - request: request, - exception: wrapper.exception, - traces: traces, - show_source_idx: source_to_show_id, - trace_to_show: trace_to_show, - routes_inspector: routes_inspector(exception), - source_extracts: wrapper.source_extracts, - line_number: wrapper.line_number, - file: wrapper.file - ) - file = "rescues/#{wrapper.rescue_template}" - if @api_only - body = { - :status => wrapper.status_code, - :error => Rack::Utils::HTTP_STATUS_CODES.fetch(wrapper.status_code, Rack::Utils::HTTP_STATUS_CODES[500]), - :exception => wrapper.exception.inspect, - :traces => traces - } - if content_type = request.formats.first - to_format = "to_#{content_type.to_sym}" - body = body.public_send(to_format) - end - format = "application/json" - elsif request.xhr? - body = template.render(template: file, layout: false, formats: [:text]) - format = "text/plain" + render_for_api_application(request, wrapper) else - body = template.render(template: file, layout: 'rescues/layout') - format = "text/html" + render_for_non_api_application(request, wrapper) end - render(wrapper.status_code, body, format) else raise exception end end + def render_for_non_api_application(request, wrapper) + template = create_template(request, wrapper) + + file = "rescues/#{wrapper.rescue_template}" + + if request.xhr? + body = template.render(template: file, layout: false, formats: [:text]) + format = "text/plain" + else + body = template.render(template: file, layout: 'rescues/layout') + format = "text/html" + end + render(wrapper.status_code, body, format) + end + + def render_for_api_application(request, wrapper) + body = { + :status => wrapper.status_code, + :error => Rack::Utils::HTTP_STATUS_CODES.fetch(wrapper.status_code, Rack::Utils::HTTP_STATUS_CODES[500]), + :exception => wrapper.exception.inspect, + :traces => wrapper.traces + } + + content_type = request.formats.first + to_format = "to_#{content_type.to_sym}" + + if content_type && body.respond_to?(to_format) + body = body.public_send(to_format) + format = content_type + else + body = body.to_json + format = Mime::JSON + end + + render(wrapper.status_code, body, format) + end + + def create_template(request, wrapper) + traces = wrapper.traces + + trace_to_show = 'Application Trace' + if traces[trace_to_show].empty? && wrapper.rescue_template != 'routing_error' + trace_to_show = 'Full Trace' + end + + if source_to_show = traces[trace_to_show].first + source_to_show_id = source_to_show[:id] + end + + DebugView.new([RESCUES_TEMPLATE_PATH], + request: request, + exception: wrapper.exception, + traces: traces, + show_source_idx: source_to_show_id, + trace_to_show: trace_to_show, + routes_inspector: routes_inspector(wrapper.exception), + source_extracts: wrapper.source_extracts, + line_number: wrapper.line_number, + file: wrapper.file + ) + end + def render(status, body, format) [status, {'Content-Type' => "#{format}; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 48eff700ce..d9c6233511 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -162,17 +162,6 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/ActionController::ParameterMissing/, body) end - test "rescue with json on API request" do - @app = ActionDispatch::DebugExceptions.new(BoomerAPI.new(true), RoutesApp, true) - - get "/index.json", headers: { 'action_dispatch.show_exceptions' => true } - assert_response 500 - assert_no_match(/
/, body) - assert_no_match(//, body) - assert_equal "application/json", response.content_type - assert_match(/RuntimeError: puke/, body) - end - test "rescue with text error for xhr request" do @app = DevelopmentApp xhr_request_env = {'action_dispatch.show_exceptions' => true, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest'} @@ -223,6 +212,68 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/ActionController::ParameterMissing/, body) end + test "rescue with json error for API request" do + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, true) + + get "/", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_no_match(/
/, body) + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + get "/not_found", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 404 + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/#{AbstractController::ActionNotFound.name}/, body) + + get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 405 + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::MethodNotAllowed/, body) + + get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 405 + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::UnknownHttpMethod/, body) + + get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 400 + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::BadRequest/, body) + + get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 400 + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::ParameterMissing/, body) + end + + test "rescue with json on API request returns only allowed formats or json as a fallback" do + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, true) + + get "/index.json", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + get "/index.html", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_no_match(/
/, body) + assert_no_match(//, body) + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + get "/index.xml", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_equal "application/xml", response.content_type + assert_match(/RuntimeError: puke/, body) + end + test "does not show filtered parameters" do @app = DevelopmentApp -- cgit v1.2.3 From a16ab35d34a3bc6440994fe20afc2eced096b618 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Tue, 7 Jul 2015 13:25:11 -0300 Subject: New hash syntax in AD::DebugExceptions --- actionpack/lib/action_dispatch/middleware/debug_exceptions.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index c81f52ec88..3093b6a8a6 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -94,10 +94,10 @@ module ActionDispatch def render_for_api_application(request, wrapper) body = { - :status => wrapper.status_code, - :error => Rack::Utils::HTTP_STATUS_CODES.fetch(wrapper.status_code, Rack::Utils::HTTP_STATUS_CODES[500]), - :exception => wrapper.exception.inspect, - :traces => wrapper.traces + status: wrapper.status_code, + error: Rack::Utils::HTTP_STATUS_CODES.fetch(wrapper.status_code, Rack::Utils::HTTP_STATUS_CODES[500]), + exception: wrapper.exception.inspect, + traces: wrapper.traces } content_type = request.formats.first -- cgit v1.2.3 From b75f5c278ad2bc51faf1ae8438afad0e794a5261 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Tue, 7 Jul 2015 13:37:52 -0300 Subject: Remove unneeded args in AD::DebugExceptions --- actionpack/lib/action_dispatch/middleware/debug_exceptions.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 3093b6a8a6..d93b5fd659 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -79,7 +79,6 @@ module ActionDispatch def render_for_non_api_application(request, wrapper) template = create_template(request, wrapper) - file = "rescues/#{wrapper.rescue_template}" if request.xhr? -- cgit v1.2.3 From d879c0ec5a7eebf9eed01c84c8f95566be2aedf4 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Tue, 7 Jul 2015 13:41:04 -0300 Subject: Minor cleanup in AD::DebugExceptions --- .../lib/action_dispatch/middleware/debug_exceptions.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index d93b5fd659..e5500ccdac 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -93,24 +93,27 @@ module ActionDispatch def render_for_api_application(request, wrapper) body = { - status: wrapper.status_code, - error: Rack::Utils::HTTP_STATUS_CODES.fetch(wrapper.status_code, Rack::Utils::HTTP_STATUS_CODES[500]), + status: wrapper.status_code, + error: Rack::Utils::HTTP_STATUS_CODES.fetch( + wrapper.status_code, + Rack::Utils::HTTP_STATUS_CODES[500] + ), exception: wrapper.exception.inspect, - traces: wrapper.traces + traces: wrapper.traces } content_type = request.formats.first to_format = "to_#{content_type.to_sym}" if content_type && body.respond_to?(to_format) - body = body.public_send(to_format) + formatted_body = body.public_send(to_format) format = content_type else - body = body.to_json + formatted_body = body.to_json format = Mime::JSON end - render(wrapper.status_code, body, format) + render(wrapper.status_code, formatted_body, format) end def create_template(request, wrapper) -- cgit v1.2.3 From 02c5c0d1565318668f8d3161374c4f4c269e6d33 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Mon, 20 Jul 2015 14:07:49 -0300 Subject: Improve regexp in AC::Http::Parameters --- actionpack/lib/action_dispatch/http/parameters.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 4084d78f49..9d450beae5 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -77,7 +77,7 @@ module ActionDispatch def format_from_path_extension path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] - if match = path && path.match(/\.(\w+)$/) + if match = path && path.match(/\.(\w+)\z/) match.captures.first end end -- cgit v1.2.3 From 6fb2afed5284c7bc0157055609a0fde9ea4518d0 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Mon, 20 Jul 2015 14:09:16 -0300 Subject: Better name for method in DebugExceptions middleware --- actionpack/lib/action_dispatch/middleware/debug_exceptions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index e5500ccdac..7912b5ce82 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -70,14 +70,14 @@ module ActionDispatch if @api_only render_for_api_application(request, wrapper) else - render_for_non_api_application(request, wrapper) + render_for_default_application(request, wrapper) end else raise exception end end - def render_for_non_api_application(request, wrapper) + def render_for_default_application(request, wrapper) template = create_template(request, wrapper) file = "rescues/#{wrapper.rescue_template}" -- cgit v1.2.3 From 6fa2023c816e8dc8d2735a5fe87098ef5d8253f9 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Mon, 20 Jul 2015 16:38:09 -0300 Subject: DebugException initialize with a response_format value --- .../lib/action_dispatch/middleware/debug_exceptions.rb | 13 +++++++------ actionpack/test/dispatch/debug_exceptions_test.rb | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 7912b5ce82..642c7183cc 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -38,10 +38,10 @@ module ActionDispatch end end - def initialize(app, routes_app = nil, api_only = false) - @app = app - @routes_app = routes_app - @api_only = api_only + def initialize(app, routes_app = nil, response_format = :default) + @app = app + @routes_app = routes_app + @response_format = response_format end def call(env) @@ -67,9 +67,10 @@ module ActionDispatch log_error(request, wrapper) if request.get_header('action_dispatch.show_detailed_exceptions') - if @api_only + case @response_format + when :api render_for_api_application(request, wrapper) - else + when :default render_for_default_application(request, wrapper) end else diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index d9c6233511..159bf10545 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -213,7 +213,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end test "rescue with json error for API request" do - @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, true) + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) get "/", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 @@ -254,7 +254,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end test "rescue with json on API request returns only allowed formats or json as a fallback" do - @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, true) + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) get "/index.json", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 -- cgit v1.2.3 From 290a536d284d49eafebff6685fdf3b56746cbdb7 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Mon, 3 Aug 2015 11:12:07 -0300 Subject: Update Changelog with the added response_format option in AD::DebugExceptions --- actionpack/CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index b8563d5076..271a57a7ad 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,13 @@ +* Add a `response_format` option to `ActionDispatch::DebugExceptions` + to configure the format of the response when errors occur in + development mode. + + If `response_format` is `:default` the debug info will be rendered + in an HTML page. In the other hand, if the provided value is `:api` + the debug info will be rendered in the original response format. + + *Jorge Bejar* + * Change the `protect_from_forgery` prepend default to `false` Per this comment -- cgit v1.2.3 From 84e8accd6fb83031e4c27e44925d7596655285f7 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Thu, 3 Dec 2015 15:19:25 -0300 Subject: Do not add format key to request_params I did this change but it is affecting how the request params end up after being processed by the router. To be in the safe side, I just take the format from the extension in the URL when is not present in those params and it's being used only for the `Request#formats` method --- .../lib/action_dispatch/http/mime_negotiation.rb | 9 +++++++++ actionpack/lib/action_dispatch/http/parameters.rb | 19 ++----------------- actionpack/test/dispatch/routing_test.rb | 1 - 3 files changed, 11 insertions(+), 18 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 7acf91902d..004713ec1a 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -67,6 +67,8 @@ module ActionDispatch v = if params_readable Array(Mime[parameters[:format]]) + elsif format_from_path_extension + [Mime[format_from_path_extension]] elsif use_accept_header && valid_accept_header accepts elsif xhr? @@ -160,6 +162,13 @@ module ActionDispatch def use_accept_header !self.class.ignore_accept_header end + + def format_from_path_extension + path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] + if match = path && path.match(/\.(\w+)\z/) + match.captures.first + end + end end end end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 9d450beae5..c9df787351 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -41,9 +41,9 @@ module ActionDispatch # Returns a hash with the \parameters used to form the \path of the request. # Returned hash keys are strings: # - # {'action' => 'my_action', 'controller' => 'my_controller', format => 'html'} + # {'action' => 'my_action', 'controller' => 'my_controller'} def path_parameters - get_header(PARAMETERS_KEY) || default_path_parameters + get_header(PARAMETERS_KEY) || {} end private @@ -66,21 +66,6 @@ module ActionDispatch def params_parsers ActionDispatch::Request.parameter_parsers end - - def default_path_parameters - if format = format_from_path_extension - { format: format } - else - {} - end - end - - def format_from_path_extension - path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] - if match = path && path.match(/\.(\w+)\z/) - match.captures.first - end - end end end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 1b711e1bf8..8972f3e74d 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3272,7 +3272,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest expected_params = { controller: 'downloads', action: 'show', - format: 'tar', id: '1' } -- cgit v1.2.3 From da5acae03266808fdbc9a59f43cf8cf7cf25a24e Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Thu, 3 Dec 2015 15:36:06 -0300 Subject: Avoid warning because of the mime type --- actionpack/lib/action_dispatch/middleware/debug_exceptions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 642c7183cc..b55c937e0c 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -111,7 +111,7 @@ module ActionDispatch format = content_type else formatted_body = body.to_json - format = Mime::JSON + format = Mime[:json] end render(wrapper.status_code, formatted_body, format) -- cgit v1.2.3 From cdb7a8477ff19f8ed6f549cedc901bd5934a61e8 Mon Sep 17 00:00:00 2001 From: Jorge Bejar Date: Wed, 9 Dec 2015 14:18:13 -0300 Subject: Avoid calling AD::MimeNegotiation#format_from_path_extension method twice --- actionpack/lib/action_dispatch/http/mime_negotiation.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 004713ec1a..0152c17ed4 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -67,8 +67,8 @@ module ActionDispatch v = if params_readable Array(Mime[parameters[:format]]) - elsif format_from_path_extension - [Mime[format_from_path_extension]] + elsif format = format_from_path_extension + Array(Mime[format]) elsif use_accept_header && valid_accept_header accepts elsif xhr? -- cgit v1.2.3