Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Introduce the request.body stream. Lazy-read to parse parameters rath…

…er than always setting RAW_POST_DATA. Reduces the memory footprint of large binary PUT requests.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6740 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 32d03af341eba4d0b263156f0241016b857c4d84 1 parent 9e3a51e
@jeremy jeremy authored
View
2  actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Introduce the request.body stream. Lazy-read to parse parameters rather than always setting RAW_POST_DATA. Reduces the memory footprint of large binary PUT requests. [Jeremy Kemper]
+
* Add some performance enhancements to ActionView.
* Cache base_paths in @@cached_base_paths
View
10 actionpack/lib/action_controller/cgi_ext.rb
@@ -1,8 +1,18 @@
+require 'action_controller/cgi_ext/stdinput'
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::Stdinput
include ActionController::CgiExt::Parameters
+
+ class << self
+ alias :escapeHTML_fail_on_nil :escapeHTML
+
+ def escapeHTML(string)
+ escapeHTML_fail_on_nil(string) unless string.nil?
+ end
+ end
end
View
20 actionpack/lib/action_controller/cgi_ext/parameters.rb
@@ -1,16 +1,6 @@
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
@@ -72,18 +62,18 @@ def parse_request_parameters(params)
parser.result
end
- def parse_formatted_request_parameters(mime_type, raw_post_data)
+ def parse_formatted_request_parameters(mime_type, body)
case strategy = ActionController::Base.param_parsers[mime_type]
when Proc
- strategy.call(raw_post_data)
+ strategy.call(body)
when :xml_simple, :xml_node
- raw_post_data.blank? ? {} : Hash.from_xml(raw_post_data).with_indifferent_access
+ body.blank? ? {} : Hash.from_xml(body).with_indifferent_access
when :yaml
- YAML.load(raw_post_data)
+ YAML.load(body)
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 }
+ "body" => body, "format" => mime_type }
end
private
View
10 actionpack/lib/action_controller/cgi_ext/query_extension.rb
@@ -31,18 +31,14 @@ def initialize_query
# Set multipart to false by default.
@multipart = false
- # POST and PUT may have params in entity body. If content type is
- # missing for POST, assume urlencoded. If content type is missing
- # for PUT, don't assume anything and don't parse the parameters:
- # it's likely binary data.
- #
- # The other HTTP methods have their params in the query string.
+ # POST and PUT may have params in entity body. If content type is missing
+ # or non-urlencoded, don't read the body or parse parameters: assume it's
+ # binary data.
if method == :post || method == :put
if boundary = extract_multipart_form_boundary(content_type)
@multipart = true
@params = read_multipart(boundary, content_length)
elsif content_type.blank? || content_type !~ %r{application/x-www-form-urlencoded}i
- read_params(method, content_length)
@params = {}
end
end
View
23 actionpack/lib/action_controller/cgi_ext/stdinput.rb
@@ -0,0 +1,23 @@
+require 'cgi'
+
+module ActionController
+ module CgiExt
+ # Publicize the CGI's internal input stream so we can lazy-read
+ # request.body. Make it writable so we don't have to play $stdin games.
+ module Stdinput
+ def self.included(base)
+ base.class_eval do
+ remove_method :stdinput
+ attr_accessor :stdinput
+ end
+
+ base.alias_method_chain :initialize, :stdinput
+ end
+
+ def initialize_with_stdinput(type = nil, stdinput = $stdin)
+ @stdinput = stdinput
+ initialize_without_stdinput(type || 'query')
+ end
+ end
+ end
+end
View
12 actionpack/lib/action_controller/cgi_process.rb
@@ -58,6 +58,16 @@ def query_string
end
end
+ # The request body is an IO input stream. If the RAW_POST_DATA environment
+ # variable is already set, wrap it in a StringIO.
+ def body
+ if raw_post = env['RAW_POST_DATA']
+ StringIO.new(raw_post)
+ else
+ @cgi.stdinput
+ end
+ end
+
def query_parameters
@query_parameters ||=
(qs = self.query_string).empty? ? {} : CGI.parse_query_parameters(qs)
@@ -66,7 +76,7 @@ def query_parameters
def request_parameters
@request_parameters ||=
if ActionController::Base.param_parsers.has_key?(content_type)
- CGI.parse_formatted_request_parameters(content_type, @env['RAW_POST_DATA'])
+ CGI.parse_formatted_request_parameters(content_type, body.read)
else
CGI.parse_request_parameters(@cgi.params)
end
View
34 actionpack/lib/action_controller/request.rb
@@ -1,24 +1,27 @@
module ActionController
- # Subclassing AbstractRequest makes these methods available to the request objects used in production and testing,
- # CgiRequest and TestRequest
+ # CgiRequest and TestRequest provide concrete implementations.
class AbstractRequest
cattr_accessor :relative_url_root
remove_method :relative_url_root
- # Returns the hash of environment variables for this request,
+ # The hash of environment variables for this request,
# such as { 'RAILS_ENV' => 'production' }.
attr_reader :env
- attr_accessor :format
+ # The requested content type, such as :html or :xml.
+ attr_writer :format
- # Returns the HTTP request method as a lowercase symbol (:get, for example). Note, HEAD is returned as :get
- # since the two are supposedly to be functionaly equivilent for all purposes except that HEAD won't return a response
- # body (which Rails also takes care of elsewhere).
+ # The HTTP request method as a lowercase symbol, such as :get.
+ # Note, HEAD is returned as :get since the two are functionally
+ # equivalent from the application's perspective.
def method
- @request_method ||= (!parameters[:_method].blank? && @env['REQUEST_METHOD'] == 'POST') ?
- parameters[:_method].to_s.downcase.to_sym :
- @env['REQUEST_METHOD'].downcase.to_sym
-
+ @request_method ||=
+ if @env['REQUEST_METHOD'] == 'POST' && !parameters[:_method].blank?
+ parameters[:_method].to_s.downcase.to_sym
+ else
+ @env['REQUEST_METHOD'].downcase.to_sym
+ end
+
@request_method == :head ? :get : @request_method
end
@@ -42,8 +45,8 @@ def delete?
method == :delete
end
- # Is this a HEAD request? HEAD is mapped as :get for request.method, so here we ask the
- # REQUEST_METHOD header directly. Thus, for head, both get? and head? will return true.
+ # Is this a HEAD request? request.method sees HEAD as :get, so check the
+ # HTTP method directly.
def head?
@env['REQUEST_METHOD'].downcase.to_sym == :head
end
@@ -269,6 +272,11 @@ def path_parameters
#--
# Must be implemented in the concrete request
#++
+
+ # The request body is an IO input stream.
+ def body
+ end
+
def query_parameters #:nodoc:
end
View
30 actionpack/lib/action_controller/test_process.rb
@@ -40,18 +40,15 @@ def reset_session
@session = TestSession.new
end
+ # Wraps raw_post in a StringIO.
+ def body
+ StringIO.new(raw_post)
+ end
+
+ # Either the RAW_POST_DATA environment variable or the URL-encoded request
+ # parameters.
def raw_post
- if raw_post = env['RAW_POST_DATA']
- raw_post
- else
- params = self.request_parameters.dup
- %w(controller action only_path).each do |k|
- params.delete(k)
- params.delete(k.to_sym)
- end
-
- params.map { |k,v| [ CGI.escape(k.to_s), CGI.escape(v.to_s) ].join('=') }.sort.join('&')
- end
+ env['RAW_POST_DATA'] ||= url_encoded_request_parameters
end
def port=(number)
@@ -140,6 +137,17 @@ def initialize_default_values
@env["SERVER_PORT"] = 80
@env['REQUEST_METHOD'] = "GET"
end
+
+ def url_encoded_request_parameters
+ params = self.request_parameters.dup
+
+ %w(controller action only_path).each do |k|
+ params.delete(k)
+ params.delete(k.to_sym)
+ end
+
+ params.to_query
+ end
end
# A refactoring of TestResponse to allow the same behavior to be applied
View
2  actionpack/test/abstract_unit.rb
@@ -3,8 +3,10 @@
$:.unshift(File.dirname(__FILE__) + '/fixtures/helpers')
require 'yaml'
+require 'stringio'
require 'test/unit'
require 'action_controller'
+require 'action_controller/cgi_ext'
require 'action_controller/test_process'
# Show backtraces for deprecated behavior for quicker cleanup.
View
53 actionpack/test/controller/raw_post_test.rb
@@ -1,6 +1,4 @@
require "#{File.dirname(__FILE__)}/../abstract_unit"
-require 'stringio'
-require 'action_controller/cgi_ext/query_extension'
class RawPostDataTest < Test::Unit::TestCase
def setup
@@ -11,57 +9,66 @@ def setup
def test_post_with_urlencoded_body
ENV['REQUEST_METHOD'] = 'POST'
ENV['CONTENT_TYPE'] = ' apPlication/x-Www-form-urlEncoded; charset=utf-8'
- assert_equal ['1'], cgi_params['a']
- assert_has_raw_post_data
+ assert_equal ['1'], cgi.params['a']
+ assert_raw_post_data
end
def test_post_with_empty_content_type_treated_as_urlencoded
ENV['REQUEST_METHOD'] = 'POST'
ENV['CONTENT_TYPE'] = ''
- assert_equal ['1'], cgi_params['a']
- assert_has_raw_post_data
+ assert_equal ['1'], cgi.params['a']
+ assert_raw_post_data
end
- def test_post_with_unrecognized_content_type_reads_body_but_doesnt_parse_params
+ def test_post_with_unrecognized_content_type_ignores_body
ENV['REQUEST_METHOD'] = 'POST'
ENV['CONTENT_TYPE'] = 'foo/bar'
- assert cgi_params.empty?
- assert_has_raw_post_data
+ assert cgi.params.empty?
+ assert_no_raw_post_data
end
def test_put_with_urlencoded_body
ENV['REQUEST_METHOD'] = 'PUT'
ENV['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'
- assert_equal ['1'], cgi_params['a']
- assert_has_raw_post_data
+ assert_equal ['1'], cgi.params['a']
+ assert_raw_post_data
end
def test_put_with_empty_content_type_ignores_body
ENV['REQUEST_METHOD'] = 'PUT'
ENV['CONTENT_TYPE'] = ''
- assert cgi_params.empty?
- assert_has_raw_post_data
+ assert cgi.params.empty?
+ assert_no_raw_post_data
end
def test_put_with_unrecognized_content_type_ignores_body
ENV['REQUEST_METHOD'] = 'PUT'
ENV['CONTENT_TYPE'] = 'foo/bar'
- assert cgi_params.empty?
- assert_has_raw_post_data
+ assert cgi.params.empty?
+ assert_no_raw_post_data
end
private
- def cgi_params
- old_stdin, $stdin = $stdin, StringIO.new(@request_body.dup)
- ENV['CONTENT_LENGTH'] = $stdin.size.to_s
- CGI.new.params
- ensure
- $stdin = old_stdin
+ def cgi
+ unless defined? @cgi
+ ENV['CONTENT_LENGTH'] = @request_body.size.to_s
+ @cgi = CGI.new('query', StringIO.new(@request_body.dup))
+ end
+
+ @cgi
end
- def assert_has_raw_post_data(expected_body = @request_body)
+ def assert_raw_post_data
assert_not_nil ENV['RAW_POST_DATA']
assert ENV['RAW_POST_DATA'].frozen?
- assert_equal expected_body, ENV['RAW_POST_DATA']
+ assert_equal @request_body, ENV['RAW_POST_DATA']
+
+ assert_equal '', cgi.stdinput.read
+ end
+
+ def assert_no_raw_post_data
+ assert_nil ENV['RAW_POST_DATA']
+
+ assert_equal @request_body, cgi.stdinput.read
end
end
View
15 actionpack/test/controller/test_test.rb
@@ -13,6 +13,10 @@ def render_raw_post
render :text => request.raw_post
end
+ def render_body
+ render :text => request.body.read
+ end
+
def test_params
render :text => params.inspect
end
@@ -93,8 +97,15 @@ def test_raw_post_handling
params = {:page => {:name => 'page name'}, 'some key' => 123}
get :render_raw_post, params.dup
- raw_post = params.map {|k,v| [CGI::escape(k.to_s), CGI::escape(v.to_s)].join('=')}.sort.join('&')
- assert_equal raw_post, @response.body
+ assert_equal params.to_query, @response.body
+ end
+
+ def test_body_stream
+ params = { :page => { :name => 'page name' }, 'some key' => 123 }
+
+ get :render_body, params.dup
+
+ assert_equal params.to_query, @response.body
end
def test_process_without_flash
View
10 actionpack/test/controller/webservice_test.rb
@@ -1,20 +1,16 @@
require File.dirname(__FILE__) + '/../abstract_unit'
-require 'stringio'
class WebServiceTest < Test::Unit::TestCase
-
class MockCGI < CGI #:nodoc:
- attr_accessor :stdinput, :stdoutput, :env_table
+ attr_accessor :stdoutput, :env_table
- def initialize(env, data = '')
+ def initialize(env, data = '')
self.env_table = env
- self.stdinput = StringIO.new(data)
self.stdoutput = StringIO.new
- super()
+ super(nil, StringIO.new(data))
end
end
-
class TestController < ActionController::Base
session :off
Please sign in to comment.
Something went wrong with that request. Please try again.