From 79840f1abf2e7080b72a7749c7cf10387a8e4066 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Thu, 2 Feb 2023 20:26:01 +0100 Subject: [PATCH 01/30] Drop commented-out line (#108) --- lib/webrick/httputils.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index 48aa1371..d95147c5 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -55,7 +55,6 @@ def normalize_path(path) "cer" => "application/pkix-cert", "crl" => "application/pkix-crl", "crt" => "application/x-x509-ca-cert", - #"crl" => "application/x-pkcs7-crl", "css" => "text/css", "dms" => "application/octet-stream", "doc" => "application/msword", From 8f0bda09d35cb5ffcb2f138039ea6b109014aedf Mon Sep 17 00:00:00 2001 From: Ryunosuke Sato Date: Fri, 10 Feb 2023 23:07:14 +0900 Subject: [PATCH 02/30] Add Ruby 3.1 & 3.1 to CI matrix --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0021fa89..2991c5a1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,7 @@ jobs: name: build (${{ matrix.ruby }} / ${{ matrix.os }}) strategy: matrix: - ruby: [ "3.0", 2.7, 2.6, 2.5, 2.4, head ] + ruby: [ 3.2, 3.1, "3.0", 2.7, 2.6, 2.5, 2.4, head ] os: [ ubuntu-latest, macos-latest ] runs-on: ${{ matrix.os }} steps: From c5b7622af70ae6ca26a4067840c52128206c2b3f Mon Sep 17 00:00:00 2001 From: ooooooo_q Date: Sun, 16 Apr 2023 21:17:37 +0900 Subject: [PATCH 03/30] fix ReDoS parse_header --- lib/webrick/httputils.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index d95147c5..d82f95d5 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -157,13 +157,13 @@ def parse_header(raw) field = nil raw.each_line{|line| case line - when /^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):\s*(.*?)\s*\z/om - field, value = $1, $2 + when /^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):(.*?)\z/om + field, value = $1, $2.strip field.downcase! header[field] = [] unless header.has_key?(field) header[field] << value - when /^\s+(.*?)\s*\z/om - value = $1 + when /^\s+(.*?)/om + value = line.strip unless field raise HTTPStatus::BadRequest, "bad header '#{line}'." end From 9e3224864876af9b75b8038545a7962d79995b46 Mon Sep 17 00:00:00 2001 From: ooooooo_q Date: Sun, 16 Apr 2023 23:41:58 +0900 Subject: [PATCH 04/30] fix ReDoS split_header_value --- lib/webrick/httputils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index d82f95d5..6b43146e 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -183,7 +183,7 @@ def parse_header(raw) # Splits a header value +str+ according to HTTP specification. def split_header_value(str) - str.scan(%r'\G((?:"(?:\\.|[^"])+?"|[^",]+)+) + str.scan(%r'\G((?:"(?:\\.|[^"])+?"|[^",]++)+) (?:,\s*|\Z)'xn).flatten end module_function :split_header_value From 96c29264519374ee41eaf27933d5049528264e98 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 15 Aug 2023 14:37:59 -0700 Subject: [PATCH 05/30] Raise HTTPStatus::BadRequest for requests with invalid/duplicate content-length headers Addresses CVE-2023-40225. Fixes #119 --- lib/webrick/httprequest.rb | 8 ++++++++ test/webrick/test_httprequest.rb | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 680ac65a..7a1686bc 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -479,6 +479,14 @@ def read_header(socket) end end @header = HTTPUtils::parse_header(@raw_header.join) + + if (content_length = @header['content-length']) && content_length.length != 0 + if content_length.length > 1 + raise HTTPStatus::BadRequest, "multiple content-length request headers" + elsif !/\A\d+\z/.match?(content_length[0]) + raise HTTPStatus::BadRequest, "invalid content-length request header" + end + end end def parse_uri(str, scheme="http") diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 2ff08d63..90332171 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -81,6 +81,31 @@ def test_request_uri_too_large } end + def test_invalid_content_length_header + ['', ' ', ' +1', ' -1', ' a'].each do |cl| + msg = <<-_end_of_message_ + GET / HTTP/1.1 + Content-Length:#{cl} + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {8}/, ""))) + } + end + end + + def test_duplicate_content_length_header + msg = <<-_end_of_message_ + GET / HTTP/1.1 + Content-Length: 1 + Content-Length: 2 + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + end + def test_parse_headers msg = <<-_end_of_message_ GET /path HTTP/1.1 From 1da346b4a2271075ca970ac9490aa91fd033d660 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Sep 2023 23:03:07 +0000 Subject: [PATCH 06/30] Bump actions/checkout from 3 to 4 Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2991c5a1..27b3082c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,7 +11,7 @@ jobs: os: [ ubuntu-latest, macos-latest ] runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 with: From 52ca8d5384fb83943854fa3b2c6c89548454e348 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 20 Oct 2023 17:28:58 +0900 Subject: [PATCH 07/30] Use re-using workflow --- .github/workflows/test.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 27b3082c..360c5824 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,11 +3,18 @@ name: test on: [push, pull_request] jobs: - build: + ruby-versions: + uses: ruby/actions/.github/workflows/ruby_versions.yml@master + with: + engine: cruby + min_version: 2.4 + + test: + needs: ruby-versions name: build (${{ matrix.ruby }} / ${{ matrix.os }}) strategy: matrix: - ruby: [ 3.2, 3.1, "3.0", 2.7, 2.6, 2.5, 2.4, head ] + ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }} os: [ ubuntu-latest, macos-latest ] runs-on: ${{ matrix.os }} steps: From 289581e566796e0cd5a15f2a4e9afb2f5a63b012 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Fri, 20 Oct 2023 17:43:02 +0900 Subject: [PATCH 08/30] Use test-unit-ruby-core gem --- Gemfile | 1 + Rakefile | 7 - test/lib/core_assertions.rb | 768 ------------------------------------ test/lib/envutil.rb | 367 ----------------- test/lib/find_executable.rb | 22 -- test/lib/helper.rb | 2 +- 6 files changed, 2 insertions(+), 1165 deletions(-) delete mode 100644 test/lib/core_assertions.rb delete mode 100644 test/lib/envutil.rb delete mode 100644 test/lib/find_executable.rb diff --git a/Gemfile b/Gemfile index 10284be2..f5b6c4b6 100644 --- a/Gemfile +++ b/Gemfile @@ -4,3 +4,4 @@ gemspec gem "rake" gem "test-unit" +gem "test-unit-ruby-core" diff --git a/Rakefile b/Rakefile index 5a7afabd..5d512c89 100644 --- a/Rakefile +++ b/Rakefile @@ -7,11 +7,4 @@ Rake::TestTask.new(:test) do |t| t.test_files = FileList["test/**/test_*.rb"] end -task :sync_tool do - require 'fileutils' - FileUtils.cp "../ruby/tool/lib/core_assertions.rb", "./test/lib" - FileUtils.cp "../ruby/tool/lib/envutil.rb", "./test/lib" - FileUtils.cp "../ruby/tool/lib/find_executable.rb", "./test/lib" -end - task :default => :test diff --git a/test/lib/core_assertions.rb b/test/lib/core_assertions.rb deleted file mode 100644 index bac3856a..00000000 --- a/test/lib/core_assertions.rb +++ /dev/null @@ -1,768 +0,0 @@ -# frozen_string_literal: true - -module Test - module Unit - module Assertions - def _assertions= n # :nodoc: - @_assertions = n - end - - def _assertions # :nodoc: - @_assertions ||= 0 - end - - ## - # Returns a proc that will output +msg+ along with the default message. - - def message msg = nil, ending = nil, &default - proc { - msg = msg.call.chomp(".") if Proc === msg - custom_message = "#{msg}.\n" unless msg.nil? or msg.to_s.empty? - "#{custom_message}#{default.call}#{ending || "."}" - } - end - end - - module CoreAssertions - require_relative 'envutil' - require 'pp' - - def mu_pp(obj) #:nodoc: - obj.pretty_inspect.chomp - end - - def assert_file - AssertFile - end - - FailDesc = proc do |status, message = "", out = ""| - now = Time.now - proc do - EnvUtil.failure_description(status, now, message, out) - end - end - - def assert_in_out_err(args, test_stdin = "", test_stdout = [], test_stderr = [], message = nil, - success: nil, **opt) - args = Array(args).dup - args.insert((Hash === args[0] ? 1 : 0), '--disable=gems') - stdout, stderr, status = EnvUtil.invoke_ruby(args, test_stdin, true, true, **opt) - desc = FailDesc[status, message, stderr] - if block_given? - raise "test_stdout ignored, use block only or without block" if test_stdout != [] - raise "test_stderr ignored, use block only or without block" if test_stderr != [] - yield(stdout.lines.map {|l| l.chomp }, stderr.lines.map {|l| l.chomp }, status) - else - all_assertions(desc) do |a| - [["stdout", test_stdout, stdout], ["stderr", test_stderr, stderr]].each do |key, exp, act| - a.for(key) do - if exp.is_a?(Regexp) - assert_match(exp, act) - elsif exp.all? {|e| String === e} - assert_equal(exp, act.lines.map {|l| l.chomp }) - else - assert_pattern_list(exp, act) - end - end - end - unless success.nil? - a.for("success?") do - if success - assert_predicate(status, :success?) - else - assert_not_predicate(status, :success?) - end - end - end - end - status - end - end - - if defined?(RubyVM::InstructionSequence) - def syntax_check(code, fname, line) - code = code.dup.force_encoding(Encoding::UTF_8) - RubyVM::InstructionSequence.compile(code, fname, fname, line) - :ok - ensure - raise if SyntaxError === $! - end - else - def syntax_check(code, fname, line) - code = code.b - code.sub!(/\A(?:\xef\xbb\xbf)?(\s*\#.*$)*(\n)?/n) { - "#$&#{"\n" if $1 && !$2}BEGIN{throw tag, :ok}\n" - } - code = code.force_encoding(Encoding::UTF_8) - catch {|tag| eval(code, binding, fname, line - 1)} - end - end - - def assert_no_memory_leak(args, prepare, code, message=nil, limit: 2.0, rss: false, **opt) - # TODO: consider choosing some appropriate limit for MJIT and stop skipping this once it does not randomly fail - pend 'assert_no_memory_leak may consider MJIT memory usage as leak' if defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled? - - require_relative 'memory_status' - raise Test::Unit::PendedError, "unsupported platform" unless defined?(Memory::Status) - - token = "\e[7;1m#{$$.to_s}:#{Time.now.strftime('%s.%L')}:#{rand(0x10000).to_s(16)}:\e[m" - token_dump = token.dump - token_re = Regexp.quote(token) - envs = args.shift if Array === args and Hash === args.first - args = [ - "--disable=gems", - "-r", File.expand_path("../memory_status", __FILE__), - *args, - "-v", "-", - ] - if defined? Memory::NO_MEMORY_LEAK_ENVS then - envs ||= {} - newenvs = envs.merge(Memory::NO_MEMORY_LEAK_ENVS) { |_, _, _| break } - envs = newenvs if newenvs - end - args.unshift(envs) if envs - cmd = [ - 'END {STDERR.puts '"#{token_dump}"'"FINAL=#{Memory::Status.new}"}', - prepare, - 'STDERR.puts('"#{token_dump}"'"START=#{$initial_status = Memory::Status.new}")', - '$initial_size = $initial_status.size', - code, - 'GC.start', - ].join("\n") - _, err, status = EnvUtil.invoke_ruby(args, cmd, true, true, **opt) - before = err.sub!(/^#{token_re}START=(\{.*\})\n/, '') && Memory::Status.parse($1) - after = err.sub!(/^#{token_re}FINAL=(\{.*\})\n/, '') && Memory::Status.parse($1) - assert(status.success?, FailDesc[status, message, err]) - ([:size, (rss && :rss)] & after.members).each do |n| - b = before[n] - a = after[n] - next unless a > 0 and b > 0 - assert_operator(a.fdiv(b), :<, limit, message(message) {"#{n}: #{b} => #{a}"}) - end - rescue LoadError - pend - end - - # :call-seq: - # assert_nothing_raised( *args, &block ) - # - #If any exceptions are given as arguments, the assertion will - #fail if one of those exceptions are raised. Otherwise, the test fails - #if any exceptions are raised. - # - #The final argument may be a failure message. - # - # assert_nothing_raised RuntimeError do - # raise Exception #Assertion passes, Exception is not a RuntimeError - # end - # - # assert_nothing_raised do - # raise Exception #Assertion fails - # end - def assert_nothing_raised(*args) - self._assertions += 1 - if Module === args.last - msg = nil - else - msg = args.pop - end - begin - line = __LINE__; yield - rescue Test::Unit::PendedError - raise - rescue Exception => e - bt = e.backtrace - as = e.instance_of?(Test::Unit::AssertionFailedError) - if as - ans = /\A#{Regexp.quote(__FILE__)}:#{line}:in /o - bt.reject! {|ln| ans =~ ln} - end - if ((args.empty? && !as) || - args.any? {|a| a.instance_of?(Module) ? e.is_a?(a) : e.class == a }) - msg = message(msg) { - "Exception raised:\n<#{mu_pp(e)}>\n" + - "Backtrace:\n" + - e.backtrace.map{|frame| " #{frame}"}.join("\n") - } - raise Test::Unit::AssertionFailedError, msg.call, bt - else - raise - end - end - end - - def prepare_syntax_check(code, fname = nil, mesg = nil, verbose: nil) - fname ||= caller_locations(2, 1)[0] - mesg ||= fname.to_s - verbose, $VERBOSE = $VERBOSE, verbose - case - when Array === fname - fname, line = *fname - when defined?(fname.path) && defined?(fname.lineno) - fname, line = fname.path, fname.lineno - else - line = 1 - end - yield(code, fname, line, message(mesg) { - if code.end_with?("\n") - "```\n#{code}```\n" - else - "```\n#{code}\n```\n""no-newline" - end - }) - ensure - $VERBOSE = verbose - end - - def assert_valid_syntax(code, *args, **opt) - prepare_syntax_check(code, *args, **opt) do |src, fname, line, mesg| - yield if defined?(yield) - assert_nothing_raised(SyntaxError, mesg) do - assert_equal(:ok, syntax_check(src, fname, line), mesg) - end - end - end - - def assert_normal_exit(testsrc, message = '', child_env: nil, **opt) - assert_valid_syntax(testsrc, caller_locations(1, 1)[0]) - if child_env - child_env = [child_env] - else - child_env = [] - end - out, _, status = EnvUtil.invoke_ruby(child_env + %W'-W0', testsrc, true, :merge_to_stdout, **opt) - assert !status.signaled?, FailDesc[status, message, out] - end - - def assert_ruby_status(args, test_stdin="", message=nil, **opt) - out, _, status = EnvUtil.invoke_ruby(args, test_stdin, true, :merge_to_stdout, **opt) - desc = FailDesc[status, message, out] - assert(!status.signaled?, desc) - message ||= "ruby exit status is not success:" - assert(status.success?, desc) - end - - ABORT_SIGNALS = Signal.list.values_at(*%w"ILL ABRT BUS SEGV TERM") - - def separated_runner(out = nil) - include(*Test::Unit::TestCase.ancestors.select {|c| !c.is_a?(Class) }) - out = out ? IO.new(out, 'w') : STDOUT - at_exit { - out.puts [Marshal.dump($!)].pack('m'), "assertions=#{self._assertions}" - } - Test::Unit::Runner.class_variable_set(:@@stop_auto_run, true) if defined?(Test::Unit::Runner) - end - - def assert_separately(args, file = nil, line = nil, src, ignore_stderr: nil, **opt) - unless file and line - loc, = caller_locations(1,1) - file ||= loc.path - line ||= loc.lineno - end - capture_stdout = true - unless /mswin|mingw/ =~ RUBY_PLATFORM - capture_stdout = false - opt[:out] = Test::Unit::Runner.output if defined?(Test::Unit::Runner) - res_p, res_c = IO.pipe - opt[:ios] = [res_c] - end - src = < marshal_error - ignore_stderr = nil - res = nil - end - if res and !(SystemExit === res) - if bt = res.backtrace - bt.each do |l| - l.sub!(/\A-:(\d+)/){"#{file}:#{line + $1.to_i}"} - end - bt.concat(caller) - else - res.set_backtrace(caller) - end - raise res - end - - # really is it succeed? - unless ignore_stderr - # the body of assert_separately must not output anything to detect error - assert(stderr.empty?, FailDesc[status, "assert_separately failed with error message", stderr]) - end - assert(status.success?, FailDesc[status, "assert_separately failed", stderr]) - raise marshal_error if marshal_error - end - - # Run Ractor-related test without influencing the main test suite - def assert_ractor(src, args: [], require: nil, require_relative: nil, file: nil, line: nil, ignore_stderr: nil, **opt) - return unless defined?(Ractor) - - require = "require #{require.inspect}" if require - if require_relative - dir = File.dirname(caller_locations[0,1][0].absolute_path) - full_path = File.expand_path(require_relative, dir) - require = "#{require}; require #{full_path.inspect}" - end - - assert_separately(args, file, line, <<~RUBY, ignore_stderr: ignore_stderr, **opt) - #{require} - previous_verbose = $VERBOSE - $VERBOSE = nil - Ractor.new {} # trigger initial warning - $VERBOSE = previous_verbose - #{src} - RUBY - end - - # :call-seq: - # assert_throw( tag, failure_message = nil, &block ) - # - #Fails unless the given block throws +tag+, returns the caught - #value otherwise. - # - #An optional failure message may be provided as the final argument. - # - # tag = Object.new - # assert_throw(tag, "#{tag} was not thrown!") do - # throw tag - # end - def assert_throw(tag, msg = nil) - ret = catch(tag) do - begin - yield(tag) - rescue UncaughtThrowError => e - thrown = e.tag - end - msg = message(msg) { - "Expected #{mu_pp(tag)} to have been thrown"\ - "#{%Q[, not #{thrown}] if thrown}" - } - assert(false, msg) - end - assert(true) - ret - end - - # :call-seq: - # assert_raise( *args, &block ) - # - #Tests if the given block raises an exception. Acceptable exception - #types may be given as optional arguments. If the last argument is a - #String, it will be used as the error message. - # - # assert_raise do #Fails, no Exceptions are raised - # end - # - # assert_raise NameError do - # puts x #Raises NameError, so assertion succeeds - # end - def assert_raise(*exp, &b) - case exp.last - when String, Proc - msg = exp.pop - end - - begin - yield - rescue Test::Unit::PendedError => e - return e if exp.include? Test::Unit::PendedError - raise e - rescue Exception => e - expected = exp.any? { |ex| - if ex.instance_of? Module then - e.kind_of? ex - else - e.instance_of? ex - end - } - - assert expected, proc { - flunk(message(msg) {"#{mu_pp(exp)} exception expected, not #{mu_pp(e)}"}) - } - - return e - ensure - unless e - exp = exp.first if exp.size == 1 - - flunk(message(msg) {"#{mu_pp(exp)} expected but nothing was raised"}) - end - end - end - - # :call-seq: - # assert_raise_with_message(exception, expected, msg = nil, &block) - # - #Tests if the given block raises an exception with the expected - #message. - # - # assert_raise_with_message(RuntimeError, "foo") do - # nil #Fails, no Exceptions are raised - # end - # - # assert_raise_with_message(RuntimeError, "foo") do - # raise ArgumentError, "foo" #Fails, different Exception is raised - # end - # - # assert_raise_with_message(RuntimeError, "foo") do - # raise "bar" #Fails, RuntimeError is raised but the message differs - # end - # - # assert_raise_with_message(RuntimeError, "foo") do - # raise "foo" #Raises RuntimeError with the message, so assertion succeeds - # end - def assert_raise_with_message(exception, expected, msg = nil, &block) - case expected - when String - assert = :assert_equal - when Regexp - assert = :assert_match - else - raise TypeError, "Expected #{expected.inspect} to be a kind of String or Regexp, not #{expected.class}" - end - - ex = m = nil - EnvUtil.with_default_internal(expected.encoding) do - ex = assert_raise(exception, msg || proc {"Exception(#{exception}) with message matches to #{expected.inspect}"}) do - yield - end - m = ex.message - end - msg = message(msg, "") {"Expected Exception(#{exception}) was raised, but the message doesn't match"} - - if assert == :assert_equal - assert_equal(expected, m, msg) - else - msg = message(msg) { "Expected #{mu_pp expected} to match #{mu_pp m}" } - assert expected =~ m, msg - block.binding.eval("proc{|_|$~=_}").call($~) - end - ex - end - - MINI_DIR = File.join(File.dirname(File.expand_path(__FILE__)), "minitest") #:nodoc: - - # :call-seq: - # assert(test, [failure_message]) - # - #Tests if +test+ is true. - # - #+msg+ may be a String or a Proc. If +msg+ is a String, it will be used - #as the failure message. Otherwise, the result of calling +msg+ will be - #used as the message if the assertion fails. - # - #If no +msg+ is given, a default message will be used. - # - # assert(false, "This was expected to be true") - def assert(test, *msgs) - case msg = msgs.first - when String, Proc - when nil - msgs.shift - else - bt = caller.reject { |s| s.start_with?(MINI_DIR) } - raise ArgumentError, "assertion message must be String or Proc, but #{msg.class} was given.", bt - end unless msgs.empty? - super - end - - # :call-seq: - # assert_respond_to( object, method, failure_message = nil ) - # - #Tests if the given Object responds to +method+. - # - #An optional failure message may be provided as the final argument. - # - # assert_respond_to("hello", :reverse) #Succeeds - # assert_respond_to("hello", :does_not_exist) #Fails - def assert_respond_to(obj, (meth, *priv), msg = nil) - unless priv.empty? - msg = message(msg) { - "Expected #{mu_pp(obj)} (#{obj.class}) to respond to ##{meth}#{" privately" if priv[0]}" - } - return assert obj.respond_to?(meth, *priv), msg - end - #get rid of overcounting - if caller_locations(1, 1)[0].path.start_with?(MINI_DIR) - return if obj.respond_to?(meth) - end - super(obj, meth, msg) - end - - # :call-seq: - # assert_not_respond_to( object, method, failure_message = nil ) - # - #Tests if the given Object does not respond to +method+. - # - #An optional failure message may be provided as the final argument. - # - # assert_not_respond_to("hello", :reverse) #Fails - # assert_not_respond_to("hello", :does_not_exist) #Succeeds - def assert_not_respond_to(obj, (meth, *priv), msg = nil) - unless priv.empty? - msg = message(msg) { - "Expected #{mu_pp(obj)} (#{obj.class}) to not respond to ##{meth}#{" privately" if priv[0]}" - } - return assert !obj.respond_to?(meth, *priv), msg - end - #get rid of overcounting - if caller_locations(1, 1)[0].path.start_with?(MINI_DIR) - return unless obj.respond_to?(meth) - end - refute_respond_to(obj, meth, msg) - end - - # pattern_list is an array which contains regexp and :*. - # :* means any sequence. - # - # pattern_list is anchored. - # Use [:*, regexp, :*] for non-anchored match. - def assert_pattern_list(pattern_list, actual, message=nil) - rest = actual - anchored = true - pattern_list.each_with_index {|pattern, i| - if pattern == :* - anchored = false - else - if anchored - match = /\A#{pattern}/.match(rest) - else - match = pattern.match(rest) - end - unless match - msg = message(msg) { - expect_msg = "Expected #{mu_pp pattern}\n" - if /\n[^\n]/ =~ rest - actual_mesg = +"to match\n" - rest.scan(/.*\n+/) { - actual_mesg << ' ' << $&.inspect << "+\n" - } - actual_mesg.sub!(/\+\n\z/, '') - else - actual_mesg = "to match " + mu_pp(rest) - end - actual_mesg << "\nafter #{i} patterns with #{actual.length - rest.length} characters" - expect_msg + actual_mesg - } - assert false, msg - end - rest = match.post_match - anchored = true - end - } - if anchored - assert_equal("", rest) - end - end - - def assert_warning(pat, msg = nil) - result = nil - stderr = EnvUtil.with_default_internal(pat.encoding) { - EnvUtil.verbose_warning { - result = yield - } - } - msg = message(msg) {diff pat, stderr} - assert(pat === stderr, msg) - result - end - - def assert_warn(*args) - assert_warning(*args) {$VERBOSE = false; yield} - end - - def assert_deprecated_warning(mesg = /deprecated/) - assert_warning(mesg) do - Warning[:deprecated] = true - yield - end - end - - def assert_deprecated_warn(mesg = /deprecated/) - assert_warn(mesg) do - Warning[:deprecated] = true - yield - end - end - - class << (AssertFile = Struct.new(:failure_message).new) - include Assertions - include CoreAssertions - def assert_file_predicate(predicate, *args) - if /\Anot_/ =~ predicate - predicate = $' - neg = " not" - end - result = File.__send__(predicate, *args) - result = !result if neg - mesg = "Expected file ".dup << args.shift.inspect - mesg << "#{neg} to be #{predicate}" - mesg << mu_pp(args).sub(/\A\[(.*)\]\z/m, '(\1)') unless args.empty? - mesg << " #{failure_message}" if failure_message - assert(result, mesg) - end - alias method_missing assert_file_predicate - - def for(message) - clone.tap {|a| a.failure_message = message} - end - end - - class AllFailures - attr_reader :failures - - def initialize - @count = 0 - @failures = {} - end - - def for(key) - @count += 1 - yield - rescue Exception => e - @failures[key] = [@count, e] - end - - def foreach(*keys) - keys.each do |key| - @count += 1 - begin - yield key - rescue Exception => e - @failures[key] = [@count, e] - end - end - end - - def message - i = 0 - total = @count.to_s - fmt = "%#{total.size}d" - @failures.map {|k, (n, v)| - v = v.message - "\n#{i+=1}. [#{fmt%n}/#{total}] Assertion for #{k.inspect}\n#{v.b.gsub(/^/, ' | ').force_encoding(v.encoding)}" - }.join("\n") - end - - def pass? - @failures.empty? - end - end - - # threads should respond to shift method. - # Array can be used. - def assert_join_threads(threads, message = nil) - errs = [] - values = [] - while th = threads.shift - begin - values << th.value - rescue Exception - errs << [th, $!] - th = nil - end - end - values - ensure - if th&.alive? - th.raise(Timeout::Error.new) - th.join rescue errs << [th, $!] - end - if !errs.empty? - msg = "exceptions on #{errs.length} threads:\n" + - errs.map {|t, err| - "#{t.inspect}:\n" + - RUBY_VERSION >= "2.5.0" ? err.full_message(highlight: false, order: :top) : err.message - }.join("\n---\n") - if message - msg = "#{message}\n#{msg}" - end - raise Test::Unit::AssertionFailedError, msg - end - end - - def assert_all?(obj, m = nil, &blk) - failed = [] - obj.each do |*a, &b| - unless blk.call(*a, &b) - failed << (a.size > 1 ? a : a[0]) - end - end - assert(failed.empty?, message(m) {failed.pretty_inspect}) - end - - def assert_all_assertions(msg = nil) - all = AllFailures.new - yield all - ensure - assert(all.pass?, message(msg) {all.message.chomp(".")}) - end - alias all_assertions assert_all_assertions - - def assert_all_assertions_foreach(msg = nil, *keys, &block) - all = AllFailures.new - all.foreach(*keys, &block) - ensure - assert(all.pass?, message(msg) {all.message.chomp(".")}) - end - alias all_assertions_foreach assert_all_assertions_foreach - - def message(msg = nil, *args, &default) # :nodoc: - if Proc === msg - super(nil, *args) do - ary = [msg.call, (default.call if default)].compact.reject(&:empty?) - if 1 < ary.length - ary[0...-1] = ary[0...-1].map {|str| str.sub(/(? Date: Fri, 20 Oct 2023 17:46:42 +0900 Subject: [PATCH 09/30] Use assert_raise instead of assert_raises --- test/webrick/test_httpresponse.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/webrick/test_httpresponse.rb b/test/webrick/test_httpresponse.rb index 83734533..6c4a1794 100644 --- a/test/webrick/test_httpresponse.rb +++ b/test/webrick/test_httpresponse.rb @@ -97,14 +97,14 @@ def test_prevent_response_splitting_cookie_headers_lf def test_set_redirect_response_splitting url = "malicious\r\nCookie: cracked_indicator_for_test" - assert_raises(URI::InvalidURIError) do + assert_raise(URI::InvalidURIError) do res.set_redirect(WEBrick::HTTPStatus::MultipleChoices, url) end end def test_set_redirect_html_injection url = 'http://example.com////?a' - assert_raises(WEBrick::HTTPStatus::MultipleChoices) do + assert_raise(WEBrick::HTTPStatus::MultipleChoices) do res.set_redirect(WEBrick::HTTPStatus::MultipleChoices, url) end res.status = 300 From 3704b62505b549baba9c0316d12ed9e116d7c6d6 Mon Sep 17 00:00:00 2001 From: KJ Tsanaktsidis Date: Wed, 27 Dec 2023 13:17:28 +1100 Subject: [PATCH 10/30] Fix WEBrick::TestFileHandler#test_short_filename test not working on mswin The test is currently skipped and can't possibly work on windows at the moment. It fails because $LOAD_PATH is not set up properly in the forked CGI process, so require 'uri' fails. This works properly in the test_cgi.rb tests, because it sets up a :RequestCallback to fix things up. Let's move the setup there into util.rb, so it can be shared with test_filehandler.rb as well. --- test/webrick/test_cgi.rb | 32 +++++--------------------------- test/webrick/test_filehandler.rb | 9 ++------- test/webrick/utils.rb | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 34 deletions(-) diff --git a/test/webrick/test_cgi.rb b/test/webrick/test_cgi.rb index 7a75cf56..a9be8f35 100644 --- a/test/webrick/test_cgi.rb +++ b/test/webrick/test_cgi.rb @@ -12,30 +12,8 @@ def teardown super end - def start_cgi_server(log_tester=TestWEBrick::DefaultLogTester, &block) - config = { - :CGIInterpreter => TestWEBrick::RubyBin, - :DocumentRoot => File.dirname(__FILE__), - :DirectoryIndex => ["webrick.cgi"], - :RequestCallback => Proc.new{|req, res| - def req.meta_vars - meta = super - meta["RUBYLIB"] = $:.join(File::PATH_SEPARATOR) - meta[RbConfig::CONFIG['LIBPATHENV']] = ENV[RbConfig::CONFIG['LIBPATHENV']] if RbConfig::CONFIG['LIBPATHENV'] - return meta - end - }, - } - if RUBY_PLATFORM =~ /mswin|mingw|cygwin|bccwin32/ - config[:CGIPathEnv] = ENV['PATH'] # runtime dll may not be in system dir. - end - TestWEBrick.start_httpserver(config, log_tester){|server, addr, port, log| - block.call(server, addr, port, log) - } - end - def test_cgi - start_cgi_server{|server, addr, port, log| + TestWEBrick.start_cgi_server{|server, addr, port, log| http = Net::HTTP.new(addr, port) req = Net::HTTP::Get.new("/webrick.cgi") http.request(req){|res| assert_equal("/webrick.cgi", res.body, log.call)} @@ -98,7 +76,7 @@ def test_bad_request log_tester = lambda {|log, access_log| assert_match(/BadRequest/, log.join) } - start_cgi_server(log_tester) {|server, addr, port, log| + TestWEBrick.start_cgi_server({}, log_tester) {|server, addr, port, log| sock = TCPSocket.new(addr, port) begin sock << "POST /webrick.cgi HTTP/1.0" << CRLF @@ -115,7 +93,7 @@ def test_bad_request end def test_cgi_env - start_cgi_server do |server, addr, port, log| + TestWEBrick.start_cgi_server do |server, addr, port, log| http = Net::HTTP.new(addr, port) req = Net::HTTP::Get.new("/webrick.cgi/dumpenv") req['proxy'] = 'http://example.com/' @@ -137,7 +115,7 @@ def test_bad_uri assert_equal(1, log.length) assert_match(/ERROR bad URI/, log[0]) } - start_cgi_server(log_tester) {|server, addr, port, log| + TestWEBrick.start_cgi_server({}, log_tester) {|server, addr, port, log| res = TCPSocket.open(addr, port) {|sock| sock << "GET /#{CtrlSeq}#{CRLF}#{CRLF}" sock.close_write @@ -155,7 +133,7 @@ def test_bad_header assert_equal(1, log.length) assert_match(/ERROR bad header/, log[0]) } - start_cgi_server(log_tester) {|server, addr, port, log| + TestWEBrick.start_cgi_server({}, log_tester) {|server, addr, port, log| res = TCPSocket.open(addr, port) {|sock| sock << "GET / HTTP/1.0#{CRLF}#{CtrlSeq}#{CRLF}#{CRLF}" sock.close_write diff --git a/test/webrick/test_filehandler.rb b/test/webrick/test_filehandler.rb index 881fb54d..452667d4 100644 --- a/test/webrick/test_filehandler.rb +++ b/test/webrick/test_filehandler.rb @@ -248,20 +248,15 @@ def test_unwise_in_path def test_short_filename return if File.executable?(__FILE__) # skip on strange file system - config = { - :CGIInterpreter => TestWEBrick::RubyBin, - :DocumentRoot => File.dirname(__FILE__), - :CGIPathEnv => ENV['PATH'], - } log_tester = lambda {|log, access_log| log = log.reject {|s| /ERROR `.*\' not found\./ =~ s } log = log.reject {|s| /WARN the request refers nondisclosure name/ =~ s } assert_equal([], log) } - TestWEBrick.start_httpserver(config, log_tester) do |server, addr, port, log| + TestWEBrick.start_cgi_server({}, log_tester) do |server, addr, port, log| http = Net::HTTP.new(addr, port) if windows? - root = config[:DocumentRoot].tr("/", "\\") + root = File.dirname(__FILE__).tr("/", "\\") fname = IO.popen(%W[dir /x #{root}\\webrick_long_filename.cgi], encoding: "binary", &:read) fname.sub!(/\A.*$^$.*$^$/m, '') if fname diff --git a/test/webrick/utils.rb b/test/webrick/utils.rb index a8568d0a..c8e84c37 100644 --- a/test/webrick/utils.rb +++ b/test/webrick/utils.rb @@ -81,4 +81,24 @@ def start_httpserver(config={}, log_tester=DefaultLogTester, &block) def start_httpproxy(config={}, log_tester=DefaultLogTester, &block) start_server(WEBrick::HTTPProxyServer, config, log_tester, &block) end + + def start_cgi_server(config={}, log_tester=TestWEBrick::DefaultLogTester, &block) + config = { + :CGIInterpreter => TestWEBrick::RubyBin, + :DocumentRoot => File.dirname(__FILE__), + :DirectoryIndex => ["webrick.cgi"], + :RequestCallback => Proc.new{|req, res| + def req.meta_vars + meta = super + meta["RUBYLIB"] = $:.join(File::PATH_SEPARATOR) + meta[RbConfig::CONFIG['LIBPATHENV']] = ENV[RbConfig::CONFIG['LIBPATHENV']] if RbConfig::CONFIG['LIBPATHENV'] + return meta + end + }, + }.merge(config) + if RUBY_PLATFORM =~ /mswin|mingw|cygwin|bccwin32/ + config[:CGIPathEnv] = ENV['PATH'] # runtime dll may not be in system dir. + end + start_server(WEBrick::HTTPServer, config, log_tester, &block) + end end From f9336e0bc3bf8d0e271c96285f92be59c6b04d3c Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Sat, 11 Nov 2023 16:46:08 -0800 Subject: [PATCH 11/30] Fix bug chunk extension detection This fixes a request smuggling vulnerability (Fixes #124). Co-authored-by: Ben Kallus --- lib/webrick/httprequest.rb | 2 +- test/webrick/test_httprequest.rb | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 7a1686bc..0cfc1c95 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -542,7 +542,7 @@ def read_body(socket, block) def read_chunk_size(socket) line = read_line(socket) - if /^([0-9a-fA-F]+)(?:;(\S+))?/ =~ line + if /\A([0-9a-fA-F]+)(?:;(\S+(?:=\S+)?))?\r\n\z/ =~ line chunk_size = $1.hex chunk_ext = $2 [ chunk_size, chunk_ext ] diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 90332171..73fa8a3d 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -289,6 +289,31 @@ def test_chunked assert_equal(expect, dst.string) end + def test_bad_chunked + crlf = "\x0d\x0a" + expect = File.binread(__FILE__).freeze + msg = <<-_end_of_message_ + POST /path HTTP/1.1\r + Transfer-Encoding: chunked\r + \r + 01x1\r + \r + 1 + _end_of_message_ + msg.gsub!(/^ {6}/, "") + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg)) + assert_raise(WEBrick::HTTPStatus::BadRequest){ req.body } + + # chunked req.body_reader + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg)) + dst = StringIO.new + assert_raise(WEBrick::HTTPStatus::BadRequest) do + IO.copy_stream(req.body_reader, dst) + end + end + def test_forwarded msg = <<-_end_of_message_ GET /foo HTTP/1.1 From a6342a59261781242cb1eabde4d3d2485c57e46a Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 13 Nov 2023 12:28:57 -0800 Subject: [PATCH 12/30] Make \r optional in chunk size detection As pointed out by Ken Ballus, WEBrick allows \n without preceding \r generally. It probably shouldn't, but since it does, do not require \r for chunk size detection. --- lib/webrick/httprequest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 0cfc1c95..02c5dcb6 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -542,7 +542,7 @@ def read_body(socket, block) def read_chunk_size(socket) line = read_line(socket) - if /\A([0-9a-fA-F]+)(?:;(\S+(?:=\S+)?))?\r\n\z/ =~ line + if /\A([0-9a-fA-F]+)(?:;(\S+(?:=\S+)?))?\r?\n\z/ =~ line chunk_size = $1.hex chunk_ext = $2 [ chunk_size, chunk_ext ] From 75e1f10bf1fd00629128cb19515241c2486f45c8 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 13 Nov 2023 12:31:30 -0800 Subject: [PATCH 13/30] Remove unneeded lines in new test --- test/webrick/test_httprequest.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 73fa8a3d..471005c6 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -290,8 +290,6 @@ def test_chunked end def test_bad_chunked - crlf = "\x0d\x0a" - expect = File.binread(__FILE__).freeze msg = <<-_end_of_message_ POST /path HTTP/1.1\r Transfer-Encoding: chunked\r From abf2bc3710c17e3c0b560eea87effe937f9a0766 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 1 Dec 2023 11:29:47 -0800 Subject: [PATCH 14/30] Revert "Make \r optional in chunk size detection" This reverts commit 44cb9cb31cb9494fd820671738219ca4fc5e4f2a. --- lib/webrick/httprequest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 02c5dcb6..0cfc1c95 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -542,7 +542,7 @@ def read_body(socket, block) def read_chunk_size(socket) line = read_line(socket) - if /\A([0-9a-fA-F]+)(?:;(\S+(?:=\S+)?))?\r?\n\z/ =~ line + if /\A([0-9a-fA-F]+)(?:;(\S+(?:=\S+)?))?\r\n\z/ =~ line chunk_size = $1.hex chunk_ext = $2 [ chunk_size, chunk_ext ] From cd0b1dd2286492287876773e719bb50a533d2698 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 1 Dec 2023 12:47:24 -0800 Subject: [PATCH 15/30] Reject null bytes in header lines Fixes #126 --- lib/webrick/httprequest.rb | 3 +++ test/webrick/test_httprequest.rb | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 0cfc1c95..43e4e8a3 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -475,6 +475,9 @@ def read_header(socket) if (@request_bytes += line.bytesize) > MAX_HEADER_LENGTH raise HTTPStatus::RequestEntityTooLarge, 'headers too large' end + if line.include?("\x00") + raise HTTPStatus::BadRequest, 'null byte in header' + end @raw_header << line end end diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 471005c6..c0fb2e9d 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -312,6 +312,17 @@ def test_bad_chunked end end + def test_null_byte_in_header + msg = <<-_end_of_message_ + POST /path HTTP/1.1\r + Evil: evil\x00\r + \r + _end_of_message_ + msg.gsub!(/^ {6}/, "") + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg)) } + end + def test_forwarded msg = <<-_end_of_message_ GET /foo HTTP/1.1 From d1dbce884f2b40e766f19aa0f9d2131bb9e30624 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 28 Mar 2024 09:45:01 +0900 Subject: [PATCH 16/30] Use www.rfc-editor.org for RFC text. We use the following site for that now: * https://tools.ietf.org/ or http * https://datatracker.ietf.org or http Today, IETF said the official site of RFC is www.rfc-editor.org. FYI: https://authors.ietf.org/en/references-in-rfcxml I replaced them to www.rfc-editor.org. --- lib/webrick/httprequest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 43e4e8a3..c422c737 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -402,7 +402,7 @@ def fixup() # :nodoc: # This method provides the metavariables defined by the revision 3 # of "The WWW Common Gateway Interface Version 1.1" # To browse the current document of CGI Version 1.1, see below: - # http://tools.ietf.org/html/rfc3875 + # https://www.rfc-editor.org/rfc/rfc3875 def meta_vars meta = Hash.new From 76751152d49ed9851cd1d3dc4c04a04796e36319 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sat, 8 Jun 2024 10:13:43 +0900 Subject: [PATCH 17/30] Fix CI by explicit use of `macos-13` instead of `-latest`. (#131) macos-latest no longer supports Ruby < 2.7 due to architectural changes. --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 360c5824..a4534c8a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }} - os: [ ubuntu-latest, macos-latest ] + os: [ ubuntu-latest, macos-13 ] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 From d2e2cac9929296a2f5f995de7e3875c5f5ddb11b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sat, 8 Jun 2024 10:21:45 +0900 Subject: [PATCH 18/30] Merge multiple cookie headers, preserving semantic correctness. (#130) --- lib/webrick/httprequest.rb | 4 ++-- lib/webrick/httputils.rb | 18 +++++++++++++++++- test/webrick/test_httprequest.rb | 7 +++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index c422c737..80b01e95 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -318,7 +318,7 @@ def content_type def [](header_name) if @header value = @header[header_name.downcase] - value.empty? ? nil : value.join(", ") + value.empty? ? nil : value.join end end @@ -329,7 +329,7 @@ def each if @header @header.each{|k, v| value = @header[k] - yield(k, value.empty? ? nil : value.join(", ")) + yield(k, value.empty? ? nil : value.join) } end end diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index 6b43146e..1653a072 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -152,6 +152,22 @@ def mime_type(filename, mime_tab) # Parses an HTTP header +raw+ into a hash of header fields with an Array # of values. + class SplitHeader < Array + def join(separator = ", ") + super + end + end + + class CookieHeader < Array + def join(separator = "; ") + super + end + end + + HEADER_CLASSES = Hash.new(SplitHeader).update({ + "cookie" => CookieHeader, + }) + def parse_header(raw) header = Hash.new([].freeze) field = nil @@ -160,7 +176,7 @@ def parse_header(raw) when /^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):(.*?)\z/om field, value = $1, $2.strip field.downcase! - header[field] = [] unless header.has_key?(field) + header[field] = HEADER_CLASSES[field].new unless header.has_key?(field) header[field] << value when /^\s+(.*?)/om value = line.strip diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index c0fb2e9d..87a27523 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -532,4 +532,11 @@ def test_eof_raised_when_line_is_nil req.parse(StringIO.new("")) } end + + def test_cookie_join + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new("GET / HTTP/1.1\r\ncookie: a=1\r\ncookie: b=2\r\n\r\n")) + assert_equal 2, req.cookies.length + assert_equal 'a=1; b=2', req['cookie'] + end end From 6f412123e368c5cc1059127993c793ef44518409 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sat, 8 Jun 2024 12:09:47 +0200 Subject: [PATCH 19/30] Test on macos-latest But exclude Ruby 2.4 and 2.5 because they aren't supported on macos-arm64. --- .github/workflows/test.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a4534c8a..d52899c4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,9 +13,15 @@ jobs: needs: ruby-versions name: build (${{ matrix.ruby }} / ${{ matrix.os }}) strategy: + fail-fast: false matrix: ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }} - os: [ ubuntu-latest, macos-13 ] + os: [ ubuntu-latest, macos-latest ] + exclude: + - os: macos-latest + ruby: "2.4" + - os: macos-latest + ruby: "2.5" runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 From a27d7ed45f630d9ee9a7e8cbd0542542e36a3219 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Sat, 8 Jun 2024 13:20:53 +0200 Subject: [PATCH 20/30] Test 2.4 & 2.5 on macos-amd64 --- .github/workflows/test.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d52899c4..edede5b1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,11 +17,13 @@ jobs: matrix: ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }} os: [ ubuntu-latest, macos-latest ] + # CRuby < 2.6 does not support macos-arm64, so test those on amd64 instead exclude: - - os: macos-latest - ruby: "2.4" - - os: macos-latest - ruby: "2.5" + - { os: macos-latest, ruby: '2.4' } + - { os: macos-latest, ruby: '2.5' } + include: + - { os: macos-13, ruby: '2.4' } + - { os: macos-13, ruby: '2.5' } runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 From ee60354bcb84ec33b9245e1d1aa6e1f7e8132101 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 25 Jun 2024 14:39:04 -0700 Subject: [PATCH 21/30] Require CRLF line endings in request line and headers Disallow bare CR, LF, NUL in header and request lines. Tighten parsing of request lines to only allow single spaces, as specified in the RFCs. Forcing this RFC-compliant behavior breaks a lot of tests, so fix the tests to correctly use CRLF instead of LF for requests (other than the specific checks for handling of bad requests). Fixes #137 --- lib/webrick/httprequest.rb | 4 +- lib/webrick/httputils.rb | 10 ++- test/webrick/test_filehandler.rb | 2 +- test/webrick/test_httprequest.rb | 149 +++++++++++++++++++++++++------ 4 files changed, 133 insertions(+), 32 deletions(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 80b01e95..62ea54c8 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -458,7 +458,7 @@ def read_request_line(socket) end @request_time = Time.now - if /^(\S+)\s+(\S++)(?:\s+HTTP\/(\d+\.\d+))?\r?\n/mo =~ @request_line + if /^(\S+) (\S++)(?: HTTP\/(\d+\.\d+))?\r\n/mo =~ @request_line @request_method = $1 @unparsed_uri = $2 @http_version = HTTPVersion.new($3 ? $3 : "0.9") @@ -471,7 +471,7 @@ def read_request_line(socket) def read_header(socket) if socket while line = read_line(socket) - break if /\A(#{CRLF}|#{LF})\z/om =~ line + break if /\A#{CRLF}\z/om =~ line if (@request_bytes += line.bytesize) > MAX_HEADER_LENGTH raise HTTPStatus::RequestEntityTooLarge, 'headers too large' end diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index 1653a072..ea67fdb0 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -173,16 +173,18 @@ def parse_header(raw) field = nil raw.each_line{|line| case line - when /^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):(.*?)\z/om - field, value = $1, $2.strip + when /^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):([^\r\n\0]*?)\r\n\z/om + field, value = $1, $2 field.downcase! header[field] = HEADER_CLASSES[field].new unless header.has_key?(field) header[field] << value - when /^\s+(.*?)/om - value = line.strip + when /^\s+([^\r\n\0]*?)\r\n/om unless field raise HTTPStatus::BadRequest, "bad header '#{line}'." end + value = line + value.lstrip! + value.slice!(-2..-1) header[field][-1] << " " << value else raise HTTPStatus::BadRequest, "bad header '#{line}'." diff --git a/test/webrick/test_filehandler.rb b/test/webrick/test_filehandler.rb index 452667d4..51654394 100644 --- a/test/webrick/test_filehandler.rb +++ b/test/webrick/test_filehandler.rb @@ -33,7 +33,7 @@ def make_range_request(range_spec) Range: #{range_spec} END_OF_REQUEST - return StringIO.new(msg.gsub(/^ {6}/, "")) + return StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n")) end def make_range_response(file, range_spec) diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 87a27523..fa181773 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -11,7 +11,7 @@ def teardown def test_simple_request msg = <<-_end_of_message_ -GET / +GET /\r _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) @@ -24,7 +24,7 @@ def test_parse_09 foobar # HTTP/0.9 request don't have header nor entity body. _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal("GET", req.request_method) assert_equal("/", req.unparsed_uri) assert_equal(WEBrick::HTTPVersion.new("0.9"), req.http_version) @@ -41,7 +41,7 @@ def test_parse_10 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal("GET", req.request_method) assert_equal("/", req.unparsed_uri) assert_equal(WEBrick::HTTPVersion.new("1.0"), req.http_version) @@ -58,7 +58,7 @@ def test_parse_11 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal("GET", req.request_method) assert_equal("/path", req.unparsed_uri) assert_equal("", req.script_name) @@ -77,7 +77,7 @@ def test_request_uri_too_large _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::RequestURITooLarge){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) } end @@ -89,11 +89,101 @@ def test_invalid_content_length_header _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {8}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {8}/, "").gsub("\n", "\r\n"))) } end end + def test_bare_lf_request_line + msg = <<-_end_of_message_ + GET / HTTP/1.1 + Content-Length: 0\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + end + + def test_bare_lf_header + msg = <<-_end_of_message_ + GET / HTTP/1.1\r + Content-Length: 0 + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + end + + def test_bare_cr_request_line + msg = <<-_end_of_message_ + GET / HTTP/1.1\r\r + Content-Length: 0\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + end + + def test_bare_cr_header + msg = <<-_end_of_message_ + GET / HTTP/1.1\r + Content-Type: foo\rbar\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + end + + def test_invalid_request_lines + msg = <<-_end_of_message_ + GET / HTTP/1.1\r + Content-Length: 0\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + + msg = <<-_end_of_message_ + GET / HTTP/1.1\r + Content-Length: 0\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + + msg = <<-_end_of_message_ + GET /\r HTTP/1.1\r + Content-Length: 0\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + + msg = <<-_end_of_message_ + GET / HTTP/1.1 \r + Content-Length: 0\r + \r + _end_of_message_ + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + } + end + def test_duplicate_content_length_header msg = <<-_end_of_message_ GET / HTTP/1.1 @@ -102,7 +192,7 @@ def test_duplicate_content_length_header _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) } end @@ -118,13 +208,13 @@ def test_parse_headers Accept-Language: en;q=0.5, *; q=0 Accept-Language: ja Content-Type: text/plain - Content-Length: 7 + Content-Length: 8 X-Empty-Header: foobar _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal( URI.parse("http://test.ruby-lang.org:8080/path"), req.request_uri) assert_equal("test.ruby-lang.org", req.host) @@ -135,9 +225,9 @@ def test_parse_headers req.accept) assert_equal(%w(gzip compress identity *), req.accept_encoding) assert_equal(%w(ja en *), req.accept_language) - assert_equal(7, req.content_length) + assert_equal(8, req.content_length) assert_equal("text/plain", req.content_type) - assert_equal("foobar\n", req.body) + assert_equal("foobar\r\n", req.body) assert_equal("", req["x-empty-header"]) assert_equal(nil, req["x-no-header"]) assert(req.query.empty?) @@ -146,7 +236,7 @@ def test_parse_headers def test_parse_header2() msg = <<-_end_of_message_ POST /foo/bar/../baz?q=a HTTP/1.0 - Content-Length: 9 + Content-Length: 10 User-Agent: FOO BAR BAZ @@ -154,14 +244,14 @@ def test_parse_header2() hogehoge _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal("POST", req.request_method) assert_equal("/foo/baz", req.path) assert_equal("", req.script_name) assert_equal("/foo/baz", req.path_info) - assert_equal("9", req['content-length']) + assert_equal("10", req['content-length']) assert_equal("FOO BAR BAZ", req['user-agent']) - assert_equal("hogehoge\n", req.body) + assert_equal("hogehoge\r\n", req.body) end def test_parse_headers3 @@ -171,7 +261,7 @@ def test_parse_headers3 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal(URI.parse("http://test.ruby-lang.org/path"), req.request_uri) assert_equal("test.ruby-lang.org", req.host) assert_equal(80, req.port) @@ -182,7 +272,7 @@ def test_parse_headers3 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal(URI.parse("http://192.168.1.1/path"), req.request_uri) assert_equal("192.168.1.1", req.host) assert_equal(80, req.port) @@ -193,7 +283,7 @@ def test_parse_headers3 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal(URI.parse("http://[fe80::208:dff:feef:98c7]/path"), req.request_uri) assert_equal("[fe80::208:dff:feef:98c7]", req.host) @@ -205,7 +295,7 @@ def test_parse_headers3 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal(URI.parse("http://192.168.1.1:8080/path"), req.request_uri) assert_equal("192.168.1.1", req.host) assert_equal(8080, req.port) @@ -216,7 +306,7 @@ def test_parse_headers3 _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) assert_equal(URI.parse("http://[fe80::208:dff:feef:98c7]:8080/path"), req.request_uri) assert_equal("[fe80::208:dff:feef:98c7]", req.host) @@ -231,7 +321,7 @@ def test_parse_get_params _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) query = req.query assert_equal("1", query["foo"]) assert_equal(["1", "2", "3"], query["foo"].to_ary) @@ -251,7 +341,7 @@ def test_parse_post_params #{param} _end_of_message_ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) query = req.query assert_equal("1", query["foo"]) assert_equal(["1", "2", "3"], query["foo"].to_ary) @@ -270,6 +360,7 @@ def test_chunked _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") File.open(__FILE__){|io| while chunk = io.read(100) msg << chunk.size.to_s(16) << crlf @@ -335,6 +426,7 @@ def test_forwarded _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server.example.com", req.server_name) @@ -355,6 +447,7 @@ def test_forwarded _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server.example.com", req.server_name) @@ -377,6 +470,7 @@ def test_forwarded _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server.example.com", req.server_name) @@ -399,6 +493,7 @@ def test_forwarded _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server1.example.com", req.server_name) @@ -421,6 +516,7 @@ def test_forwarded _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server1.example.com", req.server_name) @@ -443,6 +539,7 @@ def test_forwarded _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server1.example.com", req.server_name) @@ -460,6 +557,7 @@ def test_continue_sent _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert req['expect'] @@ -476,6 +574,7 @@ def test_continue_not_sent _end_of_message_ msg.gsub!(/^ {6}/, "") + msg.gsub!("\n", "\r\n") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert !req['expect'] @@ -495,7 +594,7 @@ def test_bad_messages _end_of_message_ assert_raise(WEBrick::HTTPStatus::LengthRequired){ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) req.body } @@ -508,7 +607,7 @@ def test_bad_messages _end_of_message_ assert_raise(WEBrick::HTTPStatus::BadRequest){ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) req.body } @@ -521,7 +620,7 @@ def test_bad_messages _end_of_message_ assert_raise(WEBrick::HTTPStatus::NotImplemented){ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) req.body } end From e72cb697836e2ff201a4a74c108fdca9d3d2d0ed Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 12 Jul 2024 02:45:43 +1200 Subject: [PATCH 22/30] Prefer squigly heredocs. (#143) --- test/webrick/test_filehandler.rb | 6 +- test/webrick/test_httprequest.rb | 207 ++++++++++++++----------------- 2 files changed, 97 insertions(+), 116 deletions(-) diff --git a/test/webrick/test_filehandler.rb b/test/webrick/test_filehandler.rb index 51654394..3b299d93 100644 --- a/test/webrick/test_filehandler.rb +++ b/test/webrick/test_filehandler.rb @@ -28,12 +28,12 @@ def get_res_body(res) end def make_range_request(range_spec) - msg = <<-END_OF_REQUEST + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.0 Range: #{range_spec} - END_OF_REQUEST - return StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n")) + HTTP + return StringIO.new(msg) end def make_range_response(file, range_spec) diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index fa181773..6088f18f 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -10,21 +10,21 @@ def teardown end def test_simple_request - msg = <<-_end_of_message_ -GET /\r - _end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") + GET / + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert(req.meta_vars) # fails if @header was not initialized and iteration is attempted on the nil reference end def test_parse_09 - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / foobar # HTTP/0.9 request don't have header nor entity body. - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal("GET", req.request_method) assert_equal("/", req.unparsed_uri) assert_equal(WEBrick::HTTPVersion.new("0.9"), req.http_version) @@ -36,12 +36,12 @@ def test_parse_09 end def test_parse_10 - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.0 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal("GET", req.request_method) assert_equal("/", req.unparsed_uri) assert_equal(WEBrick::HTTPVersion.new("1.0"), req.http_version) @@ -53,12 +53,12 @@ def test_parse_10 end def test_parse_11 - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal("GET", req.request_method) assert_equal("/path", req.unparsed_uri) assert_equal("", req.script_name) @@ -72,21 +72,21 @@ def test_parse_11 end def test_request_uri_too_large - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /#{"a"*2084} HTTP/1.1 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::RequestURITooLarge){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) } end def test_invalid_content_length_header ['', ' ', ' +1', ' -1', ' a'].each do |cl| - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1 Content-Length:#{cl} - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {8}/, "").gsub("\n", "\r\n"))) @@ -95,11 +95,11 @@ def test_invalid_content_length_header end def test_bare_lf_request_line - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1 Content-Length: 0\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) @@ -107,11 +107,11 @@ def test_bare_lf_request_line end def test_bare_lf_header - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r Content-Length: 0 \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) @@ -119,11 +119,11 @@ def test_bare_lf_header end def test_bare_cr_request_line - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r\r Content-Length: 0\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) @@ -131,11 +131,11 @@ def test_bare_cr_request_line end def test_bare_cr_header - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r Content-Type: foo\rbar\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) @@ -143,41 +143,41 @@ def test_bare_cr_header end def test_invalid_request_lines - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r Content-Length: 0\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) } - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r Content-Length: 0\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) } - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /\r HTTP/1.1\r Content-Length: 0\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) } - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1 \r Content-Length: 0\r \r - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) @@ -185,19 +185,19 @@ def test_invalid_request_lines end def test_duplicate_content_length_header - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1 Content-Length: 1 Content-Length: 2 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) } end def test_parse_headers - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 Host: test.ruby-lang.org:8080 Connection: close @@ -212,9 +212,9 @@ def test_parse_headers X-Empty-Header: foobar - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal( URI.parse("http://test.ruby-lang.org:8080/path"), req.request_uri) assert_equal("test.ruby-lang.org", req.host) @@ -234,7 +234,7 @@ def test_parse_headers end def test_parse_header2() - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /foo/bar/../baz?q=a HTTP/1.0 Content-Length: 10 User-Agent: @@ -242,9 +242,9 @@ def test_parse_header2() BAZ hogehoge - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal("POST", req.request_method) assert_equal("/foo/baz", req.path) assert_equal("", req.script_name) @@ -255,58 +255,58 @@ def test_parse_header2() end def test_parse_headers3 - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 Host: test.ruby-lang.org - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal(URI.parse("http://test.ruby-lang.org/path"), req.request_uri) assert_equal("test.ruby-lang.org", req.host) assert_equal(80, req.port) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 Host: 192.168.1.1 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal(URI.parse("http://192.168.1.1/path"), req.request_uri) assert_equal("192.168.1.1", req.host) assert_equal(80, req.port) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 Host: [fe80::208:dff:feef:98c7] - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal(URI.parse("http://[fe80::208:dff:feef:98c7]/path"), req.request_uri) assert_equal("[fe80::208:dff:feef:98c7]", req.host) assert_equal(80, req.port) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 Host: 192.168.1.1:8080 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal(URI.parse("http://192.168.1.1:8080/path"), req.request_uri) assert_equal("192.168.1.1", req.host) assert_equal(8080, req.port) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 Host: [fe80::208:dff:feef:98c7]:8080 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) assert_equal(URI.parse("http://[fe80::208:dff:feef:98c7]:8080/path"), req.request_uri) assert_equal("[fe80::208:dff:feef:98c7]", req.host) @@ -315,13 +315,13 @@ def test_parse_headers3 def test_parse_get_params param = "foo=1;foo=2;foo=3;bar=x" - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /path?#{param} HTTP/1.1 Host: test.ruby-lang.org:8080 - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) query = req.query assert_equal("1", query["foo"]) assert_equal(["1", "2", "3"], query["foo"].to_ary) @@ -332,16 +332,16 @@ def test_parse_get_params def test_parse_post_params param = "foo=1;foo=2;foo=3;bar=x" - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path?foo=x;foo=y;foo=z;bar=1 HTTP/1.1 Host: test.ruby-lang.org:8080 Content-Length: #{param.size} Content-Type: application/x-www-form-urlencoded #{param} - _end_of_message_ + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) query = req.query assert_equal("1", query["foo"]) assert_equal(["1", "2", "3"], query["foo"].to_ary) @@ -353,14 +353,12 @@ def test_parse_post_params def test_chunked crlf = "\x0d\x0a" expect = File.binread(__FILE__).freeze - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path HTTP/1.1 Host: test.ruby-lang.org:8080 Transfer-Encoding: chunked - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP File.open(__FILE__){|io| while chunk = io.read(100) msg << chunk.size.to_s(16) << crlf @@ -381,15 +379,14 @@ def test_chunked end def test_bad_chunked - msg = <<-_end_of_message_ + msg = <<~HTTP POST /path HTTP/1.1\r Transfer-Encoding: chunked\r \r 01x1\r \r 1 - _end_of_message_ - msg.gsub!(/^ {6}/, "") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.body } @@ -404,18 +401,18 @@ def test_bad_chunked end def test_null_byte_in_header - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path HTTP/1.1\r Evil: evil\x00\r \r - _end_of_message_ + HTTP msg.gsub!(/^ {6}/, "") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg)) } end def test_forwarded - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /foo HTTP/1.1 Host: localhost:10080 User-Agent: w3m/0.5.2 @@ -424,9 +421,7 @@ def test_forwarded X-Forwarded-Server: server.example.com Connection: Keep-Alive - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server.example.com", req.server_name) @@ -436,7 +431,7 @@ def test_forwarded assert_equal("123.123.123.123", req.remote_ip) assert(!req.ssl?) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /foo HTTP/1.1 Host: localhost:10080 User-Agent: w3m/0.5.2 @@ -445,9 +440,7 @@ def test_forwarded X-Forwarded-Server: server.example.com Connection: Keep-Alive - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server.example.com", req.server_name) @@ -457,7 +450,7 @@ def test_forwarded assert_equal("123.123.123.123", req.remote_ip) assert(!req.ssl?) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /foo HTTP/1.1 Host: localhost:10080 Client-IP: 234.234.234.234 @@ -468,9 +461,7 @@ def test_forwarded X-Requested-With: XMLHttpRequest Connection: Keep-Alive - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server.example.com", req.server_name) @@ -480,7 +471,7 @@ def test_forwarded assert_equal("234.234.234.234", req.remote_ip) assert(req.ssl?) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /foo HTTP/1.1 Host: localhost:10080 Client-IP: 234.234.234.234 @@ -491,9 +482,7 @@ def test_forwarded X-Requested-With: XMLHttpRequest Connection: Keep-Alive - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server1.example.com", req.server_name) @@ -503,7 +492,7 @@ def test_forwarded assert_equal("234.234.234.234", req.remote_ip) assert(req.ssl?) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /foo HTTP/1.1 Host: localhost:10080 Client-IP: 234.234.234.234 @@ -514,9 +503,7 @@ def test_forwarded X-Requested-With: XMLHttpRequest Connection: Keep-Alive - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server1.example.com", req.server_name) @@ -526,7 +513,7 @@ def test_forwarded assert_equal("234.234.234.234", req.remote_ip) assert(req.ssl?) - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") GET /foo HTTP/1.1 Host: localhost:10080 Client-IP: 234.234.234.234 @@ -537,9 +524,7 @@ def test_forwarded X-Requested-With: XMLHttpRequest Connection: Keep-Alive - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert_equal("server1.example.com", req.server_name) @@ -551,13 +536,11 @@ def test_forwarded end def test_continue_sent - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path HTTP/1.1 Expect: 100-continue - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert req['expect'] @@ -569,12 +552,10 @@ def test_continue_sent end def test_continue_not_sent - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path HTTP/1.1 - _end_of_message_ - msg.gsub!(/^ {6}/, "") - msg.gsub!("\n", "\r\n") + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new(msg)) assert !req['expect'] @@ -585,42 +566,42 @@ def test_continue_not_sent def test_bad_messages param = "foo=1;foo=2;foo=3;bar=x" - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path?foo=x;foo=y;foo=z;bar=1 HTTP/1.1 Host: test.ruby-lang.org:8080 Content-Type: application/x-www-form-urlencoded #{param} - _end_of_message_ + HTTP assert_raise(WEBrick::HTTPStatus::LengthRequired){ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) req.body } - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path?foo=x;foo=y;foo=z;bar=1 HTTP/1.1 Host: test.ruby-lang.org:8080 Content-Length: 100000 body is too short. - _end_of_message_ + HTTP assert_raise(WEBrick::HTTPStatus::BadRequest){ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) req.body } - msg = <<-_end_of_message_ + msg = <<~HTTP.gsub("\n", "\r\n") POST /path?foo=x;foo=y;foo=z;bar=1 HTTP/1.1 Host: test.ruby-lang.org:8080 Transfer-Encoding: foobar body is too short. - _end_of_message_ + HTTP assert_raise(WEBrick::HTTPStatus::NotImplemented){ req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) req.body } end From 426e214532bb0be5e4ab8b3c9cef328432012d0d Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 5 Jul 2024 10:36:09 -0700 Subject: [PATCH 23/30] Only strip space and horizontal tab in headers Previously, all whitespace was stripped, but that goes against the related RFCs. Fixes #139 --- lib/webrick/httputils.rb | 19 +++++++++++-------- test/webrick/test_httprequest.rb | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index ea67fdb0..92f3044d 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -178,12 +178,12 @@ def parse_header(raw) field.downcase! header[field] = HEADER_CLASSES[field].new unless header.has_key?(field) header[field] << value - when /^\s+([^\r\n\0]*?)\r\n/om + when /^[ \t]+([^\r\n\0]*?)\r\n/om unless field raise HTTPStatus::BadRequest, "bad header '#{line}'." end value = line - value.lstrip! + value.gsub!(/\A[ \t]+/, '') value.slice!(-2..-1) header[field][-1] << " " << value else @@ -191,7 +191,10 @@ def parse_header(raw) end } header.each{|key, values| - values.each(&:strip!) + values.each{|value| + value.gsub!(/\A[ \t]+/, '') + value.gsub!(/[ \t]+\z/, '') + } } header end @@ -202,7 +205,7 @@ def parse_header(raw) def split_header_value(str) str.scan(%r'\G((?:"(?:\\.|[^"])+?"|[^",]++)+) - (?:,\s*|\Z)'xn).flatten + (?:,[ \t]*|\Z)'xn).flatten end module_function :split_header_value @@ -230,9 +233,9 @@ def parse_range_header(ranges_specifier) def parse_qvalues(value) tmp = [] if value - parts = value.split(/,\s*/) + parts = value.split(/,[ \t]*/) parts.each {|part| - if m = %r{^([^\s,]+?)(?:;\s*q=(\d+(?:\.\d+)?))?$}.match(part) + if m = %r{^([^ \t,]+?)(?:;[ \t]*q=(\d+(?:\.\d+)?))?$}.match(part) val = m[1] q = (m[2] or 1).to_f tmp.push([val, q]) @@ -331,8 +334,8 @@ def <<(str) elsif str == CRLF @header = HTTPUtils::parse_header(@raw_header.join) if cd = self['content-disposition'] - if /\s+name="(.*?)"/ =~ cd then @name = $1 end - if /\s+filename="(.*?)"/ =~ cd then @filename = $1 end + if /[ \t]+name="(.*?)"/ =~ cd then @name = $1 end + if /[ \t]+filename="(.*?)"/ =~ cd then @filename = $1 end end else @raw_header << str diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 6088f18f..ad00d60d 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -118,6 +118,27 @@ def test_bare_lf_header } end + def test_header_vt_ff_whitespace + msg = <<~HTTP + GET / HTTP/1.1\r + Foo: \x0b1\x0c\r + \r + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + assert_equal("\x0b1\x0c", req["Foo"]) + + msg = <<~HTTP + GET / HTTP/1.1\r + Foo: \x0b1\x0c\r + \x0b2\x0c\r + \r + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + assert_equal("\x0b1\x0c \x0b2\x0c", req["Foo"]) + end + def test_bare_cr_request_line msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r\r From e4efb4a2300540f14f93c09c06bf0357ac1597dc Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Thu, 11 Jul 2024 15:02:21 -0700 Subject: [PATCH 24/30] Remove unnecessary gsub calls in test_httprequest.rb --- test/webrick/test_httprequest.rb | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index ad00d60d..84faefca 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -89,7 +89,7 @@ def test_invalid_content_length_header HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {8}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) } end end @@ -102,7 +102,7 @@ def test_bare_lf_request_line HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -114,7 +114,7 @@ def test_bare_lf_header HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -125,7 +125,7 @@ def test_header_vt_ff_whitespace \r HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) assert_equal("\x0b1\x0c", req["Foo"]) msg = <<~HTTP @@ -135,7 +135,7 @@ def test_header_vt_ff_whitespace \r HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) assert_equal("\x0b1\x0c \x0b2\x0c", req["Foo"]) end @@ -147,7 +147,7 @@ def test_bare_cr_request_line HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -159,7 +159,7 @@ def test_bare_cr_header HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -171,7 +171,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } msg = <<~HTTP.gsub("\n", "\r\n") @@ -181,7 +181,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } msg = <<~HTTP.gsub("\n", "\r\n") @@ -191,7 +191,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } msg = <<~HTTP.gsub("\n", "\r\n") @@ -201,7 +201,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -427,7 +427,6 @@ def test_null_byte_in_header Evil: evil\x00\r \r HTTP - msg.gsub!(/^ {6}/, "") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg)) } end From 2b38d5614e876d313fe981e87c4e35b91556d226 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 5 Jul 2024 11:03:21 -0700 Subject: [PATCH 25/30] Treat missing CRLF separator after headers as an EOFError Fix tests that did not have correctly formatted headers. This changes one test, with a request line that ends in bare LF instead of CRLF, from raising BadRequest to raising EOFError, but that seems reasonable. Fixes #140 --- lib/webrick/httprequest.rb | 10 +++++++++- test/webrick/test_httprequest.rb | 23 ++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 62ea54c8..f6d0b671 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -470,8 +470,13 @@ def read_request_line(socket) def read_header(socket) if socket + end_of_headers = false + while line = read_line(socket) - break if /\A#{CRLF}\z/om =~ line + if line == CRLF + end_of_headers = true + break + end if (@request_bytes += line.bytesize) > MAX_HEADER_LENGTH raise HTTPStatus::RequestEntityTooLarge, 'headers too large' end @@ -480,6 +485,9 @@ def read_header(socket) end @raw_header << line end + + # Allow if @header already set to support chunked trailers + raise HTTPStatus::EOFError unless end_of_headers || @header end @header = HTTPUtils::parse_header(@raw_header.join) diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 84faefca..122a7c1b 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -86,6 +86,7 @@ def test_invalid_content_length_header msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1 Content-Length:#{cl} + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ @@ -101,7 +102,7 @@ def test_bare_lf_request_line \r HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - assert_raise(WEBrick::HTTPStatus::BadRequest){ + assert_raise(WEBrick::HTTPStatus::EOFError){ req.parse(StringIO.new(msg)) } end @@ -210,6 +211,7 @@ def test_duplicate_content_length_header GET / HTTP/1.1 Content-Length: 1 Content-Length: 2 + HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ @@ -633,6 +635,25 @@ def test_eof_raised_when_line_is_nil } end + def test_eof_raised_with_missing_line_between_headers_and_body + msg = <<~HTTP.gsub("\n", "\r\n") + GET / HTTP/1.0 + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::EOFError) { + req.parse(StringIO.new(msg)) + } + + msg = <<~HTTP.gsub("\n", "\r\n") + GET / HTTP/1.0 + Foo: 1 + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + assert_raise(WEBrick::HTTPStatus::EOFError) { + req.parse(StringIO.new(msg)) + } + end + def test_cookie_join req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) req.parse(StringIO.new("GET / HTTP/1.1\r\ncookie: a=1\r\ncookie: b=2\r\n\r\n")) From 15a93914782789520837c334e0c302702aec34e2 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 21 Jun 2024 16:49:13 -0700 Subject: [PATCH 26/30] Return 400 response for chunked requests with unexpected data after chunk Fixes #133 --- lib/webrick/httprequest.rb | 6 +++++- test/webrick/test_httprequest.rb | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index f6d0b671..4e1de8cf 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -574,7 +574,11 @@ def read_chunked(socket, block) block.call(data) end while (chunk_size -= sz) > 0 - read_line(socket) # skip CRLF + line = read_line(socket) # skip CRLF + unless line == "\r\n" + raise HTTPStatus::BadRequest, "extra data after chunk `#{line}'." + end + chunk_size, = read_chunk_size(socket) end read_header(socket) # trailer + CRLF diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 122a7c1b..ea7e5a9d 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -423,6 +423,29 @@ def test_bad_chunked end end + def test_bad_chunked_extra_data + msg = <<~HTTP + POST /path HTTP/1.1\r + Transfer-Encoding: chunked\r + \r + 3\r + ABCthis-all-gets-ignored\r + 0\r + \r + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg)) + assert_raise(WEBrick::HTTPStatus::BadRequest){ req.body } + + # chunked req.body_reader + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg)) + dst = StringIO.new + assert_raise(WEBrick::HTTPStatus::BadRequest) do + IO.copy_stream(req.body_reader, dst) + end + end + def test_null_byte_in_header msg = <<~HTTP.gsub("\n", "\r\n") POST /path HTTP/1.1\r From 0c600e169bd4ae267cb5eeb6197277c848323bbe Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 23 Jul 2024 14:22:49 +0200 Subject: [PATCH 27/30] Fix reference to URI::REGEXP::PATTERN::HOST Recent changes in the URI gem make it so that this constant may not be there. And I think even with older versions of the gem, it would depend on what the default parser was set to. --- lib/webrick/httprequest.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 4e1de8cf..527099fa 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -522,9 +522,10 @@ def parse_uri(str, scheme="http") return URI::parse(uri.to_s) end + host_pattern = URI::RFC2396_Parser.new.pattern.fetch(:HOST) + HOST_PATTERN = /\A(#{host_pattern})(?::(\d+))?\z/n def parse_host_request_line(host) - pattern = /\A(#{URI::REGEXP::PATTERN::HOST})(?::(\d+))?\z/no - host.scan(pattern)[0] + host.scan(HOST_PATTERN)[0] end def read_body(socket, block) From f5faca9222541591e1a7c3c97552ebb0c92733c7 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 18 Sep 2024 14:11:49 -0700 Subject: [PATCH 28/30] Prevent request smuggling If a request has both a content-length and transfer-encoding headers, return a 400 response. This is allowed by RFC 7230 section 3.3.3.3. Fixes #145 --- lib/webrick/httprequest.rb | 4 ++++ test/webrick/test_httprequest.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 527099fa..0351a13e 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -531,6 +531,10 @@ def parse_host_request_line(host) def read_body(socket, block) return unless socket if tc = self['transfer-encoding'] + if self['content-length'] + raise HTTPStatus::BadRequest, "request with both transfer-encoding and content-length, possible request smuggling" + end + case tc when /\Achunked\z/io then read_chunked(socket, block) else raise HTTPStatus::NotImplemented, "Transfer-Encoding: #{tc}." diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index ea7e5a9d..d1283d4e 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -219,6 +219,24 @@ def test_duplicate_content_length_header } end + def test_content_length_and_transfer_encoding_headers_smuggling + msg = <<~HTTP.gsub("\n", "\r\n") + POST /user HTTP/1.1 + Content-Length: 28 + Transfer-Encoding: chunked + + 0 + + GET /admin HTTP/1.1 + + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg)) + assert_raise(WEBrick::HTTPStatus::BadRequest){ + req.body + } + end + def test_parse_headers msg = <<~HTTP.gsub("\n", "\r\n") GET /path HTTP/1.1 From b9a4c81ea94dec02a750c6b34092c55234519bf1 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 24 Sep 2024 11:26:39 +0900 Subject: [PATCH 29/30] Removed trailing spaces --- lib/webrick/httprequest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index 0351a13e..1fa5dbb4 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -531,7 +531,7 @@ def parse_host_request_line(host) def read_body(socket, block) return unless socket if tc = self['transfer-encoding'] - if self['content-length'] + if self['content-length'] raise HTTPStatus::BadRequest, "request with both transfer-encoding and content-length, possible request smuggling" end From 0fb9de6788a3ba5fe903e63d778a0fb8c1dce786 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 24 Sep 2024 11:30:49 +0900 Subject: [PATCH 30/30] Bump up v1.8.2 --- lib/webrick/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/webrick/version.rb b/lib/webrick/version.rb index ceeefc33..fbea0dda 100644 --- a/lib/webrick/version.rb +++ b/lib/webrick/version.rb @@ -14,5 +14,5 @@ module WEBrick ## # The WEBrick version - VERSION = "1.8.1" + VERSION = "1.8.2" end