diff options
author | Christian Neukirchen <chneukirchen@gmail.com> | 2008-07-24 11:26:17 +0200 |
---|---|---|
committer | Christian Neukirchen <chneukirchen@gmail.com> | 2008-07-24 11:26:17 +0200 |
commit | b780e9f73ddff3bc8549654d722832ab873267b1 (patch) | |
tree | 7f2a10a058a3d9d23a4aa9133e3e128c16c18580 | |
parent | 6f500ddd3bcbd46218a01c408572ad168fd2902a (diff) | |
parent | 86403ff46e0dd375240c3091d50144a2b9086928 (diff) | |
download | rack-b780e9f73ddff3bc8549654d722832ab873267b1.tar.gz |
Merge git://github.com/dkubb/rack
-rw-r--r-- | lib/rack/directory.rb | 11 | ||||
-rw-r--r-- | lib/rack/file.rb | 9 | ||||
-rw-r--r-- | lib/rack/lint.rb | 78 | ||||
-rw-r--r-- | lib/rack/showstatus.rb | 4 | ||||
-rw-r--r-- | test/spec_rack_directory.rb | 2 | ||||
-rw-r--r-- | test/spec_rack_lint.rb | 63 | ||||
-rw-r--r-- | test/spec_rack_request.rb | 3 | ||||
-rw-r--r-- | test/spec_rack_showstatus.rb | 9 | ||||
-rw-r--r-- | test/spec_rack_utils.rb | 2 | ||||
-rw-r--r-- | test/testrequest.rb | 4 |
10 files changed, 140 insertions, 45 deletions
diff --git a/lib/rack/directory.rb b/lib/rack/directory.rb index 972b4bc6..31e0db84 100644 --- a/lib/rack/directory.rb +++ b/lib/rack/directory.rb @@ -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 @@ table { width:100%%; } 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 @@ table { width:100%%; } 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 diff --git a/lib/rack/file.rb b/lib/rack/file.rb index 7538fd69..afb21383 100644 --- a/lib/rack/file.rb +++ b/lib/rack/file.rb @@ -23,7 +23,9 @@ module Rack 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 @@ module Rack "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 diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index d55e4b4a..2dec03f6 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -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 @@ module Rack check_headers headers ## and the *body*. check_content_type status, headers + check_content_length status, headers [status, headers, self] end @@ -57,7 +60,7 @@ module Rack 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 @@ module Rack ## 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 @@ module Rack } } - ## + ## ## There are the following restrictions: ## * <tt>rack.version</tt> must be an array of Integers. @@ -301,8 +304,8 @@ module Rack ## === 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 @@ -325,7 +328,7 @@ module Rack ## 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, but the value of " + "'#{key}' doesn't (is #{value.class})") { value.respond_to? :each } @@ -346,18 +349,69 @@ module Rack 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.downcase != '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 @@ -371,11 +425,11 @@ module Rack } 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 diff --git a/lib/rack/showstatus.rb b/lib/rack/showstatus.rb index 8aa1d441..ca81f7d8 100644 --- a/lib/rack/showstatus.rb +++ b/lib/rack/showstatus.rb @@ -25,7 +25,9 @@ module Rack 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 diff --git a/test/spec_rack_directory.rb b/test/spec_rack_directory.rb index c791b197..bf0b8794 100644 --- a/test/spec_rack_directory.rb +++ b/test/spec_rack_directory.rb @@ -7,7 +7,7 @@ require 'rack/mock' 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 diff --git a/test/spec_rack_lint.rb b/test/spec_rack_lint.rb index f3fbd3b2..a8112b85 100644 --- a/test/spec_rack_lint.rb +++ b/test/spec_rack_lint.rb @@ -8,11 +8,11 @@ context "Rack::Lint" do 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 @@ context "Rack::Lint" do ["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 @@ context "Rack::Lint" do 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,7 +260,7 @@ context "Rack::Lint" do 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/) @@ -241,7 +268,7 @@ context "Rack::Lint" do 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,7 +292,7 @@ context "Rack::Lint" do 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/) @@ -273,7 +300,7 @@ context "Rack::Lint" do 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/) @@ -281,7 +308,7 @@ context "Rack::Lint" do 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 @@ context "Rack::Lint" do 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,7 +327,7 @@ context "Rack::Lint" do 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/) @@ -308,7 +335,7 @@ context "Rack::Lint" do 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/) diff --git a/test/spec_rack_request.rb b/test/spec_rack_request.rb index 2b30ff3f..0b2c9fc6 100644 --- a/test/spec_rack_request.rb +++ b/test/spec_rack_request.rb @@ -359,7 +359,8 @@ EOF specify "does conform to the Rack spec" do app = lambda { |env| content = Rack::Request.new(env).POST["file"].inspect - [200, {"Content-Type" => "text/html"}, content] + size = content.respond_to?(:bytesize) ? content.bytesize : content.size + [200, {"Content-Type" => "text/html", "Content-Length" => size.to_s}, content] } input = <<EOF diff --git a/test/spec_rack_showstatus.rb b/test/spec_rack_showstatus.rb index 03e57032..78700134 100644 --- a/test/spec_rack_showstatus.rb +++ b/test/spec_rack_showstatus.rb @@ -6,7 +6,7 @@ require 'rack/mock' context "Rack::ShowStatus" do specify "should provide a default status message" do req = Rack::MockRequest.new(Rack::ShowStatus.new(lambda { |env| - [404, {"Content-Type" => "text/plain"}, []] + [404, {"Content-Type" => "text/plain", "Content-Length" => "0"}, []] })) res = req.get("/", :lint => true) @@ -21,7 +21,7 @@ context "Rack::ShowStatus" do specify "should let the app provide additional information" do req = Rack::MockRequest.new(Rack::ShowStatus.new(lambda { |env| env["rack.showstatus.detail"] = "gone too meta." - [404, {"Content-Type" => "text/plain"}, []] + [404, {"Content-Type" => "text/plain", "Content-Length" => "0"}, []] })) res = req.get("/", :lint => true) @@ -36,7 +36,7 @@ context "Rack::ShowStatus" do specify "should not replace existing messages" do req = Rack::MockRequest.new(Rack::ShowStatus.new(lambda { |env| - [404, {"Content-Type" => "text/plain"}, ["foo!"]] + [404, {"Content-Type" => "text/plain", "Content-Length" => "4"}, ["foo!"]] })) res = req.get("/", :lint => true) res.should.be.not_found @@ -56,7 +56,7 @@ context "Rack::ShowStatus" do specify "should replace existing messages if there is detail" do req = Rack::MockRequest.new(Rack::ShowStatus.new(lambda { |env| env["rack.showstatus.detail"] = "gone too meta." - [404, {"Content-Type" => "text/plain"}, ["foo!"]] + [404, {"Content-Type" => "text/plain", "Content-Length" => "4"}, ["foo!"]] })) res = req.get("/", :lint => true) @@ -64,6 +64,7 @@ context "Rack::ShowStatus" do res.should.be.not.empty res["Content-Type"].should.equal("text/html") + res["Content-Length"].should.not.equal("4") res.should =~ /404/ res.should =~ /too meta/ res.body.should.not =~ /foo/ diff --git a/test/spec_rack_utils.rb b/test/spec_rack_utils.rb index 072d0cf2..0ec0a39f 100644 --- a/test/spec_rack_utils.rb +++ b/test/spec_rack_utils.rb @@ -108,7 +108,7 @@ context "Rack::Utils::Context" do test_target1 = proc{|e| e.to_s+' world' } test_target2 = proc{|e| e.to_i+2 } test_target3 = proc{|e| nil } - test_target4 = proc{|e| [200,{'Content-Type'=>'text/plain'},['']] } + test_target4 = proc{|e| [200,{'Content-Type'=>'text/plain', 'Content-Length'=>'0'},['']] } test_target5 = Object.new specify "should perform checks on both arguments" do diff --git a/test/testrequest.rb b/test/testrequest.rb index 1b045ea7..348cd495 100644 --- a/test/testrequest.rb +++ b/test/testrequest.rb @@ -5,7 +5,9 @@ class TestRequest def call(env) status = env["QUERY_STRING"] =~ /secret/ ? 403 : 200 env["test.postdata"] = env["rack.input"].read - [status, {"Content-Type" => "text/yaml"}, [env.to_yaml]] + body = env.to_yaml + size = body.respond_to?(:bytesize) ? body.bytesize : body.size + [status, {"Content-Type" => "text/yaml", "Content-Length" => size.to_s}, [body]] end module Helpers |