[前][次][番号順一覧][スレッド一覧]

ruby-changes:14437

From: shyouhei <ko1@a...>
Date: Sun, 10 Jan 2010 18:34:08 +0900 (JST)
Subject: [ruby-changes:14437] Ruby:r26267 (trunk, ruby_1_8): * lib/webrick/accesslog.rb : Escape needed.

shyouhei	2010-01-10 18:33:47 +0900 (Sun, 10 Jan 2010)

  New Revision: 26267

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=26267

  Log:
    * lib/webrick/accesslog.rb : Escape needed.
    
    * lib/webrick/httpstatus.rb : ditto.
    
    * lib/webrick/httprequest.rb : ditto.
    
    * lib/webrick/httputils.rb : ditto.
    
    * test/webrick/test_cgi.rb (TestWEBrickCGI::test_bad_): Test for it.

  Modified files:
    branches/ruby_1_8/ChangeLog
    branches/ruby_1_8/lib/webrick/accesslog.rb
    branches/ruby_1_8/lib/webrick/httprequest.rb
    branches/ruby_1_8/lib/webrick/httpstatus.rb
    branches/ruby_1_8/lib/webrick/httputils.rb
    trunk/ChangeLog
    trunk/lib/webrick/accesslog.rb
    trunk/lib/webrick/httprequest.rb
    trunk/lib/webrick/httpstatus.rb
    trunk/lib/webrick/httputils.rb
    trunk/test/webrick/test_cgi.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 26266)
+++ ChangeLog	(revision 26267)
@@ -1,3 +1,15 @@
+Sun Jan 10 17:25:24 2010  Nobuyoshi Nakada  <nobu@r...>
+
+	* lib/webrick/accesslog.rb : Escape needed.
+
+	* lib/webrick/httpstatus.rb : ditto.
+
+	* lib/webrick/httprequest.rb : ditto.
+
+	* lib/webrick/httputils.rb : ditto.
+
+	* test/webrick/test_cgi.rb (TestWEBrickCGI::test_bad_): Test for it.
+
 Sun Jan 10 04:54:36 2010  Nobuyoshi Nakada  <nobu@r...>
 
 	* class.c (rb_define_class): raise TypeError same as class
Index: lib/webrick/httpstatus.rb
===================================================================
--- lib/webrick/httpstatus.rb	(revision 26266)
+++ lib/webrick/httpstatus.rb	(revision 26267)
@@ -12,7 +12,17 @@
 
   module HTTPStatus
 
-    class Status      < StandardError; end
+    class Status      < StandardError
+      def initialize(message, *rest)
+        super(AccessLog.escape(message), *rest)
+      end
+      class << self
+        attr_reader :code, :reason_phrase
+      end
+      def code() self::class::code end
+      def reason_phrase() self::class::reason_phrase end
+      alias to_i code
+    end
     class Info        < Status; end
     class Success     < Status; end
     class Redirect    < Status; end
@@ -68,6 +78,7 @@
     CodeToError = {}
 
     StatusMessage.each{|code, message|
+      message.freeze
       var_name = message.gsub(/[ \-]/,'_').upcase
       err_name = message.gsub(/[ \-]/,'')
 
@@ -79,18 +90,12 @@
       when 500...600; parent = ServerError
       end
 
-      eval %-
-        RC_#{var_name} = #{code}
-        class #{err_name} < #{parent}
-          def self.code() RC_#{var_name} end
-          def self.reason_phrase() StatusMessage[code] end
-          def code() self::class::code end
-          def reason_phrase() self::class::reason_phrase end
-          alias to_i code
-        end
-      -
-
-      CodeToError[code] = const_get(err_name)
+      const_set("RC_#{var_name}", code)
+      err_class = Class.new(parent)
+      err_class.instance_variable_set(:@code, code)
+      err_class.instance_variable_set(:@reason_phrase, message)
+      const_set(err_name, err_class)
+      CodeToError[code] = err_class
     }
 
     def reason_phrase(code)
Index: lib/webrick/httprequest.rb
===================================================================
--- lib/webrick/httprequest.rb	(revision 26266)
+++ lib/webrick/httprequest.rb	(revision 26267)
@@ -266,11 +266,7 @@
           @raw_header << line
         end
       end
-      begin
-        @header = HTTPUtils::parse_header(@raw_header.join)
-      rescue => ex
-        raise  HTTPStatus::BadRequest, ex.message
-      end
+      @header = HTTPUtils::parse_header(@raw_header.join)
     end
 
     def parse_uri(str, scheme="http")
Index: lib/webrick/httputils.rb
===================================================================
--- lib/webrick/httputils.rb	(revision 26266)
+++ lib/webrick/httputils.rb	(revision 26267)
@@ -129,11 +129,11 @@
         when /^\s+(.*?)\s*\z/om
           value = $1
           unless field
-            raise "bad header '#{line.inspect}'."
+            raise HTTPStatus::BadRequest, "bad header '#{line}'."
           end
           header[field][-1] << " " << value
         else
-          raise "bad header '#{line.inspect}'."
+          raise HTTPStatus::BadRequest, "bad header '#{line}'."
         end
       }
       header.each{|key, values|
Index: lib/webrick/accesslog.rb
===================================================================
--- lib/webrick/accesslog.rb	(revision 26266)
+++ lib/webrick/accesslog.rb	(revision 26267)
@@ -53,15 +53,23 @@
          when ?e, ?i, ?n, ?o
            raise AccessLogError,
              "parameter is required for \"#{spec}\"" unless param
-           params[spec][param] || "-"
+           param = params[spec][param] ? escape(param) : "-"
          when ?t
            params[spec].strftime(param || CLF_TIME_FORMAT)
          when ?%
            "%"
          else
-           params[spec]
+           escape(params[spec].to_s)
          end
       }
     end
+
+    def escape(data)
+      if data.tainted?
+        data.gsub(/[[:cntrl:]\\]+/) {$&.dump[1...-1]}.untaint
+      else
+        data
+      end
+    end
   end
 end
Index: test/webrick/test_cgi.rb
===================================================================
--- test/webrick/test_cgi.rb	(revision 26266)
+++ test/webrick/test_cgi.rb	(revision 26267)
@@ -98,4 +98,36 @@
       end
     }
   end
+
+  CtrlSeq = [0x7f, *(1..31)].pack("C*").gsub(/\s+/, '')
+  CtrlPat = /#{Regexp.quote(CtrlSeq)}/o
+  DumpPat = /#{Regexp.quote(CtrlSeq.dump[1...-1])}/o
+
+  def test_bad_uri
+    start_cgi_server{|server, addr, port, log|
+      res = TCPSocket.open(addr, port) {|sock|
+        sock << "GET /#{CtrlSeq}#{CRLF}#{CRLF}"
+        sock.close_write
+        sock.read
+      }
+      assert_match(%r{\AHTTP/\d.\d 400 Bad Request}, res)
+      s = log.call.each_line.grep(/ERROR bad URI/)[0]
+      assert_match(DumpPat, s)
+      assert_not_match(CtrlPat, s)
+    }
+  end
+
+  def test_bad_header
+    start_cgi_server{|server, addr, port, log|
+      res = TCPSocket.open(addr, port) {|sock|
+        sock << "GET / HTTP/1.0#{CRLF}#{CtrlSeq}#{CRLF}#{CRLF}"
+        sock.close_write
+        sock.read
+      }
+      assert_match(%r{\AHTTP/\d.\d 400 Bad Request}, res)
+      s = log.call.each_line.grep(/ERROR bad header/)[0]
+      assert_match(DumpPat, s)
+      assert_not_match(CtrlPat, s)
+    }
+  end
 end
Index: ruby_1_8/ChangeLog
===================================================================
--- ruby_1_8/ChangeLog	(revision 26266)
+++ ruby_1_8/ChangeLog	(revision 26267)
@@ -1,3 +1,13 @@
+Sun Jan 10 17:25:24 2010  Nobuyoshi Nakada  <nobu@r...>
+
+	* lib/webrick/accesslog.rb : Escape needed.
+
+	* lib/webrick/httpstatus.rb : ditto.
+
+	* lib/webrick/httprequest.rb : ditto.
+
+	* lib/webrick/httputils.rb : ditto.
+
 Sun Jan 10 04:54:36 2010  Nobuyoshi Nakada  <nobu@r...>
 
 	* class.c (rb_define_class): raise TypeError same as class
Index: ruby_1_8/lib/webrick/httpstatus.rb
===================================================================
--- ruby_1_8/lib/webrick/httpstatus.rb	(revision 26266)
+++ ruby_1_8/lib/webrick/httpstatus.rb	(revision 26267)
@@ -12,7 +12,17 @@
 
   module HTTPStatus
 
-    class Status      < StandardError; end
+    class Status      < StandardError
+      def initialize(message, *rest)
+        super(AccessLog.escape(message), *rest)
+      end
+      class << self
+        attr_reader :code, :reason_phrase
+      end
+      def code() self::class::code end
+      def reason_phrase() self::class::reason_phrase end
+      alias to_i code
+    end
     class Info        < Status; end
     class Success     < Status; end
     class Redirect    < Status; end
@@ -68,6 +78,7 @@
     CodeToError = {}
 
     StatusMessage.each{|code, message|
+      message.freeze
       var_name = message.gsub(/[ \-]/,'_').upcase
       err_name = message.gsub(/[ \-]/,'')
 
@@ -79,18 +90,12 @@
       when 500...600; parent = ServerError
       end
 
-      eval %-
-        RC_#{var_name} = #{code}
-        class #{err_name} < #{parent}
-          def self.code() RC_#{var_name} end
-          def self.reason_phrase() StatusMessage[code] end
-          def code() self::class::code end 
-          def reason_phrase() self::class::reason_phrase end
-          alias to_i code
-        end
-      -
-
-      CodeToError[code] = const_get(err_name)
+      const_set("RC_#{var_name}", code)
+      err_class = Class.new(parent)
+      err_class.instance_variable_set(:@code, code)
+      err_class.instance_variable_set(:@reason_phrase, message)
+      const_set(err_name, err_class)
+      CodeToError[code] = err_class
     }
 
     def reason_phrase(code)
Index: ruby_1_8/lib/webrick/httprequest.rb
===================================================================
--- ruby_1_8/lib/webrick/httprequest.rb	(revision 26266)
+++ ruby_1_8/lib/webrick/httprequest.rb	(revision 26267)
@@ -242,11 +242,7 @@
           @raw_header << line
         end
       end
-      begin
-        @header = HTTPUtils::parse_header(@raw_header)
-      rescue => ex
-        raise  HTTPStatus::BadRequest, ex.message
-      end
+      @header = HTTPUtils::parse_header(@raw_header.join)
     end
 
     def parse_uri(str, scheme="http")
Index: ruby_1_8/lib/webrick/httputils.rb
===================================================================
--- ruby_1_8/lib/webrick/httputils.rb	(revision 26266)
+++ ruby_1_8/lib/webrick/httputils.rb	(revision 26267)
@@ -128,11 +128,11 @@
         when /^\s+(.*?)\s*\z/om
           value = $1
           unless field
-            raise "bad header '#{line.inspect}'."
+            raise HTTPStatus::BadRequest, "bad header '#{line}'."
           end
           header[field][-1] << " " << value
         else
-          raise "bad header '#{line.inspect}'."
+          raise HTTPStatus::BadRequest, "bad header '#{line}'."
         end
       }
       header.each{|key, values|
Index: ruby_1_8/lib/webrick/accesslog.rb
===================================================================
--- ruby_1_8/lib/webrick/accesslog.rb	(revision 26266)
+++ ruby_1_8/lib/webrick/accesslog.rb	(revision 26267)
@@ -53,15 +53,23 @@
          when ?e, ?i, ?n, ?o
            raise AccessLogError,
              "parameter is required for \"#{spec}\"" unless param
-           params[spec][param] || "-"
+           param = params[spec][param] ? escape(param) : "-"
          when ?t
            params[spec].strftime(param || CLF_TIME_FORMAT)
          when ?%
            "%"
          else
-           params[spec]
+           escape(params[spec].to_s)
          end
       }
     end
+
+    def escape(data)
+      if data.tainted?
+        data.gsub(/[[:cntrl:]\\]+/) {$&.dump[1...-1]}.untaint
+      else
+        data
+      end
+    end
   end
 end

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]