Permalink
Browse files

Updated Rake::Lint to ensure Content-Length header is present for non…

…-chunked responses
  • Loading branch information...
1 parent ebefdb2 commit b18186149bc9e688fbf4d94c2049e3642d037c08 Dan Kubb committed Jul 24, 2008
View
@@ -1,3 +1,5 @@
+require 'time'
+
module Rack
# Rack::Directory serves entries below the +root+ given, according to the
# path info of the Rack request. If a directory is found, the file's contents
@@ -51,7 +53,9 @@ def call(env)
def _call(env)
if env["PATH_INFO"].include? ".."
- return [403, {"Content-Type" => "text/plain"}, ["Forbidden\n"]]
+ body = "Forbidden\n"
+ size = body.respond_to?(:bytesize) ? body.bytesize : body.size
+ return [403, {"Content-Type" => "text/plain","Content-Length" => size.to_s}, [body]]
end
@path = F.join(@root, Utils.unescape(env['PATH_INFO']))
@@ -77,8 +81,9 @@ def _call(env)
end
end
- return [404, {"Content-Type" => "text/plain"},
- ["Entity not found: #{env["PATH_INFO"]}\n"]]
+ body = "Entity not found: #{env["PATH_INFO"]}\n"
+ size = body.respond_to?(:bytesize) ? body.bytesize : body.size
+ return [404, {"Content-Type" => "text/plain", "Content-Length" => size.to_s}, [body]]
end
def each
View
@@ -23,7 +23,9 @@ def call(env)
def _call(env)
if env["PATH_INFO"].include? ".."
- return [403, {"Content-Type" => "text/plain"}, ["Forbidden\n"]]
+ body = "Forbidden\n"
+ size = body.respond_to?(:bytesize) ? body.bytesize : body.size
+ return [403, {"Content-Type" => "text/plain","Content-Length" => size.to_s}, [body]]
end
@path = F.join(@root, Utils.unescape(env["PATH_INFO"]))
@@ -36,8 +38,9 @@ def _call(env)
"Content-Length" => F.size(@path).to_s
}, self]
else
- return [404, {"Content-Type" => "text/plain"},
- ["File not found: #{env["PATH_INFO"]}\n"]]
+ body = "File not found: #{env["PATH_INFO"]}\n"
+ size = body.respond_to?(:bytesize) ? body.bytesize : body.size
+ [404, {"Content-Type" => "text/plain", "Content-Length" => size.to_s}, [body]]
end
end
View
@@ -3,6 +3,8 @@ module Rack
# responses according to the Rack spec.
class Lint
+ STATUS_WITH_NO_ENTITY_BODY = (100..199).to_a << 204 << 304
+
def initialize(app)
@app = app
end
@@ -45,6 +47,7 @@ def call(env=nil)
check_headers headers
## and the *body*.
check_content_type status, headers
+ check_content_length status, headers
[status, headers, self]
end
@@ -57,7 +60,7 @@ def check_env(env)
env.instance_of? Hash
}
- ##
+ ##
## The environment is required to include these variables
## (adopted from PEP333), except when they'd be empty, but see
## below.
@@ -115,7 +118,7 @@ def check_env(env)
## and should be prefixed uniquely. The prefix <tt>rack.</tt>
## is reserved for use with the Rack core distribution and must
## not be used otherwise.
- ##
+ ##
%w[REQUEST_METHOD SERVER_NAME SERVER_PORT
QUERY_STRING
@@ -141,7 +144,7 @@ def check_env(env)
}
}
- ##
+ ##
## There are the following restrictions:
## * <tt>rack.version</tt> must be an array of Integers.
@@ -301,8 +304,8 @@ def close(*args)
## === The Status
def check_status(status)
- ## The status, if parsed as integer (+to_i+), must be bigger than 100.
- assert("Status must be >100 seen as integer") { status.to_i > 100 }
+ ## The status, if parsed as integer (+to_i+), must be greater than or equal to 100.
+ assert("Status must be >=100 seen as integer") { status.to_i >= 100 }
end
## === The Headers
@@ -323,7 +326,7 @@ def check_headers(header)
## but only contain keys that consist of
## letters, digits, <tt>_</tt> or <tt>-</tt> and start with a letter.
assert("invalid header name: #{key}") { key =~ /\A[a-zA-Z][a-zA-Z0-9_-]*\z/ }
- ##
+ ##
## The values of the header must respond to #each.
assert("header values must respond to #each") { value.respond_to? :each }
value.each { |item|
@@ -343,18 +346,69 @@ def check_headers(header)
def check_content_type(status, headers)
headers.each { |key, value|
## There must be a <tt>Content-Type</tt>, except when the
- ## +Status+ is 204 or 304, in which case there must be none
+ ## +Status+ is 1xx, 204 or 304, in which case there must be none
## given.
if key.downcase == "content-type"
- assert("Content-Type header found in #{status} response, not allowed"){
- not [204, 304].include? status.to_i
+ assert("Content-Type header found in #{status} response, not allowed") {
+ not STATUS_WITH_NO_ENTITY_BODY.include? status.to_i
}
return
end
}
assert("No Content-Type header found") {
- [201, 204, 304].include? status.to_i
+ STATUS_WITH_NO_ENTITY_BODY.include? status.to_i
+ }
+ end
+
+ ## === The Content-Length
+ def check_content_length(status, headers)
+ chunked_response = false
+ headers.each { |key, value|
+ if key.downcase == 'transfer-encoding'
+ chunked_response = value != 'identity'
+ end
}
+
+ headers.each { |key, value|
+ if key.downcase == 'content-length'
+ ## There must be a <tt>Content-Length</tt>, except when the
+ ## +Status+ is 1xx, 204 or 304, in which case there must be none
+ ## given.
+ assert("Content-Length header found in #{status} response, not allowed") {
+ not STATUS_WITH_NO_ENTITY_BODY.include? status.to_i
+ }
+
+ assert('Content-Length header should not be used if body is chunked') {
+ not chunked_response
+ }
+
+ bytes = 0
+ string_body = true
+
+ @body.each { |part|
+ unless part.kind_of?(String)
+ string_body = false
+ break
+ end
+
+ bytes += (part.respond_to?(:bytesize) ? part.bytesize : part.size)
+ }
+
+ if string_body
+ assert("Content-Length header was #{value}, but should be #{bytes}") {
+ value == bytes.to_s
+ }
+ end
+
+ return
+ end
+ }
+
+ if [ String, Array ].include?(@body.class) && !chunked_response
+ assert('No Content-Length header found') {
+ STATUS_WITH_NO_ENTITY_BODY.include? status.to_i
+ }
+ end
end
## === The Body
@@ -368,11 +422,11 @@ def each
}
yield part
}
- ##
+ ##
## If the Body responds to #close, it will be called after iteration.
# XXX howto: assert("Body has not been closed") { @closed }
- ##
+ ##
## The Body commonly is an Array of Strings, the application
## instance itself, or a File-like object.
end
View
@@ -25,7 +25,9 @@ def call(env)
req = Rack::Request.new(env)
message = Rack::Utils::HTTP_STATUS_CODES[status.to_i] || status.to_s
detail = env["rack.showstatus.detail"] || message
- [status, headers.merge("Content-Type" => "text/html"), [@template.result(binding)]]
+ body = @template.result(binding)
+ size = body.respond_to?(:bytesize) ? body.bytesize : body.size
+ [status, headers.merge("Content-Type" => "text/html", "Content-Length" => size.to_s), [body]]
else
[status, headers, body]
end
@@ -7,7 +7,7 @@
context "Rack::Directory" do
DOCROOT = File.expand_path(File.dirname(__FILE__))
- FILE_CATCH = proc{|env| [200, {'Content-Type'=>'text/plain'}, 'passed!'] }
+ FILE_CATCH = proc{|env| [200, {'Content-Type'=>'text/plain', "Content-Length" => "7"}, 'passed!'] }
app = Rack::Directory.new DOCROOT, FILE_CATCH
specify "serves directory indices" do
View
@@ -8,11 +8,11 @@
def env(*args)
Rack::MockRequest.env_for("/", *args)
end
-
+
specify "passes valid request" do
lambda {
Rack::Lint.new(lambda { |env|
- [200, {"Content-type" => "test/plain"}, "foo"]
+ [200, {"Content-type" => "test/plain", "Content-length" => "3"}, "foo"]
}).call(env({}))
}.should.not.raise
end
@@ -120,14 +120,14 @@ def env(*args)
["cc", {}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
- message.should.match(/must be >100 seen as integer/)
+ message.should.match(/must be >=100 seen as integer/)
lambda {
Rack::Lint.new(lambda { |env|
[42, {}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
- message.should.match(/must be >100 seen as integer/)
+ message.should.match(/must be >=100 seen as integer/)
end
specify "notices header errors" do
@@ -199,30 +199,57 @@ def env(*args)
specify "notices content-type errors" do
lambda {
Rack::Lint.new(lambda { |env|
- [200, {}, ""]
+ [200, {"Content-length" => "0"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
message.should.match(/No Content-Type/)
+ [100, 101, 204, 304].each do |status|
+ lambda {
+ Rack::Lint.new(lambda { |env|
+ [status, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
+ }).call(env({}))
+ }.should.raise(Rack::Lint::LintError).
+ message.should.match(/Content-Type header found/)
+ end
+ end
+
+ specify "notices content-length errors" do
+ lambda {
+ Rack::Lint.new(lambda { |env|
+ [200, {"Content-type" => "text/plain"}, ""]
+ }).call(env({}))
+ }.should.raise(Rack::Lint::LintError).
+ message.should.match(/No Content-Length/)
+
+ [100, 101, 204, 304].each do |status|
+ lambda {
+ Rack::Lint.new(lambda { |env|
+ [status, {"Content-length" => "0"}, ""]
+ }).call(env({}))
+ }.should.raise(Rack::Lint::LintError).
+ message.should.match(/Content-Length header found/)
+ end
+
lambda {
Rack::Lint.new(lambda { |env|
- [204, {"Content-Type" => "text/plain"}, ""]
+ [200, {"Content-type" => "text/plain", "Content-Length" => "0", "Transfer-Encoding" => "chunked"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
- message.should.match(/Content-Type header found/)
+ message.should.match(/Content-Length header should not be used/)
lambda {
Rack::Lint.new(lambda { |env|
- [204, {"Content-type" => "text/plain"}, ""]
+ [200, {"Content-type" => "text/plain", "Content-Length" => "1"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
- message.should.match(/Content-Type header found/)
+ message.should.match(/Content-Length header was 1, but should be 0/)
end
specify "notices body errors" do
lambda {
status, header, body = Rack::Lint.new(lambda { |env|
- [200, {"Content-type" => "text/plain"}, [1,2,3]]
+ [200, {"Content-type" => "text/plain","Content-length" => "3"}, [1,2,3]]
}).call(env({}))
body.each { |part| }
}.should.raise(Rack::Lint::LintError).
@@ -233,15 +260,15 @@ def env(*args)
lambda {
Rack::Lint.new(lambda { |env|
env["rack.input"].gets("\r\n")
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
message.should.match(/gets called with arguments/)
lambda {
Rack::Lint.new(lambda { |env|
env["rack.input"].read("foo")
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
message.should.match(/read called with non-integer argument/)
@@ -265,23 +292,23 @@ def each
lambda {
Rack::Lint.new(lambda { |env|
env["rack.input"].gets
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env("rack.input" => weirdio))
}.should.raise(Rack::Lint::LintError).
message.should.match(/gets didn't return a String/)
lambda {
Rack::Lint.new(lambda { |env|
env["rack.input"].each { |x| }
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env("rack.input" => weirdio))
}.should.raise(Rack::Lint::LintError).
message.should.match(/each didn't yield a String/)
lambda {
Rack::Lint.new(lambda { |env|
env["rack.input"].read
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env("rack.input" => weirdio))
}.should.raise(Rack::Lint::LintError).
message.should.match(/read didn't return a String/)
@@ -290,7 +317,7 @@ def each
lambda {
Rack::Lint.new(lambda { |env|
env["rack.input"].close
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
message.should.match(/close must not be called/)
@@ -300,15 +327,15 @@ def each
lambda {
Rack::Lint.new(lambda { |env|
env["rack.errors"].write(42)
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
message.should.match(/write not called with a String/)
lambda {
Rack::Lint.new(lambda { |env|
env["rack.errors"].close
- [201, {"Content-type" => "text/plain"}, ""]
+ [201, {"Content-type" => "text/plain", "Content-length" => "0"}, ""]
}).call(env({}))
}.should.raise(Rack::Lint::LintError).
message.should.match(/close must not be called/)
Oops, something went wrong.

0 comments on commit b181861

Please sign in to comment.