Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make env['rack.input'] optional. #2018

Merged
merged 1 commit into from Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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