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/