diff options
author | Samuel Williams <samuel.williams@oriontransfer.co.nz> | 2023-01-19 16:53:48 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-19 16:53:48 -0800 |
commit | 54368a0981c46c2c62137e47795b38de50dabefb (patch) | |
tree | bac26fec0cb31cf085294b57d64aa2325414bb5a | |
parent | 35a8574710b310966a5f3a7cb7f937b93509aeb3 (diff) | |
download | rack-54368a0981c46c2c62137e47795b38de50dabefb.tar.gz |
Make `env['rack.input']` optional. (#2018)
-rw-r--r-- | CHANGELOG.md | 6 | ||||
-rw-r--r-- | SPEC.rdoc | 2 | ||||
-rw-r--r-- | lib/rack/lint.rb | 26 | ||||
-rw-r--r-- | lib/rack/mock_request.rb | 18 | ||||
-rw-r--r-- | lib/rack/multipart.rb | 7 | ||||
-rw-r--r-- | lib/rack/request.rb | 19 | ||||
-rw-r--r-- | test/spec_lint.rb | 8 | ||||
-rw-r--r-- | test/spec_mock_request.rb | 13 | ||||
-rw-r--r-- | test/spec_mock_response.rb | 5 | ||||
-rw-r--r-- | test/spec_multipart.rb | 10 | ||||
-rw-r--r-- | test/spec_request.rb | 8 |
11 files changed, 79 insertions, 43 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f93d6b34..7fcc45a7 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 @@ -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. diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index ee3ec716..dc78b94d 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -56,9 +56,6 @@ module Rack 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 @@ module Rack ## 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"] @@ -328,10 +323,17 @@ module Rack 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 @@ -384,7 +386,7 @@ module Rack ## ## 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 @@ module Rack ## ## === 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 b6d7ef4f..8e111653 100644 --- a/lib/rack/mock_request.rb +++ b/lib/rack/mock_request.rb @@ -41,11 +41,6 @@ module Rack end end - DEFAULT_ENV = { - RACK_INPUT => StringIO.new, - RACK_ERRORS => StringIO.new, - }.freeze - def initialize(app) @app = app end @@ -104,7 +99,7 @@ module Rack 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 @@ module Rack 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 3347662a..76b25a51 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 40922a21..a3eb9926 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -501,10 +501,20 @@ module Rack 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 @@ module Rack 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 081ff367..f7e5852a 100644 --- a/test/spec_lint.rb +++ b/test/spec_lint.rb @@ -13,8 +13,12 @@ describe Rack::Lint do [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 b2a16fde..b702cea5 100644 --- a/test/spec_mock_request.rb +++ b/test/spec_mock_request.rb @@ -14,7 +14,10 @@ end 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 @@ describe Rack::MockRequest do 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 @@ describe Rack::MockRequest do 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 @@ describe Rack::MockRequest do 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 @@ describe Rack::MockRequest do 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 5cc4fe76..0704ef95 100644 --- a/test/spec_mock_response.rb +++ b/test/spec_mock_response.rb @@ -14,7 +14,10 @@ end 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 43bb90b2..7097528a 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -30,8 +30,7 @@ describe Rack::Multipart do 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 @@ describe Rack::Multipart do }.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 68782625..295abf95 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 @@ class RackRequestTest < Minitest::Spec 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 |