From 636567bc43d279df3db8145c25d9cff3702c071f Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 17 Jan 2023 19:55:00 +1300 Subject: [PATCH] Make `env['rack.input']` optional. --- CHANGELOG.md | 6 ++++++ SPEC.rdoc | 2 +- lib/rack/lint.rb | 26 ++++++++++++++------------ lib/rack/mock_request.rb | 18 +++++++----------- lib/rack/multipart.rb | 7 ++++++- lib/rack/request.rb | 19 +++++++++++++++---- test/spec_lint.rb | 8 ++++++-- test/spec_mock_request.rb | 13 ++++++++----- test/spec_mock_response.rb | 5 ++++- test/spec_multipart.rb | 10 ++++++++-- test/spec_request.rb | 8 ++++---- 11 files changed, 79 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f93d6b340..7fcc45a79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 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/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..76b25a51b 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -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 diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 40922a219..a3eb9926a 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -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 @@ -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 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..7097528ae 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 @@ -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) diff --git a/test/spec_request.rb b/test/spec_request.rb index 687826251..295abf95b 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 @@ -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