From d6a89ac03f8e9628ab2e71d8fb56fdbdaec16569 Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Thu, 5 Mar 2009 21:20:17 -0800 Subject: [PATCH 1/3] Add Rack::Chunked (Transfer-Encoding: chunked) middleware --- lib/rack.rb | 1 + lib/rack/chunked.rb | 49 +++++++++++++++++++++++++++++++++++ test/spec_rack_chunked.rb | 62 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 0 deletions(-) create mode 100644 lib/rack/chunked.rb create mode 100644 test/spec_rack_chunked.rb diff --git a/lib/rack.rb b/lib/rack.rb index 2643630..987744f 100644 --- a/lib/rack.rb +++ b/lib/rack.rb @@ -28,6 +28,7 @@ module Rack autoload :Builder, "rack/builder" autoload :Cascade, "rack/cascade" + autoload :Chunked, "rack/chunked" autoload :CommonLogger, "rack/commonlogger" autoload :ConditionalGet, "rack/conditionalget" autoload :ContentLength, "rack/content_length" diff --git a/lib/rack/chunked.rb b/lib/rack/chunked.rb new file mode 100644 index 0000000..280d89d --- /dev/null +++ b/lib/rack/chunked.rb @@ -0,0 +1,49 @@ +require 'rack/utils' + +module Rack + + # Middleware that applies chunked transfer encoding to response bodies + # when the response does not include a Content-Length header. + class Chunked + include Rack::Utils + + def initialize(app) + @app = app + end + + def call(env) + status, headers, body = @app.call(env) + headers = HeaderHash.new(headers) + + if env['HTTP_VERSION'] == 'HTTP/1.0' || + STATUS_WITH_NO_ENTITY_BODY.include?(status) || + headers['Content-Length'] || + headers['Transfer-Encoding'] + [status, headers.to_hash, body] + else + dup.chunk(status, headers, body) + end + end + + def chunk(status, headers, body) + @body = body + headers.delete('Content-Length') + headers['Transfer-Encoding'] = 'chunked' + [status, headers.to_hash, self] + end + + def each + term = "\r\n" + @body.each do |chunk| + size = bytesize(chunk) + next if size == 0 + yield [size.to_s(16), term, chunk, term].join + end + yield ["0", term, "", term].join + end + + def close + @body.close if @body.respond_to?(:close) + end + end +end diff --git a/test/spec_rack_chunked.rb b/test/spec_rack_chunked.rb new file mode 100644 index 0000000..39eea48 --- /dev/null +++ b/test/spec_rack_chunked.rb @@ -0,0 +1,62 @@ +require 'rack/mock' +require 'rack/chunked' +require 'rack/utils' + +context "Rack::Chunked" do + + before do + @env = Rack::MockRequest. + env_for('/', 'HTTP_VERSION' => '1.1', 'REQUEST_METHOD' => 'GET') + end + + specify 'chunks responses with no Content-Length' do + app = lambda { |env| [200, {}, ['Hello', ' ', 'World!']] } + response = Rack::MockResponse.new(*Rack::Chunked.new(app).call(@env)) + response.headers.should.not.include 'Content-Length' + response.headers['Transfer-Encoding'].should.equal 'chunked' + response.body.should.equal "5\r\nHello\r\n1\r\n \r\n6\r\nWorld!\r\n0\r\n\r\n" + end + + specify 'chunks empty bodies properly' do + app = lambda { |env| [200, {}, []] } + response = Rack::MockResponse.new(*Rack::Chunked.new(app).call(@env)) + response.headers.should.not.include 'Content-Length' + response.headers['Transfer-Encoding'].should.equal 'chunked' + response.body.should.equal "0\r\n\r\n" + end + + specify 'does not modify response when Content-Length header present' do + app = lambda { |env| [200, {'Content-Length'=>'12'}, ['Hello', ' ', 'World!']] } + status, headers, body = Rack::Chunked.new(app).call(@env) + status.should.equal 200 + headers.should.not.include 'Transfer-Encoding' + headers.should.include 'Content-Length' + body.join.should.equal 'Hello World!' + end + + specify 'does not modify response when client is HTTP/1.0' do + app = lambda { |env| [200, {}, ['Hello', ' ', 'World!']] } + @env['HTTP_VERSION'] = 'HTTP/1.0' + status, headers, body = Rack::Chunked.new(app).call(@env) + status.should.equal 200 + headers.should.not.include 'Transfer-Encoding' + body.join.should.equal 'Hello World!' + end + + specify 'does not modify response when Transfer-Encoding header already present' do + app = lambda { |env| [200, {'Transfer-Encoding' => 'identity'}, ['Hello', ' ', 'World!']] } + status, headers, body = Rack::Chunked.new(app).call(@env) + status.should.equal 200 + headers['Transfer-Encoding'].should.equal 'identity' + body.join.should.equal 'Hello World!' + end + + [100, 204, 304].each do |status_code| + specify "does not modify response when status code is #{status_code}" do + app = lambda { |env| [status_code, {}, []] } + status, headers, body = Rack::Chunked.new(app).call(@env) + status.should.equal status_code + headers.should.not.include 'Transfer-Encoding' + end + end +end -- 1.6.2 From d848cc43ddcd949eec85da93a636aef32541619c Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Thu, 5 Mar 2009 21:20:58 -0800 Subject: [PATCH 2/3] Rack::Lint no longer requires a Content-Length response header --- lib/rack/lint.rb | 54 ++++++++++++++++------------------------------- test/spec_rack_lint.rb | 14 ------------ 2 files changed, 19 insertions(+), 49 deletions(-) diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index e549a9e..e77ee55 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -372,59 +372,43 @@ module Rack ## === The Content-Length def check_content_length(status, headers, env) - chunked_response = false - headers.each { |key, value| - if key.downcase == 'transfer-encoding' - chunked_response = value.downcase != 'identity' - end - } - headers.each { |key, value| if key.downcase == 'content-length' - ## There must be a Content-Length, except when the - ## +Status+ is 1xx, 204 or 304, in which case there must be none - ## given. + ## There must not be a Content-Length header when the + ## +Status+ is 1xx, 204 or 304. assert("Content-Length header found in #{status} response, not allowed") { not Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include? status.to_i } - assert('Content-Length header should not be used if body is chunked') { - not chunked_response - } - bytes = 0 string_body = true - @body.each { |part| - unless part.kind_of?(String) - string_body = false - break - end + if @body.respond_to?(:to_ary) + @body.each { |part| + unless part.kind_of?(String) + string_body = false + break + end - bytes += Rack::Utils.bytesize(part) - } - - if env["REQUEST_METHOD"] == "HEAD" - assert("Response body was given for HEAD request, but should be empty") { - bytes == 0 + bytes += Rack::Utils.bytesize(part) } - else - if string_body - assert("Content-Length header was #{value}, but should be #{bytes}") { - value == bytes.to_s + + if env["REQUEST_METHOD"] == "HEAD" + assert("Response body was given for HEAD request, but should be empty") { + bytes == 0 } + else + if string_body + assert("Content-Length header was #{value}, but should be #{bytes}") { + value == bytes.to_s + } + end end end return end } - - if [ String, Array ].include?(@body.class) && !chunked_response - assert('No Content-Length header found') { - Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include? status.to_i - } - end end ## === The Body diff --git a/test/spec_rack_lint.rb b/test/spec_rack_lint.rb index 2e78748..0b4d6fb 100644 --- a/test/spec_rack_lint.rb +++ b/test/spec_rack_lint.rb @@ -222,13 +222,6 @@ context "Rack::Lint" do end specify "notices content-length errors" do - lambda { - Rack::Lint.new(lambda { |env| - [200, {"Content-type" => "text/plain"}, []] - }).call(env({})) - }.should.raise(Rack::Lint::LintError). - message.should.match(/No Content-Length/) - [100, 101, 204, 304].each do |status| lambda { Rack::Lint.new(lambda { |env| @@ -240,13 +233,6 @@ context "Rack::Lint" do lambda { Rack::Lint.new(lambda { |env| - [200, {"Content-type" => "text/plain", "Content-Length" => "0", "Transfer-Encoding" => "chunked"}, []] - }).call(env({})) - }.should.raise(Rack::Lint::LintError). - message.should.match(/Content-Length header should not be used/) - - lambda { - Rack::Lint.new(lambda { |env| [200, {"Content-type" => "text/plain", "Content-Length" => "1"}, []] }).call(env({})) }.should.raise(Rack::Lint::LintError). -- 1.6.2 From 17feb4c52dd7807ee930645c2eb129b6886f66fa Mon Sep 17 00:00:00 2001 From: Ryan Tomayko Date: Thu, 5 Mar 2009 21:21:52 -0800 Subject: [PATCH 3/3] Handlers use ContentLength and Chunked middleware where needed Each Rack handler now automatically wraps the app in one or more pieces of middleware based on how the server is implemented. All handlers use the Rack::ContentLength middleware and some handlers use the Rack::Chunked middleware. Handlers that don't use the Chunked middleware either do not require CE for some reason or implement CE at the server level. * The Thin handler uses Chunked and ContentLength middleware. Thin has built-in CE support but also allows the app to apply it explicitly. Using our middleware for consistency and also because I believe the Thin folks want to remove CE support in the future. * The Mongrel handler uses Chunked and ContentLength middleware. Mongrel has built-in CE support but also allows the app to apply it explicitly. Using our middleware for consistency across handlers. * The SCGI handler uses Chunked and ContentLength middleware. * The WEBrick handler uses ContentLength middleware only; WEBrick has a really touchy CE implementation that doesn't like it when CE is applied by the application. * The FastCGI handler uses ContentLength only. FastCGI has its own mechanism for transferring bodies that do not include an explicit Content-Length. The FastCGI server applies the chunked encoding if needed. * The CGI handler uses ContentLength only. The CGI spec forbids the use of the Transfer-Encoding header in output from CGI programs. The server program is responsible for applying the chunked encoding if needed. Closing stdout signals the end of the body so it's not really needed anyway. * The LSWS handler uses ContentLength middleware only. Like CGI, LSWS closes the stream after transfer so chunked encoding is never required. --- lib/rack/handler/cgi.rb | 4 ++++ lib/rack/handler/fastcgi.rb | 3 +++ lib/rack/handler/lsws.rb | 5 ++++- lib/rack/handler/mongrel.rb | 4 +++- lib/rack/handler/scgi.rb | 4 +++- lib/rack/handler/thin.rb | 3 +++ lib/rack/handler/webrick.rb | 3 ++- 7 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/rack/handler/cgi.rb b/lib/rack/handler/cgi.rb index f2c976c..e38156c 100644 --- a/lib/rack/handler/cgi.rb +++ b/lib/rack/handler/cgi.rb @@ -1,3 +1,5 @@ +require 'rack/content_length' + module Rack module Handler class CGI @@ -6,6 +8,8 @@ module Rack end def self.serve(app) + app = ContentLength.new(app) + env = ENV.to_hash env.delete "HTTP_CONTENT_LENGTH" diff --git a/lib/rack/handler/fastcgi.rb b/lib/rack/handler/fastcgi.rb index f03e161..6324c7d 100644 --- a/lib/rack/handler/fastcgi.rb +++ b/lib/rack/handler/fastcgi.rb @@ -1,5 +1,6 @@ require 'fcgi' require 'socket' +require 'rack/content_length' module Rack module Handler @@ -29,6 +30,8 @@ module Rack end def self.serve(request, app) + app = Rack::ContentLength.new(app) + env = request.env env.delete "HTTP_CONTENT_LENGTH" diff --git a/lib/rack/handler/lsws.rb b/lib/rack/handler/lsws.rb index 1f850fc..93b5a16 100644 --- a/lib/rack/handler/lsws.rb +++ b/lib/rack/handler/lsws.rb @@ -1,5 +1,6 @@ require 'lsapi' -#require 'cgi' +require 'rack/content_length' + module Rack module Handler class LSWS @@ -9,6 +10,8 @@ module Rack end end def self.serve(app) + app = Rack::ContentLength.new(app) + env = ENV.to_hash env.delete "HTTP_CONTENT_LENGTH" env["SCRIPT_NAME"] = "" if env["SCRIPT_NAME"] == "/" diff --git a/lib/rack/handler/mongrel.rb b/lib/rack/handler/mongrel.rb index 178a1a8..f0c0d58 100644 --- a/lib/rack/handler/mongrel.rb +++ b/lib/rack/handler/mongrel.rb @@ -1,5 +1,7 @@ require 'mongrel' require 'stringio' +require 'rack/content_length' +require 'rack/chunked' module Rack module Handler @@ -33,7 +35,7 @@ module Rack end def initialize(app) - @app = app + @app = Rack::Chunked.new(Rack::ContentLength.new(app)) end def process(request, response) diff --git a/lib/rack/handler/scgi.rb b/lib/rack/handler/scgi.rb index fd18a83..9495c66 100644 --- a/lib/rack/handler/scgi.rb +++ b/lib/rack/handler/scgi.rb @@ -1,5 +1,7 @@ require 'scgi' require 'stringio' +require 'rack/content_length' +require 'rack/chunked' module Rack module Handler @@ -14,7 +16,7 @@ module Rack end def initialize(settings = {}) - @app = settings[:app] + @app = Rack::Chunked.new(Rack::ContentLength.new(settings[:app])) @log = Object.new def @log.info(*args); end def @log.error(*args); end diff --git a/lib/rack/handler/thin.rb b/lib/rack/handler/thin.rb index 7ad088b..3d4fedf 100644 --- a/lib/rack/handler/thin.rb +++ b/lib/rack/handler/thin.rb @@ -1,9 +1,12 @@ require "thin" +require "rack/content_length" +require "rack/chunked" module Rack module Handler class Thin def self.run(app, options={}) + app = Rack::Chunked.new(Rack::ContentLength.new(app)) server = ::Thin::Server.new(options[:Host] || '0.0.0.0', options[:Port] || 8080, app) diff --git a/lib/rack/handler/webrick.rb b/lib/rack/handler/webrick.rb index 40be79d..4d535ac 100644 --- a/lib/rack/handler/webrick.rb +++ b/lib/rack/handler/webrick.rb @@ -1,5 +1,6 @@ require 'webrick' require 'stringio' +require 'rack/content_length' module Rack module Handler @@ -14,7 +15,7 @@ module Rack def initialize(server, app) super server - @app = app + @app = Rack::ContentLength.new(app) end def service(req, res) -- 1.6.2