Skip to content

Commit

Permalink
Add spec changes to Rack::Lint
Browse files Browse the repository at this point in the history
Added the following Lint errors:
- "Middleware must not call #each directly"
- "New body must yield at least once per iteration of old body"
- "Body has not been closed"
- "#to_ary not identical to contents produced by calling #each"
  • Loading branch information
wjordan committed Apr 27, 2021
1 parent 90be4d9 commit 3e74ada
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 23 deletions.
19 changes: 13 additions & 6 deletions SPEC.rdoc
Original file line number Diff line number Diff line change
Expand Up @@ -273,20 +273,27 @@ and must only yield String values.
The Body itself should not be an instance of String, as this will
break in Ruby 1.9.

Middleware must not call +each+ directly on the Body.
Instead, middleware can return a new Body that calls +each+ on the
original Body, yielding at least once per iteration.

If the Body responds to +to_ary+, it must return an Array whose
contents are identical to that produced by calling +each+.
Middleware may call +to_ary+ directly on the Body and return a new Body in its place.
In other words, middleware can only process the Body directly if it responds to +to_ary+.

If the Body responds to +close+, it will be called after iteration. If
the body is replaced by a middleware after action, the original body
must be closed first, if it responds to close.
the original Body is replaced by a new Body, the new Body
must close the original Body after iteration, if it responds to +close+.
If the Body responds to both +to_ary+ and +close+, its
implementation of +to_ary+ must call +close+ after iteration.

If the Body responds to +to_path+, it must return a String
identifying the location of a file whose contents are identical
to that produced by calling +each+; this may be used by the
server as an alternative, possibly more efficient way to
transport the response.

Strings must be processed individually as they are yielded by +each+.
However, if the Body responds to +to_ary+ it can be implicitly coerced to
an Array, which may then be processed all at once.

The Body commonly is an Array of Strings, the application
instance itself, or a File-like object.
== Thanks
Expand Down
83 changes: 66 additions & 17 deletions lib/rack/lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def call(env = nil)
end

def _call(env)
@env = env
## It takes exactly one argument, the *environment*
raise LintError, "No env given" unless env
check_env env
Expand Down Expand Up @@ -67,6 +68,13 @@ def _call(env)
check_content_type status, headers
check_content_length status, headers
@head_request = env[REQUEST_METHOD] == HEAD

@lint = (env['rack.lint'] ||= []) << self

if (env['rack.lint.body_iteration'] ||= 0) > 0
raise LintError, "Middleware must not call #each directly"
end

[status, headers, self]
end

Expand Down Expand Up @@ -741,43 +749,84 @@ def each
unless part.kind_of? String
raise LintError, "Body yielded non-string value #{part.inspect}"
end
##
## The Body itself should not be an instance of String, as this will
## break in Ruby 1.9.
##
## Middleware must not call +each+ directly on the Body.
## Instead, middleware can return a new Body that calls +each+ on the
## original Body, yielding at least once per iteration.
if @lint[0] == self
@env['rack.lint.body_iteration'] += 1
else
if (@env['rack.lint.body_iteration'] -= 1) > 0
raise LintError, "New body must yield at least once per iteration of old body"
end
end

bytes += part.bytesize
yield part
}
verify_content_length(bytes)

##
## The Body itself should not be an instance of String, as this will
## break in Ruby 1.9.
##
## If the Body responds to +close+, it will be called after iteration. If
## the body is replaced by a middleware after action, the original body
## must be closed first, if it responds to close.
# XXX howto: raise LintError, "Body has not been closed" unless @closed
verify_to_path
end

def respond_to?(sym, *)
if sym.to_s == :to_ary
@body.respond_to? sym
else
super
end
end

##
## If the Body responds to +to_ary+, it must return an Array whose
## contents are identical to that produced by calling +each+.
## Middleware may call +to_ary+ directly on the Body and return a new Body in its place.
## In other words, middleware can only process the Body directly if it responds to +to_ary+.
def to_ary
@body.to_ary.tap do |content|
unless content == @body.enum_for.to_a
raise LintError, "#to_ary not identical to contents produced by calling #each"
end
end
ensure
close
end

##
## If the Body responds to +close+, it will be called after iteration. If
## the original Body is replaced by a new Body, the new Body
## must close the original Body after iteration, if it responds to +close+.
## If the Body responds to both +to_ary+ and +close+, its
## implementation of +to_ary+ must call +close+ after iteration.
def close
@closed = true
@body.close if @body.respond_to?(:close)
index = @lint.index(self)
unless @env['rack.lint'][0..index].all? {|lint| lint.instance_variable_get(:@closed)}
raise LintError, "Body has not been closed"
end
end

def verify_to_path
##
## If the Body responds to +to_path+, it must return a String
## identifying the location of a file whose contents are identical
## to that produced by calling +each+; this may be used by the
## server as an alternative, possibly more efficient way to
## transport the response.

if @body.respond_to?(:to_path)
unless ::File.exist? @body.to_path
raise LintError, "The file identified by body.to_path does not exist"
end
end

##
## The Body commonly is an Array of Strings, the application
## instance itself, or a File-like object.
end

def close
@closed = true
@body.close if @body.respond_to?(:close)
end
##
## The Body commonly is an Array of Strings, the application
## instance itself, or a File-like object.

# :startdoc:

Expand Down
86 changes: 86 additions & 0 deletions test/spec_lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,92 @@ def result.name
body.each { |part| }
}.must_raise(Rack::Lint::LintError).
message.must_match(/yielded non-string/)

# Lint before and after the Rack middleware being tested.
def stacked_lint(app)
Rack::Lint.new(lambda do |env|
Rack::Lint.new(app).call(env).tap {|response| response[2] = yield response[2]}
end)
end

yielder_app = lambda do |_|
input = Object.new
def input.each; 10.times {yield 'foo'}; end
[200, {"Content-type" => "text/plain", "Content-length" => "30"}, input]
end

lambda {
body = stacked_lint(yielder_app) {|body|
new_body = Struct.new(:body) do
def each(&block)
body.each { |part| yield part.upcase }
body.close
end
end
new_body.new(body)
}.call(env({}))[2]
body.each {|part| part.must_equal 'FOO'}
body.close
}.call

lambda {
body = stacked_lint(yielder_app) { |body|
body.enum_for.to_a
}.call(env({}))[2]
body.each {}
body.close
}.must_raise(Rack::Lint::LintError).
message.must_match(/Middleware must not call #each directly/)

lambda {
body = stacked_lint(yielder_app) { |body|
new_body = Struct.new(:body) do
def each(&block)
body.enum_for.each_slice(2) { |parts| yield parts.join }
end
end
new_body.new(body)
}.call(env({}))[2]
body.each {}
body.close
}.must_raise(Rack::Lint::LintError).
message.must_match(/New body must yield at least once per iteration of old body/)

lambda {
body = stacked_lint(yielder_app) { |body|
Struct.new(:body) do
def each; body.each {|part| yield part} end
end.new(body)
}.call(env({}))[2]
body.each {}
body.close
}.must_raise(Rack::Lint::LintError).
message.must_match(/Body has not been closed/)

static_app = lambda do |_|
input = ['foo'] * 10
[200, {"Content-type" => "text/plain", "Content-length" => "30"}, input]
end

lambda {
body = stacked_lint(static_app) { |body| body.to_ary}.call(env({}))[2]
body.each {}
body.close
}.call

array_mismatch = lambda do |_|
input = Object.new
def input.to_ary; ['bar'] * 10; end
def input.each; 10.times {yield 'foo'}; end
[200, {"Content-type" => "text/plain", "Content-length" => "30"}, input]
end

lambda {
body = stacked_lint(array_mismatch) { |body| body.to_ary}.call(env({}))[2]
body.each {}
body.close
}.must_raise(Rack::Lint::LintError).
message.must_match(/#to_ary not identical to contents produced by calling #each/)
end

it "notice input handling errors" do
Expand Down

0 comments on commit 3e74ada

Please sign in to comment.