diff --git a/lib/aikido/zen/context/rack_request.rb b/lib/aikido/zen/context/rack_request.rb index 301d12a8..a597231d 100644 --- a/lib/aikido/zen/context/rack_request.rb +++ b/lib/aikido/zen/context/rack_request.rb @@ -20,10 +20,17 @@ module Aikido::Zen {} end + # Parsing the request body reads rack.input and caches the result on env. + # Ensure that rack.input can be rewound, create a copy of env and rewind + # the rack.input stream so that the application can still read the full + # request body and env does not cache the parsed result. body = begin - req.POST + env["rack.input"] = Helpers.rewindable(env["rack.input"]) + Rack::Request.new(env.dup).POST rescue {} + ensure + env["rack.input"]&.rewind end header = begin diff --git a/lib/aikido/zen/context/rails_request.rb b/lib/aikido/zen/context/rails_request.rb index 65197f2c..0da2c6b1 100644 --- a/lib/aikido/zen/context/rails_request.rb +++ b/lib/aikido/zen/context/rails_request.rb @@ -40,10 +40,17 @@ def self.router {} end + # Parsing the request body reads rack.input and caches the result on env. + # Ensure that rack.input can be rewound, create a copy of env and rewind + # the rack.input stream so that the application can still read the full + # request body and env does not cache the parsed result. body = begin - req.request_parameters + env["rack.input"] = Helpers.rewindable(env["rack.input"]) + ActionDispatch::Request.new(env.dup).request_parameters rescue {} + ensure + env["rack.input"]&.rewind end route = begin diff --git a/lib/aikido/zen/helpers.rb b/lib/aikido/zen/helpers.rb index f20606bf..078f5a3a 100644 --- a/lib/aikido/zen/helpers.rb +++ b/lib/aikido/zen/helpers.rb @@ -45,6 +45,17 @@ def self.encode_safely(string) string.encode("UTF-8", invalid: :replace, undef: :replace) end end + + # Rack 3 no longer requires rack.input to respond to #rewind. Returns the + # input if it does, or wrapped in Rack::RewindableInput if not. + # + # @param input [#read, nil] the Rack input stream. + # @return [#rewind, nil] + def self.rewindable(input) + return input if input.nil? || input.respond_to?(:rewind) + + Rack::RewindableInput.new(input) + end end end end diff --git a/test/aikido/zen/context_test.rb b/test/aikido/zen/context_test.rb index d95a857d..68263093 100644 --- a/test/aikido/zen/context_test.rb +++ b/test/aikido/zen/context_test.rb @@ -5,6 +5,33 @@ class Aikido::Zen::ContextTest < ActiveSupport::TestCase DummyRequest = Struct.new(:env) + # Rack 3 does not require rack.input to respond to #rewind. + class NonRewindableInput + def initialize(string) + @io = StringIO.new(string) + end + + def set_encoding(*args) + @io.set_encoding(*args) + end + + def read(*args) + @io.read(*args) + end + + def gets(*args) + @io.gets(*args) + end + + def each(&block) + @io.each(&block) + end + + def close + @io.close + end + end + test "context links to a framework-specific request object" do env = {} @@ -68,6 +95,56 @@ module GenericTests assert_includes context.payloads, stub_payload(:body, "3", "c") end + test "form bodies are extracted without consuming the request body" do + input = StringIO.new("username=mallory&role=admin") + context = build_context_for("/example/path", { + :method => "POST", + :input => input, + "CONTENT_TYPE" => "application/x-www-form-urlencoded" + }) + + assert_includes context.payloads, stub_payload(:body, "admin", "role") + assert_equal "username=mallory&role=admin", input.read + end + + test "form bodies are extracted without consuming a request body that cannot be rewound" do + env = env_for("/example/path", { + :method => "POST", + :input => NonRewindableInput.new("username=mallory&role=admin"), + "CONTENT_TYPE" => "application/x-www-form-urlencoded" + }) + context = Aikido::Zen::Context.from_rack_env(env) + + assert_includes context.payloads, stub_payload(:body, "admin", "role") + assert_equal "username=mallory&role=admin", env["rack.input"].read + end + + test "form bodies are extracted without consuming a replacement request body that cannot be rewound" do + env = env_for("/example/path", { + :method => "POST", + :input => "username=mallory", + "CONTENT_TYPE" => "application/x-www-form-urlencoded" + }) + context = Aikido::Zen::Context.from_rack_env(env) + env["rack.input"] = NonRewindableInput.new("username=mallory&role=admin") + + assert_includes context.payloads, stub_payload(:body, "admin", "role") + assert_equal "username=mallory&role=admin", env["rack.input"].read + end + + test "form bodies are extracted without caching the parsed form on the request env" do + env = env_for("/example/path", { + :method => "POST", + :input => "username=mallory&role=admin", + "CONTENT_TYPE" => "application/x-www-form-urlencoded" + }) + context = Aikido::Zen::Context.from_rack_env(env) + context.payloads + + assert_nil context.request.env["rack.request.form_hash"] + assert_nil context.request.env["rack.request.form_pairs"] + end + test "header params are sourced from the normalized headers" do context = build_context_for("/path", { "HTTP_ACCEPT" => "application/json", @@ -172,8 +249,12 @@ class RackRequestTest < ActiveSupport::TestCase Aikido::Zen.config.harden = false end + def env_for(path, env = {}) + Rack::MockRequest.env_for(path, env) + end + def build_context_for(path, env = {}) - env = Rack::MockRequest.env_for(path, env) + env = env_for(path, env) Aikido::Zen::Context.from_rack_env(env) end @@ -197,6 +278,19 @@ def stub_payload(source, value, path) assert_empty context.payloads.select { |payload| payload.source == :body } end + test "form bodies are extracted without overriding a later replacement of the body by middleware" do + env = Rack::MockRequest.env_for("/path", { + :method => "POST", + :input => "username=mallory&role=admin", + "CONTENT_TYPE" => "application/x-www-form-urlencoded" + }) + Aikido::Zen::Context.from_rack_env(env).payloads + + env["rack.input"] = StringIO.new("username=mallory") + + assert_equal({"username" => "mallory"}, Rack::Request.new(env).POST) + end + test "route params are empty on the base case as they are framework dependent" do context = build_context_for("/some/path")