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

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 b5232e00..e36cc21f 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
@@ -323,7 +326,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") { value.respond_to? :each }
         value.each { |item|
@@ -343,18 +346,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 != '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 @@ 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 e863559c..01178b7f 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