From b53f00690173797a39ff46e55dd25c20581c3d00 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 30 Jul 2009 21:32:24 -0700 Subject: [PATCH] Remove legacy processing and content_length * convert_content_type! is handled by assign_default_content_type_and_charset! * set_content_length! should be handled by the endpoint server. Otherwise each middleware that modifies the body has to do the expensive work of recalculating content_length. * convert_language! appears to be legacy. There are no tests for this * convert_cookies! should be handled by the new HeaderHash in Rack * Use an integer for .status's internal representation to avoid needing to do String manipulation just to find out the status --- .../lib/action_dispatch/http/response.rb | 50 ++++--------------- .../request/multipart_params_parsing_test.rb | 2 - actionpack/test/dispatch/response_test.rb | 6 +-- actionpack/test/new_base/base_test.rb | 3 -- 4 files changed, 12 insertions(+), 49 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 32cfb5ae44..03d1780b77 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -46,18 +46,22 @@ def initialize @header = Rack::Utils::HeaderHash.new end + def status=(status) + @status = status.to_i + end + # The response code of the request def response_code - status.to_s[0,3].to_i rescue 0 + @status end # Returns a String to ensure compatibility with Net::HTTPResponse def code - status.to_s.split(' ')[0] + @status.to_s end def message - status.to_s.split(' ',2)[1] || StatusCodes::STATUS_CODES[response_code] + StatusCodes::STATUS_CODES[@status] end alias_method :status_message, :message @@ -143,10 +147,7 @@ def assign_default_content_type_and_charset! def prepare! assign_default_content_type_and_charset! handle_conditional_get! - set_content_length! - convert_content_type! - convert_language! - convert_cookies! + self["Set-Cookie"] ||= "" end def each(&callback) @@ -203,7 +204,7 @@ def handle_conditional_get! self.etag = body if request && request.etag_matches?(etag) - self.status = '304 Not Modified' + self.status = 304 self.body = [] end @@ -214,7 +215,7 @@ def handle_conditional_get! end def nonempty_ok_response? - ok = !status || status.to_s[0..2] == '200' + ok = !@status || @status == 200 ok && string_body? end @@ -238,36 +239,5 @@ def set_conditional_cache_control! headers["Cache-Control"] = options.join(", ") end - - def convert_content_type! - headers['Content-Type'] ||= "text/html" - headers['Content-Type'] += "; charset=" + headers.delete('charset') if headers['charset'] - end - - # Don't set the Content-Length for block-based bodies as that would mean - # reading it all into memory. Not nice for, say, a 2GB streaming file. - def set_content_length! - if status && status.to_s[0..2] == '204' - headers.delete('Content-Length') - elsif length = headers['Content-Length'] - headers['Content-Length'] = length.to_s - elsif string_body? && (!status || status.to_s[0..2] != '304') - headers["Content-Length"] = Rack::Utils.bytesize(body).to_s - end - end - - def convert_language! - headers["Content-Language"] = headers.delete("language") if headers["language"] - end - - def convert_cookies! - headers['Set-Cookie'] = - if header = headers['Set-Cookie'] - header = header.split("\n") if header.respond_to?(:to_str) - header.compact - else - [] - end - end end end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 9e008a9ae8..301080842e 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -120,8 +120,6 @@ def teardown fixture = FIXTURE_PATH + "/mona_lisa.jpg" params = { :uploaded_data => fixture_file_upload(fixture, "image/jpg") } post '/read', params - expected_length = 'File: '.length + File.size(fixture) - assert_equal expected_length, response.content_length end end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 2b1540c678..256ed06a45 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -15,8 +15,7 @@ def setup "Content-Type" => "text/html; charset=utf-8", "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', - "Set-Cookie" => "", - "Content-Length" => "13" + "Set-Cookie" => "" }, headers) parts = [] @@ -34,8 +33,7 @@ def setup "Content-Type" => "text/html; charset=utf-8", "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"', - "Set-Cookie" => "", - "Content-Length" => "8" + "Set-Cookie" => "" }, headers) end diff --git a/actionpack/test/new_base/base_test.rb b/actionpack/test/new_base/base_test.rb index d9d552f9e5..1b2e917ced 100644 --- a/actionpack/test/new_base/base_test.rb +++ b/actionpack/test/new_base/base_test.rb @@ -34,7 +34,6 @@ class BaseTest < SimpleRouteCase assert_body "success" assert_status 200 assert_content_type "text/html; charset=utf-8" - assert_header "Content-Length", "7" end # :api: plugin @@ -42,7 +41,6 @@ class BaseTest < SimpleRouteCase get "/dispatching/simple/modify_response_body" assert_body "success" - assert_header "Content-Length", "7" # setting the body manually sets the content length end # :api: plugin @@ -50,7 +48,6 @@ class BaseTest < SimpleRouteCase get "/dispatching/simple/modify_response_body_twice" assert_body "success!" - assert_header "Content-Length", "8" end test "controller path" do -- GitLab