Skip to content

Commit

Permalink
Make env['rack.input'] optional.
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed Jan 20, 2023
1 parent 35a8574 commit c993a37
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 43 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,12 @@

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])

## [3.0.3] - 2022-12-07

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion SPEC.rdoc
Expand Up @@ -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:
* <tt>rack.url_scheme</tt> must either be +http+ or +https+.
* There must be a valid input stream in <tt>rack.input</tt>.
* There may be a valid input stream in <tt>rack.input</tt>.
* There must be a valid error stream in <tt>rack.errors</tt>.
* There may be a valid hijack callback in <tt>rack.hijack</tt>
* The <tt>REQUEST_METHOD</tt> must be a valid token.
Expand Down
26 changes: 14 additions & 12 deletions lib/rack/lint.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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 <tt>SERVER_PORT</tt> must be an Integer if set.
server_port = env["SERVER_PORT"]
Expand Down Expand Up @@ -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 <tt>rack.input</tt>.
check_input env[RACK_INPUT]
## * There may be a valid input stream in <tt>rack.input</tt>.
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 <tt>rack.errors</tt>.
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 <tt>rack.hijack</tt>
check_hijack env

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 7 additions & 11 deletions lib/rack/mock_request.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/rack/multipart.rb
Expand Up @@ -13,9 +13,14 @@ module Rack
module Multipart
MULTIPART_BOUNDARY = "AaB03x"

class MissingInputError < StandardError
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
Expand Down
19 changes: 15 additions & 4 deletions lib/rack/request.rb
Expand Up @@ -501,10 +501,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
Expand All @@ -516,6 +526,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
Expand Down
8 changes: 6 additions & 2 deletions test/spec_lint.rb
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions test/spec_mock_request.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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 ''
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion test/spec_mock_response.rb
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions test/spec_multipart.rb
Expand Up @@ -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

Expand All @@ -42,6 +41,13 @@ def multipart_file(name)
}.must_raise Rack::Multipart::Error
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::Multipart::MissingInputError
end

it "parse multipart content when content type present but disposition is not" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_disposition))
params = Rack::Multipart.parse_multipart(env)
Expand Down
8 changes: 4 additions & 4 deletions test/spec_request.rb
Expand Up @@ -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"

Expand All @@ -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

Expand Down Expand Up @@ -712,9 +712,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
Expand Down

0 comments on commit c993a37

Please sign in to comment.