From d8049153097361df2f25b3787c54eb1528cac5e7 Mon Sep 17 00:00:00 2001 From: Blake Williams Date: Fri, 4 Feb 2022 10:56:05 -0500 Subject: [PATCH 1/2] Add rack.response_finished to Rack::Lint This updates Rack::Lint to validate that `rack.response_finished` is an array of callables when present in the `env`. e.g. procs, lambdas, or objects that respond to `call`. This validates that: * `rack.response_finished` is an array * The contents of the array all respond to `call` --- CHANGELOG.md | 5 ++++- SPEC.rdoc | 4 ++++ lib/rack/constants.rb | 1 + lib/rack/lint.rb | 14 +++++++++++++- test/spec_lint.rb | 21 +++++++++++++++++++++ 5 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbad25452..1126472b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,10 @@ All notable changes to this project will be documented in this file. For info on - Middleware must no longer call `#each` on the body, but they can call `#to_ary` on the body if it responds to `#to_ary`. - `rack.input` is no longer required to be rewindable. - `rack.multithread`/`rack.multiprocess`/`rack.run_once`/`rack.version` are no longer required environment keys. -- `SERVER_PROTOCOL` is now a required key, matching the HTTP protocol used in the request. +- `SERVER_PROTOCOL` is now a required environment key, matching the HTTP protocol used in the request. - `rack.hijack?` (partial hijack) and `rack.hijack` (full hijack) are now independently optional. - `rack.hijack_io` has been removed completely. +- `rack.response_finished` is an optional environment key which contains an array of callable objects that must accept `(env, status, headers, error)` and is invoked after the response is finished (either successfully or unsucessfully). ### Removed @@ -41,6 +42,7 @@ All notable changes to this project will be documented in this file. For info on - Allow response headers to contain array of values. ([#1598](https://github.com/rack/rack/issues/1598), [@ioquatix]) - Support callable body for explicit streaming support and clarify streaming response body behaviour. ([#1745](https://github.com/rack/rack/pull/1745), [@ioquatix], [#1748](https://github.com/rack/rack/pull/1748), [@wjordan]) - Allow `Rack::Builder#run` to take a block instead of an argument. ([#1942](https://github.com/rack/rack/pull/1942), [@ioquatix]) +- Add `rack.response_finished` to `Rack::Lint`. ([#1802](https://github.com/rack/rack/pull/1802), [@BlakeWilliams], [#1952](https://github.com/rack/rack/pull/1952), [@ioquatix]) ### Changed @@ -787,3 +789,4 @@ Items below this line are from the previously maintained HISTORY.md and NEWS.md [@jeremyevans]: https://github.com/jeremyevans "Jeremy Evans" [@amatsuda]: https://github.com/amatsuda "Akira Matsuda" [@wjordan]: https://github.com/wjordan "Will Jordan" +[@BlakeWilliams]: https://github.com/BlakeWilliams "Blake Williams" diff --git a/SPEC.rdoc b/SPEC.rdoc index f97f69d08..41a9bed20 100644 --- a/SPEC.rdoc +++ b/SPEC.rdoc @@ -132,6 +132,10 @@ There are the following restrictions: set. PATH_INFO should be / if SCRIPT_NAME is empty. SCRIPT_NAME never should be /, but instead be empty. +rack.response_finished:: An array of callables run after the HTTP response has been finished, +either normally or with an error (e.g. the client disconnected). +Invoked with (env, status, headers, error) arguments. If there is no error sending the response, +the error argument will be +nil+, otherwise you can expect an exception object, e.g. +IOError+. === The Input Stream diff --git a/lib/rack/constants.rb b/lib/rack/constants.rb index 26fe9d14b..d99b63673 100644 --- a/lib/rack/constants.rb +++ b/lib/rack/constants.rb @@ -51,6 +51,7 @@ module Rack RACK_RECURSIVE_INCLUDE = 'rack.recursive.include' RACK_MULTIPART_BUFFER_SIZE = 'rack.multipart.buffer_size' RACK_MULTIPART_TEMPFILE_FACTORY = 'rack.multipart.tempfile_factory' + RACK_RESPONSE_FINISHED = 'rack.response_finished' RACK_REQUEST_FORM_INPUT = 'rack.request.form_input' RACK_REQUEST_FORM_HASH = 'rack.request.form_hash' RACK_REQUEST_FORM_VARS = 'rack.request.form_vars' diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index 9ac3e2bca..f1f9a1d43 100755 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -363,6 +363,18 @@ def check_environment(env) unless env[SCRIPT_NAME] != "/" raise LintError, "SCRIPT_NAME cannot be '/', make it '' and PATH_INFO '/'" end + + ## rack.response_finished:: An array of callables run after the HTTP response has been finished, + ## either normally or with an error (e.g. the client disconnected). + ## Invoked with (env, status, headers, error) arguments. If there is no error sending the response, + ## the error argument will be +nil+, otherwise you can expect an exception object, e.g. +IOError+. + if callables = env[RACK_RESPONSE_FINISHED] + raise LintError, "rack.response_finished must be an array of callable objects" unless callables.is_a?(Array) + + callables.each do |callable| + raise LintError, "rack.response_finished values must respond to call(env, status, headers, error)" unless callable.respond_to?(:call) + end + end end ## @@ -582,7 +594,7 @@ def check_hijack_response(headers, env) ## ignore the +body+ part of the response tuple when the ## +rack.hijack+ response header is present. Using an empty +Array+ ## instance is recommended. - else + else ## ## The special response header +rack.hijack+ must only be set ## if the request +env+ has a truthy +rack.hijack?+. diff --git a/test/spec_lint.rb b/test/spec_lint.rb index 6ad2ca78b..2af47f786 100755 --- a/test/spec_lint.rb +++ b/test/spec_lint.rb @@ -253,6 +253,16 @@ def obj.fatal(*) end Rack::Lint.new(nil).call(env("SCRIPT_NAME" => "/")) }.must_raise(Rack::Lint::LintError). message.must_match(/cannot be .* make it ''/) + + lambda { + Rack::Lint.new(nil).call(env("rack.response_finished" => "not a callable")) + }.must_raise(Rack::Lint::LintError). + message.must_match(/rack.response_finished must be an array of callable objects/) + + lambda { + Rack::Lint.new(nil).call(env("rack.response_finished" => [-> (env) {}, "not a callable"])) + }.must_raise(Rack::Lint::LintError). + message.must_match(/rack.response_finished values must respond to call/) end it "notice input errors" do @@ -811,4 +821,15 @@ def assert_lint(*args) }.must_raise(Rack::Lint::LintError). message.must_equal 'rack.hijack header must respond to #call' end + + it "pass valid rack.response_finished" do + callable_object = Class.new do + def call(env, status, headers, error) + end + end.new + + Rack::Lint.new(lambda { |env| + [200, {}, ["foo"]] + }).call(env({ "rack.response_finished" => [-> (env) {}, lambda { |env| }, callable_object], "content-length" => "3" })).first.must_equal 200 + end end From 4098c4b8891a4368e2cd9c21e012398d0d68fd7b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sat, 27 Aug 2022 10:15:40 +1200 Subject: [PATCH 2/2] Clarified more behaviour. --- CHANGELOG.md | 2 +- SPEC.rdoc | 10 ++++++---- lib/rack/lint.rb | 10 ++++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1126472b5..198f22f2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ All notable changes to this project will be documented in this file. For info on - `SERVER_PROTOCOL` is now a required environment key, matching the HTTP protocol used in the request. - `rack.hijack?` (partial hijack) and `rack.hijack` (full hijack) are now independently optional. - `rack.hijack_io` has been removed completely. -- `rack.response_finished` is an optional environment key which contains an array of callable objects that must accept `(env, status, headers, error)` and is invoked after the response is finished (either successfully or unsucessfully). +- `rack.response_finished` is an optional environment key which contains an array of callable objects that must accept `#call(env, status, headers, error)` and are invoked after the response is finished (either successfully or unsucessfully). ### Removed diff --git a/SPEC.rdoc b/SPEC.rdoc index 41a9bed20..703ff5cd8 100644 --- a/SPEC.rdoc +++ b/SPEC.rdoc @@ -132,10 +132,12 @@ There are the following restrictions: set. PATH_INFO should be / if SCRIPT_NAME is empty. SCRIPT_NAME never should be /, but instead be empty. -rack.response_finished:: An array of callables run after the HTTP response has been finished, -either normally or with an error (e.g. the client disconnected). -Invoked with (env, status, headers, error) arguments. If there is no error sending the response, -the error argument will be +nil+, otherwise you can expect an exception object, e.g. +IOError+. +rack.response_finished:: An array of callables run by the server after the response has been +processed. This would typically be invoked after sending the response to the client, but it could also be +invoked if an error occurs while generating the response or sending the response; in that case, the error +argument will be a subclass of +Exception+. +The callables are invoked with +env, status, headers, error+ arguments and should not raise any +exceptions. They should be invoked in reverse order of registration. === The Input Stream diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index f1f9a1d43..7baca8671 100755 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -364,10 +364,12 @@ def check_environment(env) raise LintError, "SCRIPT_NAME cannot be '/', make it '' and PATH_INFO '/'" end - ## rack.response_finished:: An array of callables run after the HTTP response has been finished, - ## either normally or with an error (e.g. the client disconnected). - ## Invoked with (env, status, headers, error) arguments. If there is no error sending the response, - ## the error argument will be +nil+, otherwise you can expect an exception object, e.g. +IOError+. + ## rack.response_finished:: An array of callables run by the server after the response has been + ## processed. This would typically be invoked after sending the response to the client, but it could also be + ## invoked if an error occurs while generating the response or sending the response; in that case, the error + ## argument will be a subclass of +Exception+. + ## The callables are invoked with +env, status, headers, error+ arguments and should not raise any + ## exceptions. They should be invoked in reverse order of registration. if callables = env[RACK_RESPONSE_FINISHED] raise LintError, "rack.response_finished must be an array of callable objects" unless callables.is_a?(Array)