Shine some sunlight on the CGI extensions. Remove unused CGI#session.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6733 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

9 files changed, 278 insertions, 295 deletions

diff --git a/actionpack/lib/action_controller/cgi_ext.rb b/actionpack/lib/action_controller/cgi_ext.rb
new file mode 100644
index 0000000000..f4c8be2f76
--- /dev/null
+++ b/actionpack/lib/action_controller/cgi_ext.rb
@@ -0,0 +1,8 @@
+require 'action_controller/cgi_ext/parameters'
+require 'action_controller/cgi_ext/query_extension'
+require 'action_controller/cgi_ext/cookie'
+require 'action_controller/cgi_ext/session'
+
+class CGI #:nodoc:
+  include ActionController::CgiExt::Parameters
+end
diff --git a/actionpack/lib/action_controller/cgi_ext/cgi_ext.rb b/actionpack/lib/action_controller/cgi_ext/cgi_ext.rb
deleted file mode 100644
index 06958272ab..0000000000
--- a/actionpack/lib/action_controller/cgi_ext/cgi_ext.rb
+++ /dev/null
@@ -1,36 +0,0 @@
-require 'cgi'
-require 'cgi/session'
-require 'cgi/session/pstore'
-require 'action_controller/cgi_ext/cgi_methods'
-
-# Wrapper around the CGIMethods that have been secluded to allow testing without 
-# an instantiated CGI object
-class CGI #:nodoc:
-  class << self
-    alias :escapeHTML_fail_on_nil :escapeHTML
-
-    def escapeHTML(string)
-      escapeHTML_fail_on_nil(string) unless string.nil?
-    end
-  end
-  
-  # Returns a parameter hash including values from both the request (POST/GET)
-  # and the query string with the latter taking precedence.
-  def parameters
-    request_parameters.update(query_parameters)
-  end
-
-  def query_parameters
-    CGIMethods.parse_query_parameters(query_string)
-  end
-
-  def request_parameters
-    CGIMethods.parse_request_parameters(params, env_table)
-  end
-  
-  def session(parameters = nil)
-    parameters = {} if parameters.nil?
-    parameters['database_manager'] = CGI::Session::PStore
-    CGI::Session.new(self, parameters)
-  end
-end
diff --git a/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb b/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb
deleted file mode 100644
index 250c3272b9..0000000000
--- a/actionpack/lib/action_controller/cgi_ext/cgi_methods.rb
+++ /dev/null
@@ -1,207 +0,0 @@
-require 'cgi'
-require 'strscan'
-
-# Static methods for parsing the query and request parameters that can be used in
-# a CGI extension class or testing in isolation.
-class CGIMethods #:nodoc:
-  class << self
-    # DEPRECATED: Use parse_form_encoded_parameters
-    def parse_query_parameters(query_string)
-      pairs = query_string.split('&').collect do |chunk|
-        next if chunk.empty?
-        key, value = chunk.split('=', 2)
-        next if key.empty?
-        value = value.nil? ? nil : CGI.unescape(value)
-        [ CGI.unescape(key), value ]
-      end.compact
-
-      FormEncodedPairParser.new(pairs).result
-    end
-
-    # DEPRECATED: Use parse_form_encoded_parameters
-    def parse_request_parameters(params)
-      parser = FormEncodedPairParser.new
-
-      params = params.dup
-      until params.empty?
-        for key, value in params
-          if key.blank?
-            params.delete key
-          elsif !key.include?('[')
-            # much faster to test for the most common case first (GET)
-            # and avoid the call to build_deep_hash
-            parser.result[key] = get_typed_value(value[0])
-            params.delete key
-          elsif value.is_a?(Array)
-            parser.parse(key, get_typed_value(value.shift))
-            params.delete key if value.empty?
-          else
-            raise TypeError, "Expected array, found #{value.inspect}"
-          end
-        end
-      end
-    
-      parser.result
-    end
-
-    def parse_formatted_request_parameters(mime_type, raw_post_data)
-      case strategy = ActionController::Base.param_parsers[mime_type]
-        when Proc
-          strategy.call(raw_post_data)
-        when :xml_simple, :xml_node
-          raw_post_data.blank? ? {} : Hash.from_xml(raw_post_data).with_indifferent_access
-        when :yaml
-          YAML.load(raw_post_data)
-      end
-    rescue Exception => e # YAML, XML or Ruby code block errors
-      { "exception" => "#{e.message} (#{e.class})", "backtrace" => e.backtrace, 
-        "raw_post_data" => raw_post_data, "format" => mime_type }
-    end
-
-    private
-      def get_typed_value(value)
-        case value
-          when String
-            value
-          when NilClass
-            ''
-          when Array
-            value.map { |v| get_typed_value(v) }
-          else
-            # Uploaded file provides content type and filename.
-            if value.respond_to?(:content_type) &&
-                  !value.content_type.blank? &&
-                  !value.original_filename.blank?
-              unless value.respond_to?(:full_original_filename)
-                class << value
-                  alias_method :full_original_filename, :original_filename
-
-                  # Take the basename of the upload's original filename.
-                  # This handles the full Windows paths given by Internet Explorer
-                  # (and perhaps other broken user agents) without affecting
-                  # those which give the lone filename.
-                  # The Windows regexp is adapted from Perl's File::Basename.
-                  def original_filename
-                    if md = /^(?:.*[:\\\/])?(.*)/m.match(full_original_filename)
-                      md.captures.first
-                    else
-                      File.basename full_original_filename
-                    end
-                  end
-                end
-              end
-
-              # Return the same value after overriding original_filename.
-              value
-
-            # Multipart values may have content type, but no filename.
-            elsif value.respond_to?(:read)
-              result = value.read
-              value.rewind
-              result
-
-            # Unknown value, neither string nor multipart.
-            else
-              raise "Unknown form value: #{value.inspect}"
-            end
-        end
-      end
-  end
-
-  class FormEncodedPairParser < StringScanner #:nodoc:
-    attr_reader :top, :parent, :result
-
-    def initialize(pairs = [])
-      super('')
-      @result = {}
-      pairs.each { |key, value| parse(key, value) }
-    end
-     
-    KEY_REGEXP = %r{([^\[\]=&]+)}
-    BRACKETED_KEY_REGEXP = %r{\[([^\[\]=&]+)\]}
-    
-    # Parse the query string
-    def parse(key, value)
-      self.string = key
-      @top, @parent = result, nil
-      
-      # First scan the bare key
-      key = scan(KEY_REGEXP) or return
-      key = post_key_check(key)
-            
-      # Then scan as many nestings as present
-      until eos? 
-        r = scan(BRACKETED_KEY_REGEXP) or return
-        key = self[1]
-        key = post_key_check(key)
-      end
- 
-      bind(key, value)
-    end
-
-    private
-      # After we see a key, we must look ahead to determine our next action. Cases:
-      # 
-      #   [] follows the key. Then the value must be an array.
-      #   = follows the key. (A value comes next)
-      #   & or the end of string follows the key. Then the key is a flag.
-      #   otherwise, a hash follows the key. 
-      def post_key_check(key)
-        if scan(/\[\]/) # a[b][] indicates that b is an array
-          container(key, Array)
-          nil
-        elsif check(/\[[^\]]/) # a[b] indicates that a is a hash
-          container(key, Hash)
-          nil
-        else # End of key? We do nothing.
-          key
-        end
-      end
-    
-      # Add a container to the stack.
-      # 
-      def container(key, klass)
-        type_conflict! klass, top[key] if top.is_a?(Hash) && top.key?(key) && ! top[key].is_a?(klass)
-        value = bind(key, klass.new)
-        type_conflict! klass, value unless value.is_a?(klass)
-        push(value)
-      end
-    
-      # Push a value onto the 'stack', which is actually only the top 2 items.
-      def push(value)
-        @parent, @top = @top, value
-      end
-    
-      # Bind a key (which may be nil for items in an array) to the provided value.
-      def bind(key, value)
-        if top.is_a? Array
-          if key
-            if top[-1].is_a?(Hash) && ! top[-1].key?(key)
-              top[-1][key] = value
-            else
-              top << {key => value}.with_indifferent_access
-              push top.last
-            end
-          else
-            top << value
-          end
-        elsif top.is_a? Hash
-          key = CGI.unescape(key)
-          parent << (@top = {}) if top.key?(key) && parent.is_a?(Array)
-          return top[key] ||= value
-        else
-          raise ArgumentError, "Don't know what to do: top is #{top.inspect}"
-        end
-
-        return value
-      end
-      
-      def type_conflict!(klass, value)
-        raise TypeError, 
-          "Conflicting types for parameter containers. " +
-          "Expected an instance of #{klass}, but found an instance of #{value.class}. " +
-          "This can be caused by passing Array and Hash based paramters qs[]=value&qs[key]=value. "
-      end
-      
-    end
-end
diff --git a/actionpack/lib/action_controller/cgi_ext/cookie_performance_fix.rb b/actionpack/lib/action_controller/cgi_ext/cookie.rb
index 59b9dda396..68ac7062f8 100644
--- a/actionpack/lib/action_controller/cgi_ext/cookie_performance_fix.rb
+++ b/actionpack/lib/action_controller/cgi_ext/cookie.rb
@@ -1,9 +1,7 @@
 CGI.module_eval { remove_const "Cookie" }
 
+# TODO: document how this differs from stdlib CGI::Cookie
 class CGI #:nodoc:
-  # This is a cookie class that fixes the performance problems with the default one that ships with 1.8.1 and below.
-  # It replaces the inheritance on SimpleDelegator with DelegateClass(Array) following the suggestion from Matz on
-  # http://groups.google.com/groups?th=e3a4e68ba042f842&seekm=c3sioe%241qvm%241%40news.cybercity.dk#link14
   class Cookie < DelegateClass(Array)
     # Create a new CGI::Cookie object.
     #
@@ -19,7 +17,7 @@ class CGI #:nodoc:
     # domain:: the domain for which this cookie applies.
     # expires:: the time at which this cookie expires, as a +Time+ object.
     # secure:: whether this cookie is a secure cookie or not (default to
-    #          false).  Secure cookies are only transmitted to HTTPS 
+    #          false).  Secure cookies are only transmitted to HTTPS
     #          servers.
     #
     # These keywords correspond to attributes of the cookie object.
@@ -39,7 +37,7 @@ class CGI #:nodoc:
         @secure = name['secure'] || false
         @path = name['path']
       end
-      
+
       unless @name
         raise ArgumentError, "`name' required"
       end
diff --git a/actionpack/lib/action_controller/cgi_ext/parameters.rb b/actionpack/lib/action_controller/cgi_ext/parameters.rb
new file mode 100644
index 0000000000..639f131e00
--- /dev/null
+++ b/actionpack/lib/action_controller/cgi_ext/parameters.rb
@@ -0,0 +1,232 @@
+require 'cgi'
+require 'strscan'
+
+class CGI #:nodoc:
+  class << self
+    alias :escapeHTML_fail_on_nil :escapeHTML
+
+    def escapeHTML(string)
+      escapeHTML_fail_on_nil(string) unless string.nil?
+    end
+  end
+end
+
+module ActionController
+  module CgiExt
+    module Parameters
+      def self.included(base)
+        base.extend ClassMethods
+      end
+
+      # Merge POST and GET parameters from the request body and query string,
+      # with GET parameters taking precedence.
+      def parameters
+        request_parameters.update(query_parameters)
+      end
+
+      def query_parameters
+        self.class.parse_query_parameters(query_string)
+      end
+
+      def request_parameters
+        self.class.parse_request_parameters(params, env_table)
+      end
+
+      module ClassMethods
+        # DEPRECATED: Use parse_form_encoded_parameters
+        def parse_query_parameters(query_string)
+          pairs = query_string.split('&').collect do |chunk|
+            next if chunk.empty?
+            key, value = chunk.split('=', 2)
+            next if key.empty?
+            value = value.nil? ? nil : CGI.unescape(value)
+            [ CGI.unescape(key), value ]
+          end.compact
+
+          FormEncodedPairParser.new(pairs).result
+        end
+
+        # DEPRECATED: Use parse_form_encoded_parameters
+        def parse_request_parameters(params)
+          parser = FormEncodedPairParser.new
+
+          params = params.dup
+          until params.empty?
+            for key, value in params
+              if key.blank?
+                params.delete key
+              elsif !key.include?('[')
+                # much faster to test for the most common case first (GET)
+                # and avoid the call to build_deep_hash
+                parser.result[key] = get_typed_value(value[0])
+                params.delete key
+              elsif value.is_a?(Array)
+                parser.parse(key, get_typed_value(value.shift))
+                params.delete key if value.empty?
+              else
+                raise TypeError, "Expected array, found #{value.inspect}"
+              end
+            end
+          end
+
+          parser.result
+        end
+
+        def parse_formatted_request_parameters(mime_type, raw_post_data)
+          case strategy = ActionController::Base.param_parsers[mime_type]
+            when Proc
+              strategy.call(raw_post_data)
+            when :xml_simple, :xml_node
+              raw_post_data.blank? ? {} : Hash.from_xml(raw_post_data).with_indifferent_access
+            when :yaml
+              YAML.load(raw_post_data)
+          end
+        rescue Exception => e # YAML, XML or Ruby code block errors
+          { "exception" => "#{e.message} (#{e.class})", "backtrace" => e.backtrace,
+            "raw_post_data" => raw_post_data, "format" => mime_type }
+        end
+
+        private
+          def get_typed_value(value)
+            case value
+              when String
+                value
+              when NilClass
+                ''
+              when Array
+                value.map { |v| get_typed_value(v) }
+              else
+                # Uploaded file provides content type and filename.
+                if value.respond_to?(:content_type) &&
+                      !value.content_type.blank? &&
+                      !value.original_filename.blank?
+                  unless value.respond_to?(:full_original_filename)
+                    class << value
+                      alias_method :full_original_filename, :original_filename
+
+                      # Take the basename of the upload's original filename.
+                      # This handles the full Windows paths given by Internet Explorer
+                      # (and perhaps other broken user agents) without affecting
+                      # those which give the lone filename.
+                      # The Windows regexp is adapted from Perl's File::Basename.
+                      def original_filename
+                        if md = /^(?:.*[:\\\/])?(.*)/m.match(full_original_filename)
+                          md.captures.first
+                        else
+                          File.basename full_original_filename
+                        end
+                      end
+                    end
+                  end
+
+                  # Return the same value after overriding original_filename.
+                  value
+
+                # Multipart values may have content type, but no filename.
+                elsif value.respond_to?(:read)
+                  result = value.read
+                  value.rewind
+                  result
+
+                # Unknown value, neither string nor multipart.
+                else
+                  raise "Unknown form value: #{value.inspect}"
+                end
+            end
+          end
+      end
+
+      class FormEncodedPairParser < StringScanner #:nodoc:
+        attr_reader :top, :parent, :result
+
+        def initialize(pairs = [])
+          super('')
+          @result = {}
+          pairs.each { |key, value| parse(key, value) }
+        end
+
+        KEY_REGEXP = %r{([^\[\]=&]+)}
+        BRACKETED_KEY_REGEXP = %r{\[([^\[\]=&]+)\]}
+
+        # Parse the query string
+        def parse(key, value)
+          self.string = key
+          @top, @parent = result, nil
+
+          # First scan the bare key
+          key = scan(KEY_REGEXP) or return
+          key = post_key_check(key)
+
+          # Then scan as many nestings as present
+          until eos?
+            r = scan(BRACKETED_KEY_REGEXP) or return
+            key = self[1]
+            key = post_key_check(key)
+          end
+
+          bind(key, value)
+        end
+
+        private
+          # After we see a key, we must look ahead to determine our next action. Cases:
+          #
+          #   [] follows the key. Then the value must be an array.
+          #   = follows the key. (A value comes next)
+          #   & or the end of string follows the key. Then the key is a flag.
+          #   otherwise, a hash follows the key.
+          def post_key_check(key)
+            if scan(/\[\]/) # a[b][] indicates that b is an array
+              container(key, Array)
+              nil
+            elsif check(/\[[^\]]/) # a[b] indicates that a is a hash
+              container(key, Hash)
+              nil
+            else # End of key? We do nothing.
+              key
+            end
+          end
+
+          # Add a container to the stack.
+          def container(key, klass)
+            type_conflict! klass, top[key] if top.is_a?(Hash) && top.key?(key) && ! top[key].is_a?(klass)
+            value = bind(key, klass.new)
+            type_conflict! klass, value unless value.is_a?(klass)
+            push(value)
+          end
+
+          # Push a value onto the 'stack', which is actually only the top 2 items.
+          def push(value)
+            @parent, @top = @top, value
+          end
+
+          # Bind a key (which may be nil for items in an array) to the provided value.
+          def bind(key, value)
+            if top.is_a? Array
+              if key
+                if top[-1].is_a?(Hash) && ! top[-1].key?(key)
+                  top[-1][key] = value
+                else
+                  top << {key => value}.with_indifferent_access
+                  push top.last
+                end
+              else
+                top << value
+              end
+            elsif top.is_a? Hash
+              key = CGI.unescape(key)
+              parent << (@top = {}) if top.key?(key) && parent.is_a?(Array)
+              return top[key] ||= value
+            else
+              raise ArgumentError, "Don't know what to do: top is #{top.inspect}"
+            end
+
+            return value
+          end
+
+          def type_conflict!(klass, value)
+            raise TypeError, "Conflicting types for parameter containers. Expected an instance of #{klass} but found an instance of #{value.class}. This can be caused by colliding Array and Hash parameters like qs[]=value&qs[key]=value."
+          end
+      end
+    end
+  end
+end
diff --git a/actionpack/lib/action_controller/cgi_ext/pstore_performance_fix.rb b/actionpack/lib/action_controller/cgi_ext/pstore_performance_fix.rb
deleted file mode 100644
index 79e427a350..0000000000
--- a/actionpack/lib/action_controller/cgi_ext/pstore_performance_fix.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-# CGI::Session::PStore.initialize requires 'digest/md5' on every call.
-# This makes sense when spawning processes per request, but is
-# unnecessarily expensive when serving requests from a long-lived
-# process.
-require 'cgi/session'
-require 'cgi/session/pstore'
-require 'digest/md5'
-
-class CGI::Session::PStore #:nodoc:
-  def initialize(session, option={})
-    dir = option['tmpdir'] || Dir::tmpdir
-    prefix = option['prefix'] || ''
-    id = session.session_id
-    md5 = Digest::MD5.hexdigest(id)[0,16]
-    path = dir+"/"+prefix+md5
-    path.untaint
-    if File::exist?(path)
-      @hash = nil
-    else
-      unless session.new_session
-        raise CGI::Session::NoSession, "uninitialized session"
-      end
-      @hash = {}
-    end
-    @p = ::PStore.new(path)
-    @p.transaction do |p|
-      File.chmod(0600, p.path)
-    end
-  end
-end
diff --git a/actionpack/lib/action_controller/cgi_ext/raw_post_data_fix.rb b/actionpack/lib/action_controller/cgi_ext/query_extension.rb
index 13e4586afa..9e836eaaf8 100644
--- a/actionpack/lib/action_controller/cgi_ext/raw_post_data_fix.rb
+++ b/actionpack/lib/action_controller/cgi_ext/query_extension.rb
@@ -1,3 +1,5 @@
+require 'cgi'
+
 class CGI #:nodoc:
   module QueryExtension
     # Remove the old initialize_query method before redefining it.
diff --git a/actionpack/lib/action_controller/cgi_ext/session_performance_fix.rb b/actionpack/lib/action_controller/cgi_ext/session.rb
index d3dc643d3f..0213ce91bd 100644
--- a/actionpack/lib/action_controller/cgi_ext/session_performance_fix.rb
+++ b/actionpack/lib/action_controller/cgi_ext/session.rb
@@ -1,14 +1,10 @@
-# CGI::Session#create_new_id requires 'digest/md5' on every call.  This makes
-# sense when spawning processes per request, but is unnecessarily expensive
-# when serving requests from a long-lived process.
-#
-# http://railsexpress.de/blog/articles/2005/11/22/speeding-up-the-creation-of-new-sessions
-#
-# Also expose the CGI instance to session stores.
-require 'cgi/session'
 require 'digest/md5'
+require 'cgi/session'
+require 'cgi/session/pstore'
 
-class CGI
+class CGI #:nodoc:
+  # * Expose the CGI instance to session stores.
+  # * Don't require 'digest/md5' whenever a new session id is generated.
   class Session #:nodoc:
     # Generate an MD5 hash including the time, a random number, the process id,
     # and a constant string. This is used to generate session ids but may be
@@ -39,5 +35,29 @@ class CGI
         @new_session = true
         self.class.generate_unique_id
       end
+
+    # * Don't require 'digest/md5' whenever a new session is started.
+    class PStore #:nodoc:
+      def initialize(session, option={})
+        dir = option['tmpdir'] || Dir::tmpdir
+        prefix = option['prefix'] || ''
+        id = session.session_id
+        md5 = Digest::MD5.hexdigest(id)[0,16]
+        path = dir+"/"+prefix+md5
+        path.untaint
+        if File::exist?(path)
+          @hash = nil
+        else
+          unless session.new_session
+            raise CGI::Session::NoSession, "uninitialized session"
+          end
+          @hash = {}
+        end
+        @p = ::PStore.new(path)
+        @p.transaction do |p|
+          File.chmod(0600, p.path)
+        end
+      end
+    end
   end
 end
diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb
index ddcc6003d0..59849c7364 100644
--- a/actionpack/lib/action_controller/cgi_process.rb
+++ b/actionpack/lib/action_controller/cgi_process.rb
@@ -1,8 +1,4 @@
-require 'action_controller/cgi_ext/cgi_ext'
-require 'action_controller/cgi_ext/cookie_performance_fix'
-require 'action_controller/cgi_ext/raw_post_data_fix'
-require 'action_controller/cgi_ext/session_performance_fix'
-require 'action_controller/cgi_ext/pstore_performance_fix'
+require 'action_controller/cgi_ext'
 require 'action_controller/session/cookie_store'
 
 module ActionController #:nodoc:
@@ -64,15 +60,15 @@ module ActionController #:nodoc:
 
     def query_parameters
       @query_parameters ||=
-        (qs = self.query_string).empty? ? {} : CGIMethods.parse_query_parameters(qs)
+        (qs = self.query_string).empty? ? {} : CGI.parse_query_parameters(qs)
     end
 
     def request_parameters
       @request_parameters ||=
         if ActionController::Base.param_parsers.has_key?(content_type)
-          CGIMethods.parse_formatted_request_parameters(content_type, @env['RAW_POST_DATA'])
+          CGI.parse_formatted_request_parameters(content_type, @env['RAW_POST_DATA'])
         else
-          CGIMethods.parse_request_parameters(@cgi.params)
+          CGI.parse_request_parameters(@cgi.params)
         end
     end