Permalink
Browse files

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

Conflicts:
	actionpack/lib/action_controller/metal/live.rb
  • Loading branch information...
1 parent 3d3c918 commit 6ffadb52704b2c3e123a0305de357a156ccee192 @tenderlove tenderlove committed with rafaelfranca Mar 13, 2014
@@ -125,9 +125,11 @@ def write(string)
end
def each
+ @response.sending!
while str = @buf.pop
yield str
end
+ @response.sent!
end
def close
@@ -179,12 +181,16 @@ def to_hash
private
- def finalize_response
+ def before_committed
super
jar = request.cookie_jar
# The response can be committed multiple times
- jar.write self unless jar.committed?
- jar.commit!
+ jar.write self unless committed?
+ end
+
+ def before_sending
+ super
+ request.cookie_jar.commit!
headers.freeze
end
@@ -595,7 +595,7 @@ def process(action, http_method = 'GET', *args)
@controller.process(name)
if cookies = @request.env['action_dispatch.cookies']
- unless cookies.committed?
+ unless @response.committed?
cookies.write(@response)
end
end
@@ -91,7 +91,10 @@ def write(string)
end
def each(&block)
- @buf.each(&block)
+ @response.sending!
+ x = @buf.each(&block)
+ @response.sent!
+ x
end
def close
@@ -118,6 +121,8 @@ def initialize(status = 200, header = {}, body = [])
@blank = false
@cv = new_cond
@committed = false
+ @sending = false
+ @sent = false
@content_type = nil
@charset = nil
@@ -138,18 +143,37 @@ def await_commit
end
end
+ def await_sent
+ synchronize { @cv.wait_until { @sent } }
+ end
+
def commit!
synchronize do
- finalize_response
+ before_committed
@committed = true
@cv.broadcast
end
end
- def committed?
- @committed
+ def sending!
+ synchronize do
+ before_sending
+ @sending = true
+ @cv.broadcast
+ end
end
+ def sent!
+ synchronize do
+ @sent = true
+ @cv.broadcast
+ end
+ end
+
+ def sending?; synchronize { @sending }; end
+ def committed?; synchronize { @committed }; end
+ def sent?; synchronize { @sent }; end
+
# Sets the HTTP status code.
def status=(status)
@status = Rack::Utils.status_code(status)
@@ -274,7 +298,10 @@ def cookies
private
- def finalize_response
+ def before_committed
+ end
+
+ def before_sending
end
def merge_default_headers(original, default)
@@ -44,7 +44,7 @@ def sse_with_id
tests SSETestController
def wait_for_response_stream_close
- response.stream.await_close
+ response.body
end
def test_basic_sse
@@ -186,8 +186,8 @@ def exception_in_exception_callback
tests TestController
def assert_stream_closed
- response.stream.await_close
assert response.stream.closed?, 'stream should be closed'
+ assert response.sent?, 'stream should be sent'
end
def capture_log_output
@@ -229,6 +229,7 @@ def test_async_stream
@controller.response = @response
t = Thread.new(@response) { |resp|
+ resp.await_commit
resp.stream.each do |part|
assert_equal parts.shift, part
ol = @controller.latch
@@ -268,6 +269,7 @@ def test_exception_handling_html
assert_raises(ActionView::MissingTemplate) do
get :exception_in_view
end
+ assert response.body
assert_stream_closed
end
@@ -35,6 +35,7 @@ def test_parallel
@response.stream.close
}
+ @response.await_commit
@response.each do |part|
assert_equal 'foo', part
latch.release
@@ -59,21 +60,30 @@ def test_content_length_is_removed
assert_nil @response.headers['Content-Length']
end
- def test_headers_cannot_be_written_after_write
+ def test_headers_cannot_be_written_after_webserver_reads
@response.stream.write 'omg'
+ latch = ActiveSupport::Concurrency::Latch.new
+ t = Thread.new {
+ @response.stream.each do |chunk|
+ latch.release
+ end
+ }
+
+ latch.await
assert @response.headers.frozen?
e = assert_raises(ActionDispatch::IllegalStateError) do
@response.headers['Content-Length'] = "zomg"
end
assert_equal 'header already sent', e.message
+ @response.stream.close
+ t.join
end
def test_headers_cannot_be_written_after_close
@response.stream.close
- assert @response.headers.frozen?
e = assert_raises(ActionDispatch::IllegalStateError) do
@response.headers['Content-Length'] = "zomg"
end

0 comments on commit 6ffadb5

Please sign in to comment.