Merge commit 'rails/master'

10 files changed, 57 insertions, 78 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index e758983d7a..aab26b411c 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *Edge*
 
+* Ruby 1.9: fix Content-Length for multibyte send_data streaming.  #2661 [Sava Chankov]
+
 * Ruby 1.9: ERB template encoding using a magic comment at the top of the file.  [Jeremy Kemper]
     <%# encoding: utf-8 %>
 
diff --git a/actionpack/lib/action_controller/base/conditional_get.rb b/actionpack/lib/action_controller/base/conditional_get.rb
index d287ec4994..6d35137428 100644
--- a/actionpack/lib/action_controller/base/conditional_get.rb
+++ b/actionpack/lib/action_controller/base/conditional_get.rb
@@ -29,11 +29,7 @@ module ActionController
       response.last_modified = options[:last_modified] if options[:last_modified]
 
       if options[:public]
-        cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip }
-        cache_control.delete("private")
-        cache_control.delete("no-cache")
-        cache_control << "public"
-        response.headers["Cache-Control"] = cache_control.join(', ')
+        response.cache_control[:public] = true
       end
 
       if request.fresh?(response)
@@ -107,21 +103,10 @@ module ActionController
     # This method will overwrite an existing Cache-Control header.
     # See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities.
     def expires_in(seconds, options = {}) #:doc:
-      cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip }
+      response.cache_control.merge!(:max_age => seconds, :public => options.delete(:public))
+      options.delete(:private)
 
-      cache_control << "max-age=#{seconds}"
-      cache_control.delete("no-cache")
-      if options[:public]
-        cache_control.delete("private")
-        cache_control << "public"
-      else
-        cache_control << "private"
-      end
-
-      # This allows for additional headers to be passed through like 'max-stale' => 5.hours
-      cache_control += options.symbolize_keys.reject{|k,v| k == :public || k == :private }.map{ |k,v| v == true ? k.to_s : "#{k.to_s}=#{v.to_s}"}
-
-      response.headers["Cache-Control"] = cache_control.join(', ')
+      response.cache_control[:extras] = options.map {|k,v| "#{k}=#{v}"}
     end
 
     # Sets a HTTP 1.1 Cache-Control header of "no-cache" so no caching should occur by the browser or
diff --git a/actionpack/lib/action_controller/base/streaming.rb b/actionpack/lib/action_controller/base/streaming.rb
index 9ff4f25f43..f0317c6e99 100644
--- a/actionpack/lib/action_controller/base/streaming.rb
+++ b/actionpack/lib/action_controller/base/streaming.rb
@@ -1,3 +1,5 @@
+require 'active_support/core_ext/string/bytesize'
+
 module ActionController #:nodoc:
   # Methods for sending arbitrary data and for streaming files to the browser,
   # instead of rendering.
@@ -142,7 +144,7 @@ module ActionController #:nodoc:
       # instead. See ActionController::Base#render for more information.
       def send_data(data, options = {}) #:doc:
         logger.info "Sending data #{options[:filename]}" if logger
-        send_file_headers! options.merge(:length => data.size)
+        send_file_headers! options.merge(:length => data.bytesize)
         @performed_render = false
         render :status => options[:status], :text => data
       end
@@ -179,7 +181,7 @@ module ActionController #:nodoc:
         # after it displays the "open/save" dialog, which means that if you
         # hit "open" the file isn't there anymore when the application that
         # is called for handling the download is run, so let's workaround that
-        headers['Cache-Control'] = 'private' if headers['Cache-Control'] == 'no-cache'
+        response.cache_control[:public] ||= false
       end
   end
 end
diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb
index e7c64d0942..d32d5562e8 100644
--- a/actionpack/lib/action_controller/testing/process.rb
+++ b/actionpack/lib/action_controller/testing/process.rb
@@ -52,7 +52,7 @@ module ActionController #:nodoc:
   class TestResponse < ActionDispatch::TestResponse
     def recycle!
       @status = 200
-      @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS)
+      @header = Rack::Utils::HeaderHash.new
       @writer = lambda { |x| @body << x }
       @block = nil
       @length = 0
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index e58b4b5e19..03d1780b77 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -32,8 +32,8 @@ module ActionDispatch # :nodoc:
   #    end
   #  end
   class Response < Rack::Response
-    DEFAULT_HEADERS = { "Cache-Control" => "no-cache" }
     attr_accessor :request
+    attr_reader :cache_control
 
     attr_writer :header
     alias_method :headers=, :header=
@@ -42,21 +42,26 @@ module ActionDispatch # :nodoc:
 
     def initialize
       super
-      @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS)
+      @cache_control = {}
+      @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
 
@@ -142,10 +147,7 @@ module ActionDispatch # :nodoc:
     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)
@@ -196,22 +198,24 @@ module ActionDispatch # :nodoc:
 
     private
       def handle_conditional_get!
-        if etag? || last_modified?
+        if etag? || last_modified? || !cache_control.empty?
           set_conditional_cache_control!
         elsif nonempty_ok_response?
           self.etag = body
 
           if request && request.etag_matches?(etag)
-            self.status = '304 Not Modified'
+            self.status = 304
             self.body = []
           end
 
           set_conditional_cache_control!
+        else
+          headers["Cache-Control"] = "no-cache"
         end
       end
 
       def nonempty_ok_response?
-        ok = !status || status.to_s[0..2] == '200'
+        ok = !@status || @status == 200
         ok && string_body?
       end
 
@@ -220,40 +224,20 @@ module ActionDispatch # :nodoc:
       end
 
       def set_conditional_cache_control!
-        if headers['Cache-Control'] == DEFAULT_HEADERS['Cache-Control']
-          headers['Cache-Control'] = 'private, max-age=0, must-revalidate'
+        if cache_control.empty?
+          cache_control.merge!(:public => false, :max_age => 0, :must_revalidate => true)
         end
-      end
 
-      def convert_content_type!
-        headers['Content-Type'] ||= "text/html"
-        headers['Content-Type'] += "; charset=" + headers.delete('charset') if headers['charset']
-      end
+        public_cache, max_age, must_revalidate, extras =
+          cache_control.values_at(:public, :max_age, :must_revalidate, :extras)
 
-      # 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
+        options = []
+        options << "max-age=#{max_age}" if max_age
+        options << (public_cache ? "public" : "private")
+        options << "must-revalidate" if must_revalidate
+        options.concat(extras) if extras
 
-      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
+        headers["Cache-Control"] = options.join(", ")
       end
   end
 end
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index acb0c895e0..d0fa67c945 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -1331,7 +1331,7 @@ class EtagRenderTest < ActionController::TestCase
   def test_render_200_should_set_etag
     get :render_hello_world_from_variable
     assert_equal etag_for("hello david"), @response.headers['ETag']
-    assert_equal "private, max-age=0, must-revalidate", @response.headers['Cache-Control']
+    assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control']
   end
 
   def test_render_against_etag_request_should_304_when_match
diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb
index ae32ee5649..0afebac68c 100644
--- a/actionpack/test/controller/send_file_test.rb
+++ b/actionpack/test/controller/send_file_test.rb
@@ -1,9 +1,10 @@
+# encoding: utf-8
 require 'abstract_unit'
 
 module TestFileUtils
   def file_name() File.basename(__FILE__) end
   def file_path() File.expand_path(__FILE__) end
-  def file_data() File.open(file_path, 'rb') { |f| f.read } end
+  def file_data() @data ||= File.open(file_path, 'rb') { |f| f.read } end
 end
 
 class SendFileController < ActionController::Base
@@ -22,6 +23,10 @@ class SendFileController < ActionController::Base
   def data
     send_data(file_data, options)
   end
+
+  def multibyte_text_data
+    send_data("Кирилица\n祝您好運", options)
+  end
 end
 
 class SendFileTest < ActionController::TestCase
@@ -55,6 +60,7 @@ class SendFileTest < ActionController::TestCase
       require 'stringio'
       output = StringIO.new
       output.binmode
+      output.string.force_encoding(file_data.encoding) if output.string.respond_to?(:force_encoding)
       assert_nothing_raised { response.body_parts.each { |part| output << part.to_s } }
       assert_equal file_data, output.string
     end
@@ -123,7 +129,7 @@ class SendFileTest < ActionController::TestCase
     # test overriding Cache-Control: no-cache header to fix IE open/save dialog
     @controller.headers = { 'Cache-Control' => 'no-cache' }
     @controller.send(:send_file_headers!, options)
-    h = @controller.headers
+    @controller.response.prepare!
     assert_equal 'private', h['Cache-Control']
   end
 
@@ -163,4 +169,11 @@ class SendFileTest < ActionController::TestCase
       assert_equal 200, @response.status
     end
   end
+
+  def test_send_data_content_length_header
+    @controller.headers = {}
+    @controller.options = { :type => :text, :filename => 'file_with_utf8_text' }
+    process('multibyte_text_data')
+    assert_equal '29', @controller.headers['Content-Length']
+  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 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest
       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 2ddc6cb2b5..256ed06a45 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -13,10 +13,9 @@ class ResponseTest < ActiveSupport::TestCase
     assert_equal 200, status
     assert_equal({
       "Content-Type" => "text/html; charset=utf-8",
-      "Cache-Control" => "private, max-age=0, must-revalidate",
+      "Cache-Control" => "max-age=0, private, must-revalidate",
       "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"',
-      "Set-Cookie" => "",
-      "Content-Length" => "13"
+      "Set-Cookie" => ""
     }, headers)
 
     parts = []
@@ -32,10 +31,9 @@ class ResponseTest < ActiveSupport::TestCase
     assert_equal 200, status
     assert_equal({
       "Content-Type" => "text/html; charset=utf-8",
-      "Cache-Control" => "private, max-age=0, must-revalidate",
+      "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 @@ module Dispatching
       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 @@ module Dispatching
       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 @@ module Dispatching
       get "/dispatching/simple/modify_response_body_twice"
 
       assert_body "success!"
-      assert_header "Content-Length", "8"
     end
 
     test "controller path" do