From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 8A61B203EA for ; Wed, 27 Jul 2016 00:00:02 +0000 (UTC) From: Eric Wong To: yahns-public@yhbt.net Subject: [PATCH] http_response: drop bodies for non-compliant responses Date: Wed, 27 Jul 2016 00:00:02 +0000 Message-Id: <20160727000002.26438-1-e@80x24.org> List-Id: Rack::Lint-compliant applications wouldn't have this problem; but apparently public-facing Rack servers (webrick/puma/thin) all implement this; so there is precedence for implementing this in yahns itself. --- lib/yahns/http_client.rb | 9 ++++++--- lib/yahns/http_response.rb | 12 +++++------- test/test_response.rb | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/lib/yahns/http_client.rb b/lib/yahns/http_client.rb index 1cdaa0f..873dd73 100644 --- a/lib/yahns/http_client.rb +++ b/lib/yahns/http_client.rb @@ -189,9 +189,11 @@ def r100_done mkinput_preread # keep looping (@state == :body) true else # :lazy, false - status, headers, body = k.app.call(env = @hs.env) + env = @hs.env + hdr_only = env['REQUEST_METHOD'] == 'HEAD'.freeze + status, headers, body = k.app.call(env) return :ignore if app_hijacked?(env, body) - http_response_write(status, headers, body) + http_response_write(status, headers, body, hdr_only) end end @@ -220,6 +222,7 @@ def app_call(input) env['SERVER_PORT'] = '443'.freeze end + hdr_only = env['REQUEST_METHOD'] == 'HEAD'.freeze # run the rack app status, headers, body = k.app.call(env) return :ignore if app_hijacked?(env, body) @@ -229,7 +232,7 @@ def app_call(input) end # this returns :wait_readable, :wait_writable, :ignore, or nil: - http_response_write(status, headers, body) + http_response_write(status, headers, body, hdr_only) end # called automatically by kgio_write diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb index d957df6..88ff9f9 100644 --- a/lib/yahns/http_response.rb +++ b/lib/yahns/http_response.rb @@ -9,7 +9,7 @@ # You use it by simply doing: # # status, headers, body = rack_app.call(env) -# http_response_write(status, headers, body) +# http_response_write(status, headers, body, env['REQUEST_METHOD']=='HEAD') # # Most header correctness (including Content-Length and Content-Type) # is the job of Rack, with the exception of the "Date" header. @@ -118,13 +118,9 @@ def kv_str(buf, key, value) end end - def have_more?(value) - value.to_i > 0 && @hs.env['REQUEST_METHOD'] != 'HEAD'.freeze - end - # writes the rack_response to socket as an HTTP response # returns :wait_readable, :wait_writable, :forget, or nil - def http_response_write(status, headers, body) + def http_response_write(status, headers, body, hdr_only) offset = 0 count = hijack = nil k = self.class @@ -133,6 +129,7 @@ def http_response_write(status, headers, body) if @hs.headers? code = status.to_i + hdr_only ||= Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include?(code) msg = Rack::Utils::HTTP_STATUS_CODES[code] buf = "#{response_start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \ "Date: #{httpdate}\r\n".dup @@ -150,7 +147,7 @@ def http_response_write(status, headers, body) # allow Rack apps to tell us they want to drop the client alive = false if value =~ /\bclose\b/i when %r{\AContent-Length\z}i - flags |= MSG_MORE if have_more?(value) + flags |= MSG_MORE if value.to_i > 0 && !hdr_only kv_str(buf, key, value) when "rack.hijack" hijack = value @@ -181,6 +178,7 @@ def http_response_write(status, headers, body) end return response_hijacked(hijack) if hijack + return http_response_done(alive) if hdr_only if body.respond_to?(:to_path) @state = body = Yahns::StreamFile.new(body, alive, offset, count) diff --git a/test/test_response.rb b/test/test_response.rb index a71fd65..34a4c90 100644 --- a/test/test_response.rb +++ b/test/test_response.rb @@ -9,6 +9,48 @@ class TestResponse < Testcase alias setup server_helper_setup alias teardown server_helper_teardown + def test_auto_head + err = @err + cfg = Yahns::Config.new + host, port = @srv.addr[3], @srv.addr[1] + str = "HELLO WORLD\n" + cfg.instance_eval do + GTL.synchronize do + app = Rack::Builder.new do + use Rack::ContentLength + use Rack::ContentType, "text/plain" + run(lambda do |env| + case env['PATH_INFO'] + when '/'; return [ 200, {}, [ str ] ] + when '/304'; return [ 304, {}, [ str ] ] + else + abort 'unsupported' + end + end) + end + app(:rack, app) { listen "#{host}:#{port}" } + end + logger(Logger.new(err.path)) + end + pid = mkserver(cfg) + s = TCPSocket.new(host, port) + s.write("HEAD / HTTP/1.0\r\n\r\n") + assert s.wait(30), "IO wait failed" + buf = s.read + assert_match %r{\r\n\r\n\z}, buf + s.close + + s = TCPSocket.new(host, port) + s.write("GET /304 HTTP/1.0\r\n\r\n") + assert s.wait(30), "IO wait failed" + buf = s.read + assert_match %r{\r\n\r\n\z}, buf + assert_match %r{\b304\b}, buf + s.close + ensure + quit_wait(pid) + end + def test_response_time_empty_body err = @err cfg = Yahns::Config.new -- EW