diff --git a/CHANGELOG.md b/CHANGELOG.md
index f93d6b340..c852a84a4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,16 @@
All notable changes to this project will be documented in this file. For info on how to format all future additions to this file please reference [Keep A Changelog](https://keepachangelog.com/en/1.0.0/).
+## [Unreleased]
+
+### SPEC Changes
+
+- `rack.input` is now optional. ([#1997](https://github.com/rack/rack/pull/1997), [@ioquatix])
+
+### Changed
+
+- Improved handling of multipart requests. `rack.input` is now optional, and if missing, will raise an error which includes `module Rack::BadRequest`. Several other exceptions also include this module. ([#1997](https://github.com/rack/rack/pull/1997), [@ioquatix])
+
## [3.0.3] - 2022-12-07
### Fixed
diff --git a/SPEC.rdoc b/SPEC.rdoc
index ddf474ae1..5c49cc7ef 100644
--- a/SPEC.rdoc
+++ b/SPEC.rdoc
@@ -121,7 +121,7 @@ If the string values for CGI keys contain non-ASCII characters,
they should use ASCII-8BIT encoding.
There are the following restrictions:
* rack.url_scheme must either be +http+ or +https+.
-* There must be a valid input stream in rack.input.
+* There may be a valid input stream in rack.input.
* There must be a valid error stream in rack.errors.
* There may be a valid hijack callback in rack.hijack
* The REQUEST_METHOD must be a valid token.
diff --git a/lib/rack.rb b/lib/rack.rb
index 5b87ea1bc..2dd8d3dc2 100644
--- a/lib/rack.rb
+++ b/lib/rack.rb
@@ -15,6 +15,7 @@
require_relative 'rack/constants'
module Rack
+ autoload :BadRequest, "rack/bad_request"
autoload :Builder, "rack/builder"
autoload :BodyProxy, "rack/body_proxy"
autoload :Cascade, "rack/cascade"
diff --git a/lib/rack/bad_request.rb b/lib/rack/bad_request.rb
new file mode 100644
index 000000000..300811a3c
--- /dev/null
+++ b/lib/rack/bad_request.rb
@@ -0,0 +1,4 @@
+module Rack
+ module BadRequest
+ end
+end
diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb
index ee3ec7161..dc78b94d8 100644
--- a/lib/rack/lint.rb
+++ b/lib/rack/lint.rb
@@ -56,9 +56,6 @@ def response
raise LintError, "No env given" unless @env
check_environment(@env)
- @env[RACK_INPUT] = InputWrapper.new(@env[RACK_INPUT])
- @env[RACK_ERRORS] = ErrorWrapper.new(@env[RACK_ERRORS])
-
## and returns a non-frozen Array of exactly three values:
@response = @app.call(@env)
raise LintError, "response is not an Array, but #{@response.class}" unless @response.kind_of? Array
@@ -265,11 +262,9 @@ def check_environment(env)
## is reserved for use with the Rack core distribution and other
## accepted specifications and must not be used otherwise.
##
-
- %w[REQUEST_METHOD SERVER_NAME QUERY_STRING SERVER_PROTOCOL
- rack.input rack.errors].each { |header|
+ %w[REQUEST_METHOD SERVER_NAME QUERY_STRING SERVER_PROTOCOL rack.errors].each do |header|
raise LintError, "env missing required key #{header}" unless env.include? header
- }
+ end
## The SERVER_PORT must be an Integer if set.
server_port = env["SERVER_PORT"]
@@ -328,10 +323,17 @@ def check_environment(env)
raise LintError, "rack.url_scheme unknown: #{env[RACK_URL_SCHEME].inspect}"
end
- ## * There must be a valid input stream in rack.input.
- check_input env[RACK_INPUT]
+ ## * There may be a valid input stream in rack.input.
+ if rack_input = env[RACK_INPUT]
+ check_input_stream(rack_input)
+ @env[RACK_INPUT] = InputWrapper.new(rack_input)
+ end
+
## * There must be a valid error stream in rack.errors.
- check_error env[RACK_ERRORS]
+ rack_errors = env[RACK_ERRORS]
+ check_error_stream(rack_errors)
+ @env[RACK_ERRORS] = ErrorWrapper.new(rack_errors)
+
## * There may be a valid hijack callback in rack.hijack
check_hijack env
@@ -384,7 +386,7 @@ def check_environment(env)
##
## The input stream is an IO-like object which contains the raw HTTP
## POST data.
- def check_input(input)
+ def check_input_stream(input)
## When applicable, its external encoding must be "ASCII-8BIT" and it
## must be opened in binary mode, for Ruby 1.9 compatibility.
if input.respond_to?(:external_encoding) && input.external_encoding != Encoding::ASCII_8BIT
@@ -488,7 +490,7 @@ def close(*args)
##
## === The Error Stream
##
- def check_error(error)
+ def check_error_stream(error)
## The error stream must respond to +puts+, +write+ and +flush+.
[:puts, :write, :flush].each { |method|
unless error.respond_to? method
diff --git a/lib/rack/mock_request.rb b/lib/rack/mock_request.rb
index b6d7ef4fe..8e1116538 100644
--- a/lib/rack/mock_request.rb
+++ b/lib/rack/mock_request.rb
@@ -41,11 +41,6 @@ def string
end
end
- DEFAULT_ENV = {
- RACK_INPUT => StringIO.new,
- RACK_ERRORS => StringIO.new,
- }.freeze
-
def initialize(app)
@app = app
end
@@ -104,7 +99,7 @@ def self.env_for(uri = "", opts = {})
uri = parse_uri_rfc2396(uri)
uri.path = "/#{uri.path}" unless uri.path[0] == ?/
- env = DEFAULT_ENV.dup
+ env = {}
env[REQUEST_METHOD] = (opts[:method] ? opts[:method].to_s.upcase : GET).b
env[SERVER_NAME] = (uri.host || "example.org").b
@@ -144,20 +139,21 @@ def self.env_for(uri = "", opts = {})
end
end
- opts[:input] ||= String.new
if String === opts[:input]
rack_input = StringIO.new(opts[:input])
else
rack_input = opts[:input]
end
- rack_input.set_encoding(Encoding::BINARY)
- env[RACK_INPUT] = rack_input
+ if rack_input
+ rack_input.set_encoding(Encoding::BINARY)
+ env[RACK_INPUT] = rack_input
- env["CONTENT_LENGTH"] ||= env[RACK_INPUT].size.to_s if env[RACK_INPUT].respond_to?(:size)
+ env["CONTENT_LENGTH"] ||= env[RACK_INPUT].size.to_s if env[RACK_INPUT].respond_to?(:size)
+ end
opts.each { |field, value|
- env[field] = value if String === field
+ env[field] = value if String === field
}
env
diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb
index 3347662ac..165b4db3a 100644
--- a/lib/rack/multipart.rb
+++ b/lib/rack/multipart.rb
@@ -6,6 +6,8 @@
require_relative 'multipart/parser'
require_relative 'multipart/generator'
+require_relative 'bad_request'
+
module Rack
# A multipart form data parser, adapted from IOWA.
#
@@ -13,9 +15,15 @@ module Rack
module Multipart
MULTIPART_BOUNDARY = "AaB03x"
+ class MissingInputError < StandardError
+ include BadRequest
+ end
+
class << self
def parse_multipart(env, params = Rack::Utils.default_query_parser)
- io = env[RACK_INPUT]
+ unless io = env[RACK_INPUT]
+ raise MissingInputError, "Missing input stream!"
+ end
if content_length = env['CONTENT_LENGTH']
content_length = content_length.to_i
diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb
index 480badaf2..995846947 100644
--- a/lib/rack/multipart/parser.rb
+++ b/lib/rack/multipart/parser.rb
@@ -3,18 +3,28 @@
require 'strscan'
require_relative '../utils'
+require_relative '../bad_request'
module Rack
module Multipart
- class MultipartPartLimitError < Errno::EMFILE; end
+ class MultipartPartLimitError < Errno::EMFILE
+ include BadRequest
+ end
# Use specific error class when parsing multipart request
# that ends early.
- class EmptyContentError < ::EOFError; end
+ class EmptyContentError < ::EOFError
+ include BadRequest
+ end
# Base class for multipart exceptions that do not subclass from
# other exception classes for backwards compatibility.
- class Error < StandardError; end
+ class BoundaryTooLongError < StandardError
+ include BadRequest
+ end
+
+ Error = BoundaryTooLongError
+ deprecate_constant :Error
EOL = "\r\n"
MULTIPART = %r|\Amultipart/.*boundary=\"?([^\";,]+)\"?|ni
@@ -96,7 +106,7 @@ def self.parse(io, content_length, content_type, tmpfile, bufsize, qp)
if boundary.length > 70
# RFC 1521 Section 7.2.1 imposes a 70 character maximum for the boundary.
# Most clients use no more than 55 characters.
- raise Error, "multipart boundary size too large (#{boundary.length} characters)"
+ raise BoundaryTooLongError, "multipart boundary size too large (#{boundary.length} characters)"
end
io = BoundedIO.new(io, content_length) if content_length
diff --git a/lib/rack/query_parser.rb b/lib/rack/query_parser.rb
index a89c4c7f4..7c9621ed5 100644
--- a/lib/rack/query_parser.rb
+++ b/lib/rack/query_parser.rb
@@ -1,5 +1,7 @@
# frozen_string_literal: true
+require_relative 'bad_request'
+
module Rack
class QueryParser
DEFAULT_SEP = /[&] */n
@@ -7,16 +9,22 @@ class QueryParser
# ParameterTypeError is the error that is raised when incoming structural
# parameters (parsed by parse_nested_query) contain conflicting types.
- class ParameterTypeError < TypeError; end
+ class ParameterTypeError < TypeError
+ include BadRequest
+ end
# InvalidParameterError is the error that is raised when incoming structural
# parameters (parsed by parse_nested_query) contain invalid format or byte
# sequence.
- class InvalidParameterError < ArgumentError; end
+ class InvalidParameterError < ArgumentError
+ include BadRequest
+ end
# ParamsTooDeepError is the error that is raised when params are recursively
# nested over the specified limit.
- class ParamsTooDeepError < RangeError; end
+ class ParamsTooDeepError < RangeError
+ include BadRequest
+ end
def self.make_default(param_depth_limit)
new Params, param_depth_limit
diff --git a/lib/rack/request.rb b/lib/rack/request.rb
index 40922a219..c364cca2f 100644
--- a/lib/rack/request.rb
+++ b/lib/rack/request.rb
@@ -469,9 +469,10 @@ def content_charset
# content-type header is provided and the request_method is POST.
def form_data?
type = media_type
- meth = get_header(RACK_METHODOVERRIDE_ORIGINAL_METHOD) || get_header(REQUEST_METHOD)
- (meth == POST && type.nil?) || FORM_DATA_MEDIA_TYPES.include?(type)
+ request_method = get_header(RACK_METHODOVERRIDE_ORIGINAL_METHOD) || get_header(REQUEST_METHOD)
+
+ (request_method == POST && type.nil?) || FORM_DATA_MEDIA_TYPES.include?(type)
end
# Determine whether the request body contains data by checking
@@ -501,10 +502,20 @@ def POST
end
begin
- if get_header(RACK_INPUT).nil?
- raise "Missing rack.input"
- elsif get_header(RACK_REQUEST_FORM_INPUT) == get_header(RACK_INPUT)
- get_header(RACK_REQUEST_FORM_HASH)
+ rack_input = get_header(RACK_INPUT)
+
+ # If the form hash was already memoized:
+ if form_hash = get_header(RACK_REQUEST_FORM_HASH)
+ # And it was memoized from the same input:
+ if get_header(RACK_REQUEST_FORM_INPUT).equal?(rack_input)
+ return form_hash
+ end
+ end
+
+ # Otherwise, figure out how to parse the input:
+ if rack_input.nil?
+ set_header RACK_REQUEST_FORM_INPUT, nil
+ set_header(RACK_REQUEST_FORM_HASH, {})
elsif form_data? || parseable_data?
unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart)
form_vars = get_header(RACK_INPUT).read
@@ -516,6 +527,7 @@ def POST
set_header RACK_REQUEST_FORM_VARS, form_vars
set_header RACK_REQUEST_FORM_HASH, parse_query(form_vars, '&')
end
+
set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT)
get_header RACK_REQUEST_FORM_HASH
else
diff --git a/test/spec_lint.rb b/test/spec_lint.rb
index 081ff367b..f7e5852aa 100644
--- a/test/spec_lint.rb
+++ b/test/spec_lint.rb
@@ -13,8 +13,12 @@
[200, { "content-type" => "test/plain", "content-length" => "3" }, ["foo"]]
end
- def env(*args)
- Rack::MockRequest.env_for("/", *args)
+ def env(options = {})
+ unless options.key?(:input)
+ options[:input] = String.new
+ end
+
+ Rack::MockRequest.env_for("/", options)
end
it "pass valid request" do
diff --git a/test/spec_mock_request.rb b/test/spec_mock_request.rb
index b2a16fded..b702cea55 100644
--- a/test/spec_mock_request.rb
+++ b/test/spec_mock_request.rb
@@ -14,7 +14,10 @@
app = Rack::Lint.new(lambda { |env|
req = Rack::Request.new(env)
- env["mock.postdata"] = env["rack.input"].read
+ if input = env["rack.input"]
+ env["mock.postdata"] = input.read
+ end
+
if req.GET["error"]
env["rack.errors"].puts req.GET["error"]
env["rack.errors"].flush
@@ -46,7 +49,7 @@
end
it "should handle a non-GET request with both :input and :params" do
- env = Rack::MockRequest.env_for("/", method: :post, input: nil, params: {})
+ env = Rack::MockRequest.env_for("/", method: :post, input: "", params: {})
env["PATH_INFO"].must_equal "/"
env.must_be_kind_of Hash
env['rack.input'].read.must_equal ''
@@ -71,7 +74,7 @@
env["PATH_INFO"].must_equal "/"
env["SCRIPT_NAME"].must_equal ""
env["rack.url_scheme"].must_equal "http"
- env["mock.postdata"].must_be :empty?
+ env["mock.postdata"].must_be_nil
end
it "allow GET/POST/PUT/DELETE/HEAD" do
@@ -194,7 +197,7 @@
env["QUERY_STRING"].must_include "baz=2"
env["QUERY_STRING"].must_include "foo%5Bbar%5D=1"
env["PATH_INFO"].must_equal "/foo"
- env["mock.postdata"].must_equal ""
+ env["mock.postdata"].must_be_nil
end
it "accept raw input in params for GET requests" do
@@ -204,7 +207,7 @@
env["QUERY_STRING"].must_include "baz=2"
env["QUERY_STRING"].must_include "foo%5Bbar%5D=1"
env["PATH_INFO"].must_equal "/foo"
- env["mock.postdata"].must_equal ""
+ env["mock.postdata"].must_be_nil
end
it "accept params and build url encoded params for POST requests" do
diff --git a/test/spec_mock_response.rb b/test/spec_mock_response.rb
index 5cc4fe764..0704ef95a 100644
--- a/test/spec_mock_response.rb
+++ b/test/spec_mock_response.rb
@@ -14,7 +14,10 @@
app = Rack::Lint.new(lambda { |env|
req = Rack::Request.new(env)
- env["mock.postdata"] = env["rack.input"].read
+ if input = env["rack.input"]
+ env["mock.postdata"] = input.read
+ end
+
if req.GET["error"]
env["rack.errors"].puts req.GET["error"]
env["rack.errors"].flush
diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb
index 43bb90b2d..f18da1fe3 100644
--- a/test/spec_multipart.rb
+++ b/test/spec_multipart.rb
@@ -30,8 +30,7 @@ def multipart_file(name)
end
it "return nil if content type is not multipart" do
- env = Rack::MockRequest.env_for("/",
- "CONTENT_TYPE" => 'application/x-www-form-urlencoded')
+ env = Rack::MockRequest.env_for("/", "CONTENT_TYPE" => 'application/x-www-form-urlencoded', :input => "")
Rack::Multipart.parse_multipart(env).must_be_nil
end
@@ -39,7 +38,14 @@ def multipart_file(name)
env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_filename, "A"*71))
lambda {
Rack::Multipart.parse_multipart(env)
- }.must_raise Rack::Multipart::Error
+ }.must_raise Rack::Multipart::BoundaryTooLongError
+ end
+
+ it "raises a bad request exception if no body is given but content type indicates a multipart body" do
+ env = Rack::MockRequest.env_for("/", "CONTENT_TYPE" => 'multipart/form-data; boundary=BurgerBurger', :input => nil)
+ lambda {
+ Rack::Multipart.parse_multipart(env)
+ }.must_raise Rack::BadRequest
end
it "parse multipart content when content type present but disposition is not" do
diff --git a/test/spec_request.rb b/test/spec_request.rb
index 9fed5029a..3a2088e16 100644
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -113,7 +113,7 @@ class RackRequestTest < Minitest::Spec
it "wrap the rack variables" do
req = make_request(Rack::MockRequest.env_for("http://example.com:8080/"))
- req.body.must_respond_to :gets
+ req.body.must_be_nil
req.scheme.must_equal "http"
req.request_method.must_equal "GET"
@@ -131,7 +131,7 @@ class RackRequestTest < Minitest::Spec
req.host.must_equal "example.com"
req.port.must_equal 8080
- req.content_length.must_equal "0"
+ req.content_length.must_be_nil
req.content_type.must_be_nil
end
@@ -696,9 +696,9 @@ def initialize(*)
message.must_equal "invalid %-encoding (a%)"
end
- it "raise if rack.input is missing" do
+ it "return empty POST data if rack.input is missing" do
req = make_request({})
- lambda { req.POST }.must_raise RuntimeError
+ req.POST.must_be_empty
end
it "parse POST data when method is POST and no content-type given" do