Main sec

lib/rack/common_logger.rb

@@ -65,6 +65,8 @@ def log(env, status, response_headers, began_at)
         length,
         Utils.clock_time - began_at)
 
+      msg.gsub!(/[^[:print:]\n]/) { |c| "\\x#{c.ord}" }
+
       logger = @logger || request.get_header(RACK_ERRORS)
       # Standard library logger doesn't support write but it supports << which actually
       # calls to write on the log device without formatting

lib/rack/lint.rb

@@ -347,7 +347,7 @@ def check_environment(env)
 
         ## * The <tt>REQUEST_METHOD</tt> must be a valid token.
         unless env[REQUEST_METHOD] =~ /\A[0-9A-Za-z!\#$%&'*+.^_`|~-]+\z/
-          raise LintError, "REQUEST_METHOD unknown: #{env[REQUEST_METHOD]}"
+          raise LintError, "REQUEST_METHOD unknown: #{env[REQUEST_METHOD].dump}"
         end
 
         ## * The <tt>SCRIPT_NAME</tt>, if non-empty, must start with <tt>/</tt>

lib/rack/multipart.rb

@@ -16,19 +16,19 @@ module Multipart
     class << self
       def parse_multipart(env, params = Rack::Utils.default_query_parser)
         io = env[RACK_INPUT]
-        
+
         if content_length = env['CONTENT_LENGTH']
           content_length = content_length.to_i
         end
-        
+
         content_type = env['CONTENT_TYPE']
-        
+
         tempfile = env[RACK_MULTIPART_TEMPFILE_FACTORY] || Parser::TEMPFILE_FACTORY
         bufsize = env[RACK_MULTIPART_BUFFER_SIZE] || Parser::BUFSIZE
-        
+
         info = Parser.parse(io, content_length, content_type, tempfile, bufsize, params)
         env[RACK_TEMPFILES] = info.tmp_files
-        
+
         return info.params
       end
 

lib/rack/multipart/parser.rb

@@ -17,8 +17,7 @@ class EmptyContentError < ::EOFError; end
     TOKEN = /[^\s()<>,;:\\"\/\[\]?=]+/
     CONDISP = /Content-Disposition:\s*#{TOKEN}\s*/i
     VALUE = /"(?:\\"|[^"])*"|#{TOKEN}/
-    BROKEN_QUOTED = /^#{CONDISP}.*;\s*filename="(.*?)"(?:\s*$|\s*;\s*#{TOKEN}=)/i
-    BROKEN_UNQUOTED = /^#{CONDISP}.*;\s*filename=(#{TOKEN})/i
+    BROKEN = /^#{CONDISP}.*;\s*filename=(#{VALUE})/i
     MULTIPART_CONTENT_TYPE = /Content-Type: (.*)#{EOL}/ni
     MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*;\s*name=(#{VALUE})/ni
     MULTIPART_CONTENT_ID = /Content-ID:\s*([^#{EOL}]*)/ni
@@ -331,8 +330,9 @@ def get_filename(head)
           elsif filename = params['filename']
             filename = $1 if filename =~ /^"(.*)"$/
           end
-        when BROKEN_QUOTED, BROKEN_UNQUOTED
+        when BROKEN
           filename = $1
+          filename = $1 if filename =~ /^"(.*)"$/
         end
 
         return unless filename

test/multipart/filename_with_escaped_quotes_and_modification_param

@@ -1,6 +1,6 @@
 --AaB03x
 content-type: image/jpeg
-content-disposition: attachment; name="files"; filename=""human" genome.jpeg"; modification-date="Wed, 12 Feb 1997 16:29:51 -0500";
+content-disposition: attachment; name="files"; filename="\"human\" genome.jpeg"; modification-date="Wed, 12 Feb 1997 16:29:51 -0500";
 Content-Description: a complete map of the human genome
 
 contents

test/spec_common_logger.rb

@@ -25,6 +25,10 @@
     [200,
      { "content-type" => "text/html", "content-length" => "0" },
      []]}
+  app_without_lint = lambda { |env|
+    [200,
+     { "content-type" => "text/html", "content-length" => length.to_s },
+     [obj]]}
 
   it "log to rack.errors by default" do
     res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/")
@@ -103,6 +107,14 @@ def with_mock_time(t = 0)
     (0..1).must_include duration.to_f
   end
 
+  it "escapes non printable characters except newline" do
+    logdev = StringIO.new
+    log = Logger.new(logdev)
+    Rack::MockRequest.new(Rack::CommonLogger.new(app_without_lint, log)).request("GET\b", "/hello")
+
+    logdev.string.must_match(/GET\\x8 \/hello HTTP\/1\.1/)
+  end
+
   it "log path with PATH_INFO" do
     logdev = StringIO.new
     log = Logger.new(logdev)

test/spec_lint.rb

@@ -212,6 +212,11 @@ def obj.fatal(*) end
     }.must_raise(Rack::Lint::LintError).
       message.must_match(/REQUEST_METHOD/)
 
+    lambda {
+      Rack::Lint.new(nil).call(env("REQUEST_METHOD" => "OOPS?\b!"))
+    }.must_raise(Rack::Lint::LintError).
+      message.must_match(/OOPS\?\\/)
+
     lambda {
       Rack::Lint.new(nil).call(env("SCRIPT_NAME" => "howdy"))
     }.must_raise(Rack::Lint::LintError).

test/spec_multipart.rb

@@ -454,19 +454,6 @@ def initialize(*)
     params["files"][:tempfile].read.must_equal "contents"
   end
 
-  it "parse filename with unescaped quotes" do
-    env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_with_unescaped_quotes))
-    params = Rack::Multipart.parse_multipart(env)
-    params["files"][:type].must_equal "application/octet-stream"
-    params["files"][:filename].must_equal "escape \"quotes"
-    params["files"][:head].must_equal "content-disposition: form-data; " +
-      "name=\"files\"; " +
-      "filename=\"escape \"quotes\"\r\n" +
-      "content-type: application/octet-stream\r\n"
-    params["files"][:name].must_equal "files"
-    params["files"][:tempfile].read.must_equal "contents"
-  end
-
   it "parse filename with escaped quotes and modification param" do
     env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_with_escaped_quotes_and_modification_param))
     params = Rack::Multipart.parse_multipart(env)
@@ -475,7 +462,7 @@ def initialize(*)
     params["files"][:head].must_equal "content-type: image/jpeg\r\n" +
       "content-disposition: attachment; " +
       "name=\"files\"; " +
-      "filename=\"\"human\" genome.jpeg\"; " +
+      "filename=\"\\\"human\\\" genome.jpeg\"; " +
       "modification-date=\"Wed, 12 Feb 1997 16:29:51 -0500\";\r\n" +
       "Content-Description: a complete map of the human genome\r\n"
     params["files"][:name].must_equal "files"