From 9af4258186027e5a80bd5a0c821862378e1492ad Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 28 Feb 2014 11:57:15 -0800 Subject: set the error callback to a nice default in case nobody set an error callback and an error happens --- actionpack/lib/action_controller/metal/live.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index fdf4ef293d..5ef4f6ccda 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -108,7 +108,7 @@ module ActionController class Buffer < ActionDispatch::Response::Buffer #:nodoc: def initialize(response) - @error_callback = nil + @error_callback = lambda { true } super(response, SizedQueue.new(10)) end -- cgit v1.2.3 From 30d21dfcb7fafe49b3805b8249454485a90097b6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 28 Feb 2014 15:22:35 -0800 Subject: live controllers should have live responses detect the type of controller we're testing and return the right type of response based on that controller. This allows us to stop doing the weird sleep thing. --- actionpack/lib/action_controller/metal/live.rb | 16 +++++++++++++-- actionpack/lib/action_controller/test_case.rb | 28 ++++++++++++++++++++------ 2 files changed, 36 insertions(+), 8 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 5ef4f6ccda..fba17746c0 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -107,8 +107,11 @@ module ActionController end class Buffer < ActionDispatch::Response::Buffer #:nodoc: + include MonitorMixin + def initialize(response) @error_callback = lambda { true } + @cv = new_cond super(response, SizedQueue.new(10)) end @@ -128,8 +131,17 @@ module ActionController end def close - super - @buf.push nil + synchronize do + super + @buf.push nil + @cv.broadcast + end + end + + def await_close + synchronize do + @cv.wait_until { @closed } + end end def on_error(&block) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 8650b75400..33a5858766 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -258,6 +258,17 @@ module ActionController end end + class LiveTestResponse < Live::Response + def recycle! + @body = nil + initialize + end + + def body + @body ||= super + end + end + # Methods #destroy and #load! are overridden to avoid calling methods on the # @store object, which does not exist for the TestSession class. class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: @@ -583,13 +594,14 @@ module ActionController end def setup_controller_request_and_response - @request = build_request - @response = build_response - @response.request = @request - @controller = nil unless defined? @controller + response_klass = TestResponse + if klass = self.class.controller_class + if klass < ActionController::Live + response_klass = LiveTestResponse + end unless @controller begin @controller = klass.new @@ -599,6 +611,10 @@ module ActionController end end + @request = build_request + @response = build_response response_klass + @response.request = @request + if @controller @controller.request = @request @controller.params = {} @@ -609,8 +625,8 @@ module ActionController TestRequest.new end - def build_response - TestResponse.new + def build_response(klass) + klass.new end included do -- cgit v1.2.3 From a7b059ec7f25c16beeacf2c545d2be593ed0388b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 28 Feb 2014 15:39:08 -0800 Subject: use built-in exception handling in live controllers when an exception happens in an action before the response has been committed, then we should re-raise the exception in the main thread. This lets us reuse the existing exception handling. --- actionpack/lib/action_controller/metal/live.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index fba17746c0..d60f1b0d44 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -203,6 +203,7 @@ module ActionController t1 = Thread.current locals = t1.keys.map { |key| [key, t1[key]] } + error = nil # This processes the action in a child thread. It lets us return the # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client @@ -217,8 +218,9 @@ module ActionController begin super(name) rescue => e - @_response.status = 500 unless @_response.committed? - @_response.status = 400 if e.class == ActionController::BadRequest + unless @_response.committed? + error = e + end begin @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html @_response.stream.call_on_error @@ -234,6 +236,7 @@ module ActionController } @_response.await_commit + raise error if error end def log_error(exception) -- cgit v1.2.3 From 38594b35275a431ccf2fdc10be7219685aeed487 Mon Sep 17 00:00:00 2001 From: Shuhei Kagawa Date: Mon, 3 Mar 2014 22:39:15 +0900 Subject: Add spaces to deep_munge log message. --- actionpack/lib/action_controller/log_subscriber.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index e920a33765..b1acca2435 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -54,9 +54,9 @@ module ActionController end def deep_munge(event) - message = "Value for params[:#{event.payload[:keys].join('][:')}] was set"\ - "to nil, because it was one of [], [null] or [null, null, ...]."\ - "Go to http://guides.rubyonrails.org/security.html#unsafe-query-generation"\ + message = "Value for params[:#{event.payload[:keys].join('][:')}] was set "\ + "to nil, because it was one of [], [null] or [null, null, ...]. "\ + "Go to http://guides.rubyonrails.org/security.html#unsafe-query-generation "\ "for more information."\ debug(message) -- cgit v1.2.3 From 67584c6ae37c88f8abba6f4fbdeedc7c1a6dfa1b Mon Sep 17 00:00:00 2001 From: "John Barton (joho)" Date: Wed, 5 Mar 2014 11:24:51 +1100 Subject: Make CSRF failure logging optional/configurable. Added the log_warning_on_csrf_failure option to ActionController::RequestForgeryProtection which is on by default. --- .../lib/action_controller/metal/request_forgery_protection.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index c88074d4c6..e3b1f5ae7c 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -68,6 +68,10 @@ module ActionController #:nodoc: config_accessor :allow_forgery_protection self.allow_forgery_protection = true if allow_forgery_protection.nil? + # Controls whether a CSRF failure logs a warning. On by default. + config_accessor :log_warning_on_csrf_failure + self.log_warning_on_csrf_failure = true + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -193,7 +197,9 @@ module ActionController #:nodoc: mark_for_same_origin_verification! if !verified_request? - logger.warn "Can't verify CSRF token authenticity" if logger + if logger && log_warning_on_csrf_failure + logger.warn "Can't verify CSRF token authenticity" + end handle_unverified_request end end -- cgit v1.2.3 From ed88a601f7b37de0f89b64249aaeed884faed836 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Fri, 28 Feb 2014 19:39:22 -0500 Subject: Do note remove `Content-Type` when `render :body` `render :body` should just not set the `Content-Type` header. By removing the header, it breaks the compatibility with other parts. After this commit, `render :body` will returns `text/html` content type, sets by default from `ActionDispatch::Response`, and it will preserve the overridden content type if you override it. Fixes #14197, #14238 This partially reverts commit 3047376870d4a7adc7ff15c3cb4852e073c8f1da. --- actionpack/lib/action_controller/metal/rack_delegation.rb | 4 ++-- actionpack/lib/action_controller/metal/rendering.rb | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/rack_delegation.rb b/actionpack/lib/action_controller/metal/rack_delegation.rb index e1bee9e60c..bdf6e88699 100644 --- a/actionpack/lib/action_controller/metal/rack_delegation.rb +++ b/actionpack/lib/action_controller/metal/rack_delegation.rb @@ -5,8 +5,8 @@ module ActionController module RackDelegation extend ActiveSupport::Concern - delegate :headers, :status=, :location=, :content_type=, :no_content_type=, - :status, :location, :content_type, :no_content_type, :to => "@_response" + delegate :headers, :status=, :location=, :content_type=, + :status, :location, :content_type, :to => "@_response" def dispatch(action, request) set_response!(request) diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 3c4ef596c7..93e7d6954c 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -45,9 +45,7 @@ module ActionController def _process_format(format, options = {}) super - if options[:body] - self.headers.delete "Content-Type" - elsif options[:plain] + if options[:plain] self.content_type = Mime::TEXT else self.content_type ||= format.to_s -- cgit v1.2.3 From 77a09218f697676f8a05f06eeb5c89a26419d489 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Mar 2014 16:07:04 -0700 Subject: only write the jar if the response isn't committed when streaming responses, we need to make sure the cookie jar is written to the headers before returning up the stack. This commit introduces a new method on the response object that writes the cookie jar to the headers as the response is committed. The middleware and test framework will not write the cookie headers if the response has already been committed. fixes #14352 --- actionpack/lib/action_controller/metal/live.rb | 12 ++++++++---- actionpack/lib/action_controller/test_case.rb | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index d60f1b0d44..43cf9b9723 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -177,13 +177,17 @@ module ActionController end end - def commit! - headers.freeze + private + + def finalize_response super + jar = request.cookie_jar + # The response can be committed multiple times + jar.write self unless jar.committed? + jar.commit! + headers.freeze end - private - def build_buffer(response, body) buf = Live::Buffer.new response body.each { |part| buf.write part } diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 33a5858766..009f83861d 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -267,6 +267,18 @@ module ActionController def body @body ||= super end + + # Was the response successful? + alias_method :success?, :successful? + + # Was the URL not found? + alias_method :missing?, :not_found? + + # Were we redirected? + alias_method :redirect?, :redirection? + + # Was there a server-side error? + alias_method :error?, :server_error? end # Methods #destroy and #load! are overridden to avoid calling methods on the @@ -583,7 +595,9 @@ module ActionController @controller.process(name) if cookies = @request.env['action_dispatch.cookies'] - cookies.write(@response) + unless cookies.committed? + cookies.write(@response) + end end @response.prepare! -- cgit v1.2.3 From c0a783610f5cf77050a55ad70b2cd2cf657bffe3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Mar 2014 16:21:01 -0700 Subject: just ask the response for the commit status, we do not need to ask the jar --- actionpack/lib/action_controller/metal/live.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 43cf9b9723..fe63cf2b98 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -183,7 +183,7 @@ module ActionController super jar = request.cookie_jar # The response can be committed multiple times - jar.write self unless jar.committed? + jar.write self unless committed? jar.commit! headers.freeze end -- cgit v1.2.3 From 3df07d093a1e4207caa63fd2e3b67599211f5800 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Mar 2014 17:40:08 -0700 Subject: use the body proxy to freeze headers avoid freezing the headers until the web server has actually read data from the body proxy. Once the webserver has read data, then we should throw an error if someone tries to set a header --- actionpack/lib/action_controller/metal/live.rb | 10 ++++++++-- actionpack/lib/action_controller/test_case.rb | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index fe63cf2b98..41b997a755 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -125,9 +125,11 @@ module ActionController end def each + @response.sending! while str = @buf.pop yield str end + @response.sent! end def close @@ -179,12 +181,16 @@ module ActionController private - def finalize_response + def before_committed super jar = request.cookie_jar # The response can be committed multiple times jar.write self unless committed? - jar.commit! + end + + def before_sending + super + request.cookie_jar.commit! headers.freeze end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 009f83861d..e9166d8747 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -595,7 +595,7 @@ module ActionController @controller.process(name) if cookies = @request.env['action_dispatch.cookies'] - unless cookies.committed? + unless @response.committed? cookies.write(@response) end end -- cgit v1.2.3