From 8e52002c3259223072b54bd040ff2f6a12b4d357 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Mon, 9 Dec 2013 21:09:14 +0100 Subject: Adds improved Rack::Deflater tests. --- test/spec_deflater.rb | 372 +++++++++++++++++++++++++++++--------------------- 1 file changed, 213 insertions(+), 159 deletions(-) diff --git a/test/spec_deflater.rb b/test/spec_deflater.rb index 6f5137ca..1dd57767 100644 --- a/test/spec_deflater.rb +++ b/test/spec_deflater.rb @@ -6,199 +6,253 @@ require 'rack/mock' require 'zlib' describe Rack::Deflater do - def deflater(app) - Rack::Lint.new Rack::Deflater.new(app) - end - def build_response(status, body, accept_encoding, headers = {}) - body = [body] if body.respond_to? :to_str + def build_response(status, body, accept_encoding, options = {}) + body = [body] if body.respond_to? :to_str app = lambda do |env| - res = [status, {}, body] - res[1]["Content-Type"] = "text/plain" unless res[0] == 304 + res = [status, options['response_headers'] || {}, body] + res[1]['Content-Type'] = 'text/plain' unless res[0] == 304 res end - request = Rack::MockRequest.env_for("", headers.merge("HTTP_ACCEPT_ENCODING" => accept_encoding)) - response = deflater(app).call(request) - return response - end + request = Rack::MockRequest.env_for('', (options['request_headers'] || {}).merge('HTTP_ACCEPT_ENCODING' => accept_encoding)) + deflater = Rack::Lint.new Rack::Deflater.new(app) - def inflate(buf) - inflater = Zlib::Inflate.new(-Zlib::MAX_WBITS) - inflater.inflate(buf) << inflater.finish + deflater.call(request) end - should "be able to deflate bodies that respond to each" do - body = Object.new - class << body; def each; yield("foo"); yield("bar"); end; end - - response = build_response(200, body, "deflate") - - response[0].should.equal(200) - response[1].should.equal({ - "Content-Encoding" => "deflate", - "Vary" => "Accept-Encoding", - "Content-Type" => "text/plain" - }) - buf = '' - response[2].each { |part| buf << part } - inflate(buf).should.equal("foobar") - end + ## + # Constructs response object and verifies if it yields right results + # + # [expected_status] expected response status, e.g. 200, 304 + # [expected_body] expected response body + # [accept_encoing] what Accept-Encoding header to send and expect, e.g. + # 'deflate' - accepts and expects deflate encoding in response + # { 'gzip' => nil } - accepts gzip but expects no encoding in response + # [options] hash of request options, i.e. + # 'app_status' - what status dummy app should return (may be changed by deflater at some point) + # 'app_body' - what body dummy app should return (may be changed by deflater at some point) + # 'request_headers' - extra reqest headers to be sent + # 'response_headers' - extra response headers to be returned + # [block] useful for doing some extra verification + def verify(expected_status, expected_body, accept_encoding, options = {}, &block) + accept_encoding, expected_encoding = if accept_encoding.kind_of?(Hash) + [accept_encoding.keys.first, accept_encoding.values.first] + else + [accept_encoding, accept_encoding.dup] + end - should "flush deflated chunks to the client as they become ready" do - body = Object.new - class << body; def each; yield("foo"); yield("bar"); end; end - - response = build_response(200, body, "deflate") - - response[0].should.equal(200) - response[1].should.equal({ - "Content-Encoding" => "deflate", - "Vary" => "Accept-Encoding", - "Content-Type" => "text/plain" - }) - buf = [] - inflater = Zlib::Inflate.new(-Zlib::MAX_WBITS) - response[2].each { |part| buf << inflater.inflate(part) } - buf << inflater.finish - buf.delete_if { |part| part.empty? } - buf.join.should.equal("foobar") + # build response + status, headers, body = build_response( + options['app_status'] || expected_status, + options['app_body'] || expected_body, + accept_encoding, + options + ) + + # verify status + status.should.equal(expected_status) + + # verify body + unless options['skip_body_verify'] + body_text = '' + body.each { |part| body_text << part } + + deflated_body = case expected_encoding + when 'deflate' + inflater = Zlib::Inflate.new(-Zlib::MAX_WBITS) + inflater.inflate(body_text) << inflater.finish + when 'gzip' + io = StringIO.new(body_text) + gz = Zlib::GzipReader.new(io) + tmp = gz.read + gz.close + tmp + else + body_text + end + + deflated_body.should.equal(expected_body) + end + + # yield full response verification + yield(status, headers, body) if block_given? end - # TODO: This is really just a special case of the above... - should "be able to deflate String bodies" do - response = build_response(200, "Hello world!", "deflate") - - response[0].should.equal(200) - response[1].should.equal({ - "Content-Encoding" => "deflate", - "Vary" => "Accept-Encoding", - "Content-Type" => "text/plain" - }) - buf = '' - response[2].each { |part| buf << part } - inflate(buf).should.equal("Hello world!") + should 'be able to deflate bodies that respond to each' do + app_body = Object.new + class << app_body; def each; yield('foo'); yield('bar'); end; end + + verify(200, 'foobar', 'deflate', { 'app_body' => app_body }) do |status, headers, body| + headers.should.equal({ + 'Content-Encoding' => 'deflate', + 'Vary' => 'Accept-Encoding', + 'Content-Type' => 'text/plain' + }) + end end - should "be able to gzip bodies that respond to each" do - body = Object.new - class << body; def each; yield("foo"); yield("bar"); end; end - - response = build_response(200, body, "gzip") - - response[0].should.equal(200) - response[1].should.equal({ - "Content-Encoding" => "gzip", - "Vary" => "Accept-Encoding", - "Content-Type" => "text/plain" - }) - - buf = '' - response[2].each { |part| buf << part } - io = StringIO.new(buf) - gz = Zlib::GzipReader.new(io) - gz.read.should.equal("foobar") - gz.close + should 'flush deflated chunks to the client as they become ready' do + app_body = Object.new + class << app_body; def each; yield('foo'); yield('bar'); end; end + + verify(200, app_body, 'deflate', { 'skip_body_verify' => true }) do |status, headers, body| + headers.should.equal({ + 'Content-Encoding' => 'deflate', + 'Vary' => 'Accept-Encoding', + 'Content-Type' => 'text/plain' + }) + + buf = [] + inflater = Zlib::Inflate.new(-Zlib::MAX_WBITS) + body.each { |part| buf << inflater.inflate(part) } + buf << inflater.finish + + buf.delete_if { |part| part.empty? }.join.should.equal('foobar') + end end - should "flush gzipped chunks to the client as they become ready" do - body = Object.new - class << body; def each; yield("foo"); yield("bar"); end; end - - response = build_response(200, body, "gzip") - - response[0].should.equal(200) - response[1].should.equal({ - "Content-Encoding" => "gzip", - "Vary" => "Accept-Encoding", - "Content-Type" => "text/plain" - }) - buf = [] - inflater = Zlib::Inflate.new(Zlib::MAX_WBITS + 32) - response[2].each { |part| buf << inflater.inflate(part) } - buf << inflater.finish - buf.delete_if { |part| part.empty? } - buf.join.should.equal("foobar") + # TODO: This is really just a special case of the above... + should 'be able to deflate String bodies' do + verify(200, 'Hello world!', 'deflate') do |status, headers, body| + headers.should.equal({ + 'Content-Encoding' => 'deflate', + 'Vary' => 'Accept-Encoding', + 'Content-Type' => 'text/plain' + }) + end end - should "be able to fallback to no deflation" do - response = build_response(200, "Hello world!", "superzip") + should 'be able to gzip bodies that respond to each' do + app_body = Object.new + class << app_body; def each; yield('foo'); yield('bar'); end; end - response[0].should.equal(200) - response[1].should.equal({ "Vary" => "Accept-Encoding", "Content-Type" => "text/plain" }) - response[2].to_enum.to_a.should.equal(["Hello world!"]) + verify(200, 'foobar', 'gzip', { 'app_body' => app_body }) do |status, headers, body| + headers.should.equal({ + 'Content-Encoding' => 'gzip', + 'Vary' => 'Accept-Encoding', + 'Content-Type' => 'text/plain' + }) + end end - should "be able to skip when there is no response entity body" do - response = build_response(304, [], "gzip") + should 'flush gzipped chunks to the client as they become ready' do + app_body = Object.new + class << app_body; def each; yield('foo'); yield('bar'); end; end - response[0].should.equal(304) - response[1].should.equal({}) - response[2].to_enum.to_a.should.equal([]) - end + verify(200, app_body, 'gzip', { 'skip_body_verify' => true }) do |status, headers, body| + headers.should.equal({ + 'Content-Encoding' => 'gzip', + 'Vary' => 'Accept-Encoding', + 'Content-Type' => 'text/plain' + }) - should "handle the lack of an acceptable encoding" do - response1 = build_response(200, "Hello world!", "identity;q=0", "PATH_INFO" => "/") - response1[0].should.equal(406) - response1[1].should.equal({"Content-Type" => "text/plain", "Content-Length" => "71"}) - response1[2].to_enum.to_a.should.equal(["An acceptable encoding for the requested resource / could not be found."]) + buf = [] + inflater = Zlib::Inflate.new(Zlib::MAX_WBITS + 32) + body.each { |part| buf << inflater.inflate(part) } + buf << inflater.finish - response2 = build_response(200, "Hello world!", "identity;q=0", "SCRIPT_NAME" => "/foo", "PATH_INFO" => "/bar") - response2[0].should.equal(406) - response2[1].should.equal({"Content-Type" => "text/plain", "Content-Length" => "78"}) - response2[2].to_enum.to_a.should.equal(["An acceptable encoding for the requested resource /foo/bar could not be found."]) + buf.delete_if { |part| part.empty? }.join.should.equal('foobar') + end end - should "handle gzip response with Last-Modified header" do - last_modified = Time.now.httpdate + should 'be able to fallback to no deflation' do + verify(200, 'Hello world!', 'superzip') do |status, headers, body| + headers.should.equal({ + 'Vary' => 'Accept-Encoding', + 'Content-Type' => 'text/plain' + }) + end + end - app = lambda { |env| [200, { "Content-Type" => "text/plain", "Last-Modified" => last_modified }, ["Hello World!"]] } - request = Rack::MockRequest.env_for("", "HTTP_ACCEPT_ENCODING" => "gzip") - response = deflater(app).call(request) - - response[0].should.equal(200) - response[1].should.equal({ - "Content-Encoding" => "gzip", - "Vary" => "Accept-Encoding", - "Last-Modified" => last_modified, - "Content-Type" => "text/plain" - }) - - buf = '' - response[2].each { |part| buf << part } - io = StringIO.new(buf) - gz = Zlib::GzipReader.new(io) - gz.read.should.equal("Hello World!") - gz.close + should 'be able to skip when there is no response entity body' do + verify(304, '', { 'gzip' => nil }, { 'app_body' => [] }) do |status, headers, body| + headers.should.equal({}) + end end - should "do nothing when no-transform Cache-Control directive present" do - app = lambda { |env| [200, {'Content-Type' => 'text/plain', 'Cache-Control' => 'no-transform'}, ['Hello World!']] } - request = Rack::MockRequest.env_for("", "HTTP_ACCEPT_ENCODING" => "gzip") - response = deflater(app).call(request) + should 'handle the lack of an acceptable encoding' do + app_body = 'Hello world!' + not_found_body1 = 'An acceptable encoding for the requested resource / could not be found.' + not_found_body2 = 'An acceptable encoding for the requested resource /foo/bar could not be found.' + options1 = { + 'app_status' => 200, + 'app_body' => app_body, + 'request_headers' => { + 'PATH_INFO' => '/' + } + } + options2 = { + 'app_status' => 200, + 'app_body' => app_body, + 'request_headers' => { + 'PATH_INFO' => '/foo/bar' + } + } + + verify(406, not_found_body1, 'identity;q=0', options1) do |status, headers, body| + headers.should.equal({ + 'Content-Type' => 'text/plain', + 'Content-Length' => not_found_body1.length.to_s + }) + end - response[0].should.equal(200) - response[1].should.not.include "Content-Encoding" - response[2].to_enum.to_a.join.should.equal("Hello World!") + verify(406, not_found_body2, 'identity;q=0', options2) do |status, headers, body| + headers.should.equal({ + 'Content-Type' => 'text/plain', + 'Content-Length' => not_found_body2.length.to_s + }) + end end - should "do nothing when Content-Encoding already present" do - app = lambda { |env| [200, {'Content-Type' => 'text/plain', 'Content-Encoding' => 'gzip'}, ['Hello World!']] } - request = Rack::MockRequest.env_for("", "HTTP_ACCEPT_ENCODING" => "gzip") - response = deflater(app).call(request) + should 'handle gzip response with Last-Modified header' do + last_modified = Time.now.httpdate + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain', + 'Last-Modified' => last_modified + } + } + + verify(200, 'Hello World!', 'gzip', options) do |status, headers, body| + headers.should.equal({ + 'Content-Encoding' => 'gzip', + 'Vary' => 'Accept-Encoding', + 'Last-Modified' => last_modified, + 'Content-Type' => 'text/plain' + }) + end + end - response[0].should.equal(200) - response[2].to_enum.to_a.join.should.equal("Hello World!") + should 'do nothing when no-transform Cache-Control directive present' do + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain', + 'Cache-Control' => 'no-transform' + } + } + verify(200, 'Hello World!', { 'gzip' => nil }, options) do |status, headers, body| + headers.should.not.include 'Content-Encoding' + end end - should "deflate when Content-Encoding is identity" do - app = lambda { |env| [200, {'Content-Type' => 'text/plain', 'Content-Encoding' => 'identity'}, ['Hello World!']] } - request = Rack::MockRequest.env_for("", "HTTP_ACCEPT_ENCODING" => "deflate") - response = deflater(app).call(request) + should 'do nothing when Content-Encoding already present' do + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain', + 'Content-Encoding' => 'gzip' + } + } + verify(200, 'Hello World!', { 'gzip' => nil }, options) + end - response[0].should.equal(200) - buf = '' - response[2].each { |part| buf << part } - inflate(buf).should.equal("Hello World!") + should 'deflate when Content-Encoding is identity' do + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain', + 'Content-Encoding' => 'identity' + } + } + verify(200, 'Hello World!', 'deflate', options) end end -- cgit v1.2.3-24-ge0c7 From 3ae2f3031b8925c641813734a5fa04c9bdafb137 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Mon, 9 Dec 2013 21:15:02 +0100 Subject: Adds deflater options to control compression on per-request level. * Adds :if option which should be given a lambda accepting env, status, headers, and body options. * When :if evaluates to false a response body won't be compressed. * Adds :include option which should be given an array of compressible content types. * When :include don't include request's content type then response body won't be compressed. --- lib/rack/deflater.rb | 39 ++++++++++++++++++++---- test/spec_deflater.rb | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/lib/rack/deflater.rb b/lib/rack/deflater.rb index fe2ac3db..638bf049 100644 --- a/lib/rack/deflater.rb +++ b/lib/rack/deflater.rb @@ -17,19 +17,26 @@ module Rack # directive of 'no-transform' is present, or when the response status # code is one that doesn't allow an entity body. class Deflater - def initialize(app) + ## + # Creates Rack::Deflater middleware. + # + # [app] rack app instance + # [options] hash of deflater options, i.e. + # 'if' - a lambda enabling / disabling deflation based on returned boolean value + # e.g use Rack::Deflater, :if => lambda { |env, status, headers, body| body.length > 512 } + # 'include' - a list of content types that should be compressed + def initialize(app, options = {}) @app = app + + @condition = options[:if] + @compressible_types = options[:include] end def call(env) status, headers, body = @app.call(env) headers = Utils::HeaderHash.new(headers) - # Skip compressing empty entity body responses and responses with - # no-transform set. - if Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status) || - headers['Cache-Control'].to_s =~ /\bno-transform\b/ || - (headers['Content-Encoding'] && headers['Content-Encoding'] !~ /\bidentity\b/) + unless should_deflate?(env, status, headers, body) return [status, headers, body] end @@ -126,5 +133,25 @@ module Rack @body.close if @body.respond_to?(:close) end end + + private + + def should_deflate?(env, status, headers, body) + # Skip compressing empty entity body responses and responses with + # no-transform set. + if Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status) || + headers['Cache-Control'].to_s =~ /\bno-transform\b/ || + (headers['Content-Encoding'] && headers['Content-Encoding'] !~ /\bidentity\b/) + return false + end + + # Skip if @compressible_types are given and does not include request's content type + return false if @compressible_types && !(headers.has_key?('Content-Type') && @compressible_types.include?(headers['Content-Type'][/[^;]*/])) + + # Skip if @condition lambda is given and evaluates to false + return false if @condition && !@condition.call(env, status, headers, body) + + true + end end end diff --git a/test/spec_deflater.rb b/test/spec_deflater.rb index 1dd57767..1e921eff 100644 --- a/test/spec_deflater.rb +++ b/test/spec_deflater.rb @@ -16,7 +16,7 @@ describe Rack::Deflater do end request = Rack::MockRequest.env_for('', (options['request_headers'] || {}).merge('HTTP_ACCEPT_ENCODING' => accept_encoding)) - deflater = Rack::Lint.new Rack::Deflater.new(app) + deflater = Rack::Lint.new Rack::Deflater.new(app, options['deflater_options'] || {}) deflater.call(request) end @@ -34,6 +34,7 @@ describe Rack::Deflater do # 'app_body' - what body dummy app should return (may be changed by deflater at some point) # 'request_headers' - extra reqest headers to be sent # 'response_headers' - extra response headers to be returned + # 'deflater_options' - options passed to deflater middleware # [block] useful for doing some extra verification def verify(expected_status, expected_body, accept_encoding, options = {}, &block) accept_encoding, expected_encoding = if accept_encoding.kind_of?(Hash) @@ -255,4 +256,84 @@ describe Rack::Deflater do } verify(200, 'Hello World!', 'deflate', options) end + + should "deflate if content-type matches :include" do + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain' + }, + 'deflater_options' => { + :include => %w(text/plain) + } + } + verify(200, 'Hello World!', 'gzip', options) + end + + should "deflate if content-type is included it :include" do + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain; charset=us-ascii' + }, + 'deflater_options' => { + :include => %w(text/plain) + } + } + verify(200, 'Hello World!', 'gzip', options) + end + + should "not deflate if content-type is not set but given in :include" do + options = { + 'deflater_options' => { + :include => %w(text/plain) + } + } + verify(304, 'Hello World!', { 'gzip' => nil }, options) + end + + should "not deflate if content-type do not match :include" do + options = { + 'response_headers' => { + 'Content-Type' => 'text/plain' + }, + 'deflater_options' => { + :include => %w(text/json) + } + } + verify(200, 'Hello World!', { 'gzip' => nil }, options) + end + + should "deflate response if :if lambda evaluates to true" do + options = { + 'deflater_options' => { + :if => lambda { |env, status, headers, body| true } + } + } + verify(200, 'Hello World!', 'deflate', options) + end + + should "not deflate if :if lambda evaluates to false" do + options = { + 'deflater_options' => { + :if => lambda { |env, status, headers, body| false } + } + } + verify(200, 'Hello World!', { 'gzip' => nil }, options) + end + + should "check for Content-Length via :if" do + body = 'Hello World!' + body_len = body.length + options = { + 'response_headers' => { + 'Content-Length' => body_len.to_s + }, + 'deflater_options' => { + :if => lambda { |env, status, headers, body| + headers['Content-Length'].to_i >= body_len + } + } + } + + verify(200, body, 'gzip', options) + end end -- cgit v1.2.3-24-ge0c7 From adc2169e1e38e13b02f440c0124a4a734f5c2f64 Mon Sep 17 00:00:00 2001 From: KITAITI Makoto Date: Sat, 18 Jan 2014 21:39:16 +0900 Subject: Close body if content is fresh enough --- lib/rack/conditionalget.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/rack/conditionalget.rb b/lib/rack/conditionalget.rb index ed87c54e..2c2113f2 100644 --- a/lib/rack/conditionalget.rb +++ b/lib/rack/conditionalget.rb @@ -25,6 +25,7 @@ module Rack status, headers, body = @app.call(env) headers = Utils::HeaderHash.new(headers) if status == 200 && fresh?(env, headers) + body.close if body.respond_to? :close status = 304 headers.delete('Content-Type') headers.delete('Content-Length') -- cgit v1.2.3-24-ge0c7 From 05c96e7ef84248de1729f344e518dd9e5403564a Mon Sep 17 00:00:00 2001 From: Artem Pyanykh Date: Mon, 20 Jan 2014 12:15:31 +0400 Subject: Reformat Rack::Lint docs. Limit lines to 80 chars. --- SPEC | 78 +++++++++++++++++++++++++++++++++------------- lib/rack/lint.rb | 94 ++++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 128 insertions(+), 44 deletions(-) diff --git a/SPEC b/SPEC index f7bfb3df..0deb57b5 100644 --- a/SPEC +++ b/SPEC @@ -40,7 +40,17 @@ below. QUERY_STRING:: The portion of the request URL that follows the ?, if any. May be empty, but is always required! -SERVER_NAME, SERVER_PORT:: When combined with SCRIPT_NAME and PATH_INFO, these variables can be used to complete the URL. Note, however, that HTTP_HOST, if present, should be used in preference to SERVER_NAME for reconstructing the request URL. SERVER_NAME and SERVER_PORT can never be empty strings, and so are always required. +SERVER_NAME, SERVER_PORT:: + When combined with SCRIPT_NAME and + PATH_INFO, these variables can be + used to complete the URL. Note, however, + that HTTP_HOST, if present, + should be used in preference to + SERVER_NAME for reconstructing + the request URL. + SERVER_NAME and SERVER_PORT + can never be empty strings, and so + are always required. HTTP_ Variables:: Variables corresponding to the client-supplied HTTP request headers (i.e., variables whose @@ -49,24 +59,47 @@ below. variables should correspond with the presence or absence of the appropriate HTTP header in the - request. See - RFC3875 section 4.1.18 for specific behavior. + request. See + + RFC3875 section 4.1.18 for + specific behavior. In addition to this, the Rack environment must include these Rack-specific variables: -rack.version:: The Array representing this version of Rack. See Rack::VERSION, that corresponds to the version of this SPEC. -rack.url_scheme:: +http+ or +https+, depending on the request URL. +rack.version:: The Array representing this version of Rack + See Rack::VERSION, that corresponds to + the version of this SPEC. +rack.url_scheme:: +http+ or +https+, depending on the + request URL. rack.input:: See below, the input stream. rack.errors:: See below, the error stream. -rack.multithread:: true if the application object may be simultaneously invoked by another thread in the same process, false otherwise. -rack.multiprocess:: true if an equivalent application object may be simultaneously invoked by another process, false otherwise. -rack.run_once:: true if the server expects (but does not guarantee!) that the application will only be invoked this one time during the life of its containing process. Normally, this will only be true for a server based on CGI (or something similar). -rack.hijack?:: present and true if the server supports connection hijacking. See below, hijacking. -rack.hijack:: an object responding to #call that must be called at least once before using rack.hijack_io. It is recommended #call return rack.hijack_io as well as setting it in env if necessary. -rack.hijack_io:: if rack.hijack? is true, and rack.hijack has received #call, this will contain an object resembling an IO. See hijacking. +rack.multithread:: true if the application object may be + simultaneously invoked by another thread + in the same process, false otherwise. +rack.multiprocess:: true if an equivalent application object + may be simultaneously invoked by another + process, false otherwise. +rack.run_once:: true if the server expects + (but does not guarantee!) that the + application will only be invoked this one + time during the life of its containing + process. Normally, this will only be true + for a server based on CGI + (or something similar). +rack.hijack?:: present and true if the server supports + connection hijacking. See below, hijacking. +rack.hijack:: an object responding to #call that must be + called at least once before using + rack.hijack_io. + It is recommended #call return rack.hijack_io + as well as setting it in env if necessary. +rack.hijack_io:: if rack.hijack? is true, and rack.hijack + has received #call, this will contain + an object resembling an IO. See hijacking. Additional environment specifications have approved to standardized middleware APIs. None of these are required to be implemented by the server. -rack.session:: A hash like interface for storing request session data. +rack.session:: A hash like interface for storing + request session data. The store must implement: store(key, value) (aliased as []=); fetch(key, default = nil) (aliased as []); @@ -110,15 +143,18 @@ must be opened in binary mode, for Ruby 1.9 compatibility. The input stream must respond to +gets+, +each+, +read+ and +rewind+. * +gets+ must be called without arguments and return a string, or +nil+ on EOF. -* +read+ behaves like IO#read. Its signature is read([length, [buffer]]). - If given, +length+ must be a non-negative Integer (>= 0) or +nil+, and +buffer+ must - be a String and may not be nil. If +length+ is given and not nil, then this method - reads at most +length+ bytes from the input stream. If +length+ is not given or nil, - then this method reads all data until EOF. - When EOF is reached, this method returns nil if +length+ is given and not nil, or "" - if +length+ is not given or is nil. - If +buffer+ is given, then the read data will be placed into +buffer+ instead of a - newly created String object. +* +read+ behaves like IO#read. + Its signature is read([length, [buffer]]). + If given, +length+ must be a non-negative Integer (>= 0) or +nil+, + and +buffer+ must be a String and may not be nil. + If +length+ is given and not nil, then this method reads at most + +length+ bytes from the input stream. + If +length+ is not given or nil, then this method reads + all data until EOF. + When EOF is reached, this method returns nil if +length+ is given + and not nil, or "" if +length+ is not given or is nil. + If +buffer+ is given, then the read data will be placed + into +buffer+ instead of a newly created String object. * +each+ must be called without arguments and only yield Strings. * +rewind+ must be called without arguments. It rewinds the input stream back to the beginning. It must not raise Errno::ESPIPE: diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index 3978b70a..667c34a6 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -102,7 +102,17 @@ module Rack ## follows the ?, if any. May be ## empty, but is always required! - ## SERVER_NAME, SERVER_PORT:: When combined with SCRIPT_NAME and PATH_INFO, these variables can be used to complete the URL. Note, however, that HTTP_HOST, if present, should be used in preference to SERVER_NAME for reconstructing the request URL. SERVER_NAME and SERVER_PORT can never be empty strings, and so are always required. + ## SERVER_NAME, SERVER_PORT:: + ## When combined with SCRIPT_NAME and + ## PATH_INFO, these variables can be + ## used to complete the URL. Note, however, + ## that HTTP_HOST, if present, + ## should be used in preference to + ## SERVER_NAME for reconstructing + ## the request URL. + ## SERVER_NAME and SERVER_PORT + ## can never be empty strings, and so + ## are always required. ## HTTP_ Variables:: Variables corresponding to the ## client-supplied HTTP request @@ -112,29 +122,60 @@ module Rack ## variables should correspond with ## the presence or absence of the ## appropriate HTTP header in the - ## request. See - ## RFC3875 section 4.1.18 for specific behavior. + ## request. See + ## + ## RFC3875 section 4.1.18 for + ## specific behavior. ## In addition to this, the Rack environment must include these ## Rack-specific variables: - ## rack.version:: The Array representing this version of Rack. See Rack::VERSION, that corresponds to the version of this SPEC. - ## rack.url_scheme:: +http+ or +https+, depending on the request URL. + ## rack.version:: The Array representing this version of Rack + ## See Rack::VERSION, that corresponds to + ## the version of this SPEC. + + ## rack.url_scheme:: +http+ or +https+, depending on the + ## request URL. + ## rack.input:: See below, the input stream. + ## rack.errors:: See below, the error stream. - ## rack.multithread:: true if the application object may be simultaneously invoked by another thread in the same process, false otherwise. - ## rack.multiprocess:: true if an equivalent application object may be simultaneously invoked by another process, false otherwise. - ## rack.run_once:: true if the server expects (but does not guarantee!) that the application will only be invoked this one time during the life of its containing process. Normally, this will only be true for a server based on CGI (or something similar). - ## rack.hijack?:: present and true if the server supports connection hijacking. See below, hijacking. - ## rack.hijack:: an object responding to #call that must be called at least once before using rack.hijack_io. It is recommended #call return rack.hijack_io as well as setting it in env if necessary. - ## rack.hijack_io:: if rack.hijack? is true, and rack.hijack has received #call, this will contain an object resembling an IO. See hijacking. - ## + + ## rack.multithread:: true if the application object may be + ## simultaneously invoked by another thread + ## in the same process, false otherwise. + + ## rack.multiprocess:: true if an equivalent application object + ## may be simultaneously invoked by another + ## process, false otherwise. + + ## rack.run_once:: true if the server expects + ## (but does not guarantee!) that the + ## application will only be invoked this one + ## time during the life of its containing + ## process. Normally, this will only be true + ## for a server based on CGI + ## (or something similar). + + ## rack.hijack?:: present and true if the server supports + ## connection hijacking. See below, hijacking. + + ## rack.hijack:: an object responding to #call that must be + ## called at least once before using + ## rack.hijack_io. + ## It is recommended #call return rack.hijack_io + ## as well as setting it in env if necessary. + + ## rack.hijack_io:: if rack.hijack? is true, and rack.hijack + ## has received #call, this will contain + ## an object resembling an IO. See hijacking. ## Additional environment specifications have approved to ## standardized middleware APIs. None of these are required to ## be implemented by the server. - ## rack.session:: A hash like interface for storing request session data. + ## rack.session:: A hash like interface for storing + ## request session data. ## The store must implement: if session = env['rack.session'] ## store(key, value) (aliased as []=); @@ -218,7 +259,6 @@ module Rack } } - ## ## There are the following restrictions: ## * rack.version must be an array of Integers. @@ -311,15 +351,23 @@ module Rack v end - ## * +read+ behaves like IO#read. Its signature is read([length, [buffer]]). - ## If given, +length+ must be a non-negative Integer (>= 0) or +nil+, and +buffer+ must - ## be a String and may not be nil. If +length+ is given and not nil, then this method - ## reads at most +length+ bytes from the input stream. If +length+ is not given or nil, - ## then this method reads all data until EOF. - ## When EOF is reached, this method returns nil if +length+ is given and not nil, or "" - ## if +length+ is not given or is nil. - ## If +buffer+ is given, then the read data will be placed into +buffer+ instead of a - ## newly created String object. + ## * +read+ behaves like IO#read. + ## Its signature is read([length, [buffer]]). + ## + ## If given, +length+ must be a non-negative Integer (>= 0) or +nil+, + ## and +buffer+ must be a String and may not be nil. + ## + ## If +length+ is given and not nil, then this method reads at most + ## +length+ bytes from the input stream. + ## + ## If +length+ is not given or nil, then this method reads + ## all data until EOF. + ## + ## When EOF is reached, this method returns nil if +length+ is given + ## and not nil, or "" if +length+ is not given or is nil. + ## + ## If +buffer+ is given, then the read data will be placed + ## into +buffer+ instead of a newly created String object. def read(*args) assert("rack.input#read called with too many arguments") { args.size <= 2 -- cgit v1.2.3-24-ge0c7 From e06a25423979e0443e6252cbe0cb4b2b41fa89f0 Mon Sep 17 00:00:00 2001 From: Luca Guidi Date: Thu, 13 Feb 2014 11:15:26 +0100 Subject: Ensure Rack::Utils.best_q_match to respect requested content type --- lib/rack/utils.rb | 2 +- test/spec_utils.rb | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 6c2bf907..eb443f59 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -185,7 +185,7 @@ module Rack values = q_values(q_value_header) values.map do |req_mime, quality| - match = available_mimes.first { |am| Rack::Mime.match?(am, req_mime) } + match = available_mimes.find { |am| Rack::Mime.match?(am, req_mime) } next unless match [match, quality] end.compact.sort_by do |match, quality| diff --git a/test/spec_utils.rb b/test/spec_utils.rb index c3867965..b79ae1df 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -300,6 +300,9 @@ describe Rack::Utils do # Higher quality matches are preferred Rack::Utils.best_q_match("text/*;q=0.5,text/plain;q=1.0", %w[text/plain text/html]).should.equal "text/plain" + # Respect requested content type + Rack::Utils.best_q_match("application/json", %w[application/vnd.lotus-1-2-3 application/json]).should.equal "application/json" + # All else equal, the available mimes are preferred in order Rack::Utils.best_q_match("text/*", %w[text/html text/plain]).should.equal "text/html" Rack::Utils.best_q_match("text/plain,text/html", %w[text/html text/plain]).should.equal "text/html" -- cgit v1.2.3-24-ge0c7 From 464888a34fdb86f2f6ea086aeba6d7850a64c3fa Mon Sep 17 00:00:00 2001 From: Grayson Wright Date: Fri, 14 Feb 2014 13:36:01 -0500 Subject: Flip the lobster better --- lib/rack/lobster.rb | 11 ++++++++--- test/spec_lobster.rb | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/rack/lobster.rb b/lib/rack/lobster.rb index d1a7f7bc..195bd945 100644 --- a/lib/rack/lobster.rb +++ b/lib/rack/lobster.rb @@ -32,9 +32,14 @@ module Rack def call(env) req = Request.new(env) if req.GET["flip"] == "left" - lobster = LobsterString.split("\n"). - map { |line| line.ljust(42).reverse }. - join("\n") + lobster = LobsterString.split("\n").map do |line| + line.ljust(42).reverse. + gsub('\\', 'TEMP'). + gsub('/', '\\'). + gsub('TEMP', '/'). + gsub('{','}'). + gsub('(',')') + end.join("\n") href = "?flip=right" elsif req.GET["flip"] == "crash" raise "Lobster crashed" diff --git a/test/spec_lobster.rb b/test/spec_lobster.rb index 56a54795..c6ec2b06 100644 --- a/test/spec_lobster.rb +++ b/test/spec_lobster.rb @@ -47,7 +47,7 @@ describe Rack::Lobster do should "be flippable" do res = lobster.get("/?flip=left") res.should.be.ok - res.body.should.include "(,,,(,,(,(" + res.body.should.include "),,,),,),)" end should "provide crashing for testing purposes" do -- cgit v1.2.3-24-ge0c7 From 7fe910e0063388e2f636724925bd5f8b9905cc89 Mon Sep 17 00:00:00 2001 From: Matt Kasa Date: Fri, 7 Mar 2014 19:37:41 -0800 Subject: Add support for RFC2324 status code --- lib/rack/utils.rb | 3 ++- test/spec_response.rb | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 6c2bf907..28e0ded6 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -587,6 +587,7 @@ module Rack 415 => 'Unsupported Media Type', 416 => 'Requested Range Not Satisfiable', 417 => 'Expectation Failed', + 418 => 'I\'m a teapot', 422 => 'Unprocessable Entity', 423 => 'Locked', 424 => 'Failed Dependency', @@ -611,7 +612,7 @@ module Rack STATUS_WITH_NO_ENTITY_BODY = Set.new((100..199).to_a << 204 << 205 << 304) SYMBOL_TO_STATUS_CODE = Hash[*HTTP_STATUS_CODES.map { |code, message| - [message.downcase.gsub(/\s|-/, '_').to_sym, code] + [message.downcase.gsub(/\s|-|'/, '_').to_sym, code] }.flatten] def status_code(status) diff --git a/test/spec_response.rb b/test/spec_response.rb index 031488bb..6b13c0c9 100644 --- a/test/spec_response.rb +++ b/test/spec_response.rb @@ -251,6 +251,11 @@ describe Rack::Response do res.should.be.client_error res.should.be.method_not_allowed + res.status = 418 + res.should.not.be.successful + res.should.be.client_error + res.should.be.i_m_a_teapot + res.status = 422 res.should.not.be.successful res.should.be.client_error -- cgit v1.2.3-24-ge0c7 From d71053f2f3734326336020f669641d56e821b734 Mon Sep 17 00:00:00 2001 From: Matt Kasa Date: Fri, 7 Mar 2014 19:43:24 -0800 Subject: Add helper method for 418 --- lib/rack/response.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/rack/response.rb b/lib/rack/response.rb index bd39da3b..12536710 100644 --- a/lib/rack/response.rb +++ b/lib/rack/response.rb @@ -129,6 +129,7 @@ module Rack def forbidden?; status == 403; end def not_found?; status == 404; end def method_not_allowed?; status == 405; end + def i_m_a_teapot?; status == 418; end def unprocessable?; status == 422; end def redirect?; [301, 302, 303, 307].include? status; end -- cgit v1.2.3-24-ge0c7 From 430892c8c98c334d8395bc5ad173c95baf3abb9f Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Thu, 10 Apr 2014 17:00:10 +0100 Subject: Fix `clean_path_info` for paths with only a slash --- lib/rack/utils.rb | 2 +- test/spec_utils.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 6c2bf907..9b12bf75 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -637,7 +637,7 @@ module Rack part == '..' ? clean.pop : clean << part end - clean.unshift '/' if parts.first.empty? + clean.unshift '/' if parts.empty? || parts.first.empty? ::File.join(*clean) end diff --git a/test/spec_utils.rb b/test/spec_utils.rb index c3867965..4808e988 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -403,6 +403,10 @@ describe Rack::Utils do should "not clean directory traversal with encoded periods" do Rack::Utils.clean_path_info("/%2E%2E/README").should.equal "/%2E%2E/README" end + + should "not clean slash only paths" do + Rack::Utils.clean_path_info("/").should.equal "/" + end end describe Rack::Utils, "byte_range" do -- cgit v1.2.3-24-ge0c7 From 6aa56de9f7e07ca19fee5082c5498cd32a5a7012 Mon Sep 17 00:00:00 2001 From: KITAITI Makoto Date: Mon, 14 Apr 2014 02:22:27 +0900 Subject: Proxy body if it is fresh enough In order to prevent response body resulting in race conditions. --- lib/rack/conditionalget.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/rack/conditionalget.rb b/lib/rack/conditionalget.rb index 2c2113f2..88573166 100644 --- a/lib/rack/conditionalget.rb +++ b/lib/rack/conditionalget.rb @@ -25,11 +25,13 @@ module Rack status, headers, body = @app.call(env) headers = Utils::HeaderHash.new(headers) if status == 200 && fresh?(env, headers) - body.close if body.respond_to? :close status = 304 headers.delete('Content-Type') headers.delete('Content-Length') - body = [] + original_body = body + body = Rack::BodyProxy.new([]) do + original_body.close if original_body.respond_to?(:close) + end end [status, headers, body] else -- cgit v1.2.3-24-ge0c7 From 7dcdcc8c8910397afa70468f5576ab93a19d1fb4 Mon Sep 17 00:00:00 2001 From: David Wilkie Date: Sat, 17 May 2014 13:48:28 +0700 Subject: Refactor methodoverride to make it easier to inherit and extend --- lib/rack/methodoverride.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/rack/methodoverride.rb b/lib/rack/methodoverride.rb index 449961ce..40b1b36f 100644 --- a/lib/rack/methodoverride.rb +++ b/lib/rack/methodoverride.rb @@ -10,7 +10,7 @@ module Rack end def call(env) - if env["REQUEST_METHOD"] == "POST" + if allowed_methods.include?(env["REQUEST_METHOD"]) method = method_override(env) if HTTP_METHODS.include?(method) env["rack.methodoverride.original_method"] = env["REQUEST_METHOD"] @@ -23,9 +23,19 @@ module Rack def method_override(env) req = Request.new(env) - method = req.POST[METHOD_OVERRIDE_PARAM_KEY] || + method = method_override_param(req) || env[HTTP_METHOD_OVERRIDE_HEADER] method.to_s.upcase end + + private + + def allowed_methods + ["POST"] + end + + def method_override_param(req) + req.POST[METHOD_OVERRIDE_PARAM_KEY] + end end end -- cgit v1.2.3-24-ge0c7 From 93e7d7a9bec530b01c72d8f87761e9e2929d2f8c Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 20 May 2014 11:01:37 -0300 Subject: Move ["POST"] to a constant --- lib/rack/methodoverride.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rack/methodoverride.rb b/lib/rack/methodoverride.rb index 40b1b36f..062f3d67 100644 --- a/lib/rack/methodoverride.rb +++ b/lib/rack/methodoverride.rb @@ -4,6 +4,7 @@ module Rack METHOD_OVERRIDE_PARAM_KEY = "_method".freeze HTTP_METHOD_OVERRIDE_HEADER = "HTTP_X_HTTP_METHOD_OVERRIDE".freeze + ALLOWED_METHODS = ["POST"] def initialize(app) @app = app @@ -31,7 +32,7 @@ module Rack private def allowed_methods - ["POST"] + ALLOWED_METHODS end def method_override_param(req) -- cgit v1.2.3-24-ge0c7 From d77bd3ffa135d887fa2fcbae6514f3f919be577d Mon Sep 17 00:00:00 2001 From: Ivan Ukhov Date: Mon, 16 Jun 2014 20:48:12 +0200 Subject: Fixed build_nested_query to handle empty arrays and hashes --- lib/rack/utils.rb | 2 +- test/spec_utils.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 6c2bf907..b5295dea 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -159,7 +159,7 @@ module Rack when Hash value.map { |k, v| build_nested_query(v, prefix ? "#{prefix}[#{escape(k)}]" : escape(k)) - }.join("&") + }.reject(&:empty?).join('&') when String raise ArgumentError, "value must be a Hash" if prefix.nil? "#{prefix}=#{escape(value)}" diff --git a/test/spec_utils.rb b/test/spec_utils.rb index c3867965..e8531369 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -240,6 +240,14 @@ describe Rack::Utils do should.equal "foo[]=" Rack::Utils.build_nested_query("foo" => ["bar"]). should.equal "foo[]=bar" + Rack::Utils.build_nested_query('foo' => []). + should.equal '' + Rack::Utils.build_nested_query('foo' => {}). + should.equal '' + Rack::Utils.build_nested_query('foo' => 'bar', 'baz' => []). + should.equal 'foo=bar' + Rack::Utils.build_nested_query('foo' => 'bar', 'baz' => {}). + should.equal 'foo=bar' # The ordering of the output query string is unpredictable with 1.8's # unordered hash. Test that build_nested_query performs the inverse -- cgit v1.2.3-24-ge0c7 From 2bc59be9905748feb5dcc806e39babd47f4feffc Mon Sep 17 00:00:00 2001 From: Thaddee Tyl Date: Thu, 19 Jun 2014 13:53:07 +0200 Subject: Readme: Use an SVG badge. Badges will look more consistent and less blurry on Retina screens (and the like). --- README.rdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rdoc b/README.rdoc index 7a3c8d58..b6f383dc 100644 --- a/README.rdoc +++ b/README.rdoc @@ -1,4 +1,4 @@ -= Rack, a modular Ruby webserver interface {Build Status}[http://travis-ci.org/rack/rack] {Dependency Status}[https://gemnasium.com/rack/rack] += Rack, a modular Ruby webserver interface {Build Status}[http://travis-ci.org/rack/rack] {Dependency Status}[https://gemnasium.com/rack/rack] Rack provides a minimal, modular and adaptable interface for developing web applications in Ruby. By wrapping HTTP requests and responses in -- cgit v1.2.3-24-ge0c7 From c8aaf0bf449ce4f7d99ff3e612e12fc3f82e84e8 Mon Sep 17 00:00:00 2001 From: Leonard Garvey Date: Sat, 28 Jun 2014 19:15:12 +1000 Subject: Fix spec_request on ruby-trunk (2.2.0dev) Manually percent-encode square brackets in query string. This fixes current travis build issues. 2.2.0dev has recently changed URI parsing from RFC2396 to RFC3986. Square brackets in RFC3986 are required to be percent-encoded when present in the query string of a URI. URI.encode on 2.2.0dev isn't currently updated to support this encoding (see: https://bugs.ruby-lang.org/issues/9990) --- test/spec_request.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/spec_request.rb b/test/spec_request.rb index f5210e57..bd67ce63 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -143,7 +143,7 @@ describe Rack::Request do end should "limit the key size per nested params hash" do - nested_query = Rack::MockRequest.env_for("/?foo[bar][baz][qux]=1") + nested_query = Rack::MockRequest.env_for("/?foo%5Bbar%5D%5Bbaz%5D%5Bqux%5D=1") plain_query = Rack::MockRequest.env_for("/?foo_bar__baz__qux_=1") old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 3 @@ -1116,7 +1116,7 @@ EOF end should "raise TypeError every time if request parameters are broken" do - broken_query = Rack::MockRequest.env_for("/?foo[]=0&foo[bar]=1") + broken_query = Rack::MockRequest.env_for("/?foo%5B%5D=0&foo%5Bbar%5D=1") req = Rack::Request.new(broken_query) lambda{req.GET}.should.raise(TypeError) lambda{req.params}.should.raise(TypeError) -- cgit v1.2.3-24-ge0c7 From 925201c99cf31d397760c8b13a2936049d745e34 Mon Sep 17 00:00:00 2001 From: David Celis Date: Mon, 30 Jun 2014 11:24:58 -0400 Subject: Fix AccessLog documentation typo --- lib/rack/server.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index be7014c6..37483afb 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -166,7 +166,7 @@ module Rack # * :Port # the port to bind to (used by supporting Rack::Handler) # * :AccessLog - # webrick acess log options (or supporting Rack::Handler) + # webrick access log options (or supporting Rack::Handler) # * :debug # turn on debug output ($DEBUG = true) # * :warn -- cgit v1.2.3-24-ge0c7 From f2cbe32a1dd1863dd68fea79905acf4dad8a2c5f Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sat, 5 Jul 2014 15:40:01 -0700 Subject: Monkey patch to fix WEBrick chunking semantics. * Previously proposed in #707, unfortunately that patch caused double encoding * Fixes #707, #618 * The longevity of this patch is dubious. If WEBrick makes identical semantics modifications as I think should be done, this patch will have no effect. If WEBrick introduces changes to internal header handling, class structure, etc, we'll break. --- lib/rack/handler/webrick.rb | 18 ++++++++++++++++++ test/spec_webrick.rb | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/lib/rack/handler/webrick.rb b/lib/rack/handler/webrick.rb index f76679b4..023d8b27 100644 --- a/lib/rack/handler/webrick.rb +++ b/lib/rack/handler/webrick.rb @@ -2,6 +2,23 @@ require 'webrick' require 'stringio' require 'rack/content_length' +# This monkey patch allows for applications to perform their own chunking +# through WEBrick::HTTPResponse iff rack is set to true. +class WEBrick::HTTPResponse + attr_accessor :rack + + alias _rack_setup_header setup_header + def setup_header + app_chunking = rack && @header['transfer-encoding'] == 'chunked' + + @chunked = app_chunking if app_chunking + + _rack_setup_header + + @chunked = false if app_chunking + end +end + module Rack module Handler class WEBrick < ::WEBrick::HTTPServlet::AbstractServlet @@ -39,6 +56,7 @@ module Rack end def service(req, res) + res.rack = true env = req.meta_vars env.delete_if { |k, v| v.nil? } diff --git a/test/spec_webrick.rb b/test/spec_webrick.rb index b29a82d5..497bfe20 100644 --- a/test/spec_webrick.rb +++ b/test/spec_webrick.rb @@ -162,5 +162,23 @@ describe Rack::Handler::WEBrick do } end + should "produce correct HTTP semantics with and without app chunking" do + @server.mount "/chunked", Rack::Handler::WEBrick, + Rack::Lint.new(lambda{ |req| + [ + 200, + {"Transfer-Encoding" => "chunked"}, + ["7\r\nchunked\r\n0\r\n\r\n"] + ] + }) + + Net::HTTP.start(@host, @port){ |http| + res = http.get("/chunked") + res["Transfer-Encoding"].should.equal "chunked" + res["Content-Length"].should.equal nil + res.body.should.equal "chunked" + } + end + @server.shutdown end -- cgit v1.2.3-24-ge0c7 From c2564e3d8235c5fd95e312aa6c97f120904f4686 Mon Sep 17 00:00:00 2001 From: Keith Duncan Date: Sun, 6 Jul 2014 14:31:02 +0100 Subject: Rename clean slash only test --- test/spec_utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec_utils.rb b/test/spec_utils.rb index 4808e988..d81835c1 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -404,7 +404,7 @@ describe Rack::Utils do Rack::Utils.clean_path_info("/%2E%2E/README").should.equal "/%2E%2E/README" end - should "not clean slash only paths" do + should "clean slash only paths" do Rack::Utils.clean_path_info("/").should.equal "/" end end -- cgit v1.2.3-24-ge0c7 From 4da6ef9f5b6a98c289ab84fe789bf32137001b9d Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 6 Jul 2014 13:26:12 -0700 Subject: Rack::Utils#best_q_match returns nil with no match --- lib/rack/utils.rb | 5 +++-- test/spec_utils.rb | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index a8cf7cc1..1be818b4 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -184,13 +184,14 @@ module Rack def best_q_match(q_value_header, available_mimes) values = q_values(q_value_header) - values.map do |req_mime, quality| + matches = values.map do |req_mime, quality| match = available_mimes.find { |am| Rack::Mime.match?(am, req_mime) } next unless match [match, quality] end.compact.sort_by do |match, quality| (match.split('/', 2).count('*') * -10) + quality - end.last.first + end.last + matches && matches.first end module_function :best_q_match diff --git a/test/spec_utils.rb b/test/spec_utils.rb index b79ae1df..38ffc861 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -306,6 +306,9 @@ describe Rack::Utils do # All else equal, the available mimes are preferred in order Rack::Utils.best_q_match("text/*", %w[text/html text/plain]).should.equal "text/html" Rack::Utils.best_q_match("text/plain,text/html", %w[text/html text/plain]).should.equal "text/html" + + # When there are no matches, return nil: + Rack::Utils.best_q_match("application/json", %w[text/html text/plain]).should.equal nil end should "escape html entities [&><'\"/]" do -- cgit v1.2.3-24-ge0c7 From 64a286226941a14f68fb900e96841ccdd0abe0f7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 8 Jul 2014 11:55:38 -0700 Subject: use RFC 2396 URI parser in the mock object this fixes `env_for` on ruby trunk --- lib/rack/mock.rb | 3 ++- test/spec_mock.rb | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/rack/mock.rb b/lib/rack/mock.rb index 3ba314e4..fada5bb2 100644 --- a/lib/rack/mock.rb +++ b/lib/rack/mock.rb @@ -79,7 +79,8 @@ module Rack # Return the Rack environment used for a request to +uri+. def self.env_for(uri="", opts={}) - uri = URI(uri) + parser = URI::Parser.new + uri = parser.parse(uri) uri.path = "/#{uri.path}" unless uri.path[0] == ?/ env = DEFAULT_ENV.dup diff --git a/test/spec_mock.rb b/test/spec_mock.rb index f49b1961..3ebd7776 100644 --- a/test/spec_mock.rb +++ b/test/spec_mock.rb @@ -30,6 +30,14 @@ describe Rack::MockRequest do env.should.include "rack.version" end + should "return an environment with a path" do + env = Rack::MockRequest.env_for("http://www.example.com/parse?location[]=1&location[]=2&age_group[]=2") + env["QUERY_STRING"].should.equal "location[]=1&location[]=2&age_group[]=2" + env["PATH_INFO"].should.equal "/parse" + env.should.be.kind_of Hash + env.should.include "rack.version" + end + should "provide sensible defaults" do res = Rack::MockRequest.new(app).request -- cgit v1.2.3-24-ge0c7 From 7a8efc2a270f363b85ce610ad897184d80b7a1d6 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 24 Jun 2014 09:44:31 +1000 Subject: Prevent IP spoofing via X-Forwarded-For and Client-IP headers By specifying the same IP in X-Forwarded-For and Client-IP an attacker is able to easily spoof the value of `request.ip`, unless a trusted proxy is configured to remove any user supplied Client-IP headers. The value of request.ip should be the value after the last trusted proxy IP in X-Forwarded-For (from right to left). --- lib/rack/request.rb | 6 ------ test/spec_request.rb | 24 ++++++++++++++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 07627ddb..cc183b44 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -353,12 +353,6 @@ module Rack forwarded_ips = split_ip_addresses(@env['HTTP_X_FORWARDED_FOR']) - if client_ip = @env['HTTP_CLIENT_IP'] - # If forwarded_ips doesn't include the client_ip, it might be an - # ip spoofing attempt, so we ignore HTTP_CLIENT_IP - return client_ip if forwarded_ips.include?(client_ip) - end - return reject_trusted_ip_addresses(forwarded_ips).last || @env["REMOTE_ADDR"] end diff --git a/test/spec_request.rb b/test/spec_request.rb index bd67ce63..c83b0526 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -1037,12 +1037,6 @@ EOF 'HTTP_CLIENT_IP' => '1.1.1.1' res.body.should.equal '1.1.1.1' - # Spoofing attempt - res = mock.get '/', - 'HTTP_X_FORWARDED_FOR' => '1.1.1.1', - 'HTTP_CLIENT_IP' => '2.2.2.2' - res.body.should.equal '1.1.1.1' - res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '8.8.8.8, 9.9.9.9' res.body.should.equal '9.9.9.9' @@ -1061,6 +1055,24 @@ EOF res.body.should.equal '3.4.5.6' end + should "not allow IP spoofing via Client-IP and X-Forwarded-For headers" do + mock = Rack::MockRequest.new(Rack::Lint.new(ip_app)) + + # IP Spoofing attempt: + # Client sends X-Forwarded-For: 6.6.6.6 + # Client-IP: 6.6.6.6 + # Load balancer adds X-Forwarded-For: 2.2.2.3, 192.168.0.7 + # App receives: X-Forwarded-For: 6.6.6.6 + # X-Forwarded-For: 2.2.2.3, 192.168.0.7 + # Client-IP: 6.6.6.6 + # Rack env: HTTP_X_FORWARDED_FOR: '6.6.6.6, 2.2.2.3, 192.168.0.7' + # HTTP_CLIENT_IP: '6.6.6.6' + res = mock.get '/', + 'HTTP_X_FORWARDED_FOR' => '6.6.6.6, 2.2.2.3, 192.168.0.7', + 'HTTP_CLIENT_IP' => '6.6.6.6' + res.body.should.equal '2.2.2.3' + end + should "regard local addresses as proxies" do req = Rack::Request.new(Rack::MockRequest.env_for("/")) req.trusted_proxy?('127.0.0.1').should.equal 0 -- cgit v1.2.3-24-ge0c7 From 71c69113f269f04207157bee6c197b5675f3df61 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 13:53:43 -0700 Subject: Do not truncate POST data on `;`, closes #543 It appears Rack has been doing this for years. It's not correct behavior for any generators that I can remember. It comes from the cookie parsing code. --- lib/rack/request.rb | 2 +- test/spec_request.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 07627ddb..1556a2b8 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -372,7 +372,7 @@ module Rack end def parse_query(qs) - Utils.parse_nested_query(qs) + Utils.parse_nested_query(qs, '&') end def parse_multipart(env) diff --git a/test/spec_request.rb b/test/spec_request.rb index bd67ce63..734eccc3 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -130,6 +130,14 @@ describe Rack::Request do req.params.should.equal "foo" => "bar", "quux" => "bla" end + should "not truncate query strings containing semi-colons #543" do + req = Rack::Request.new(Rack::MockRequest.env_for("/?foo=bar&quux=b;la")) + req.query_string.should.equal "foo=bar&quux=b;la" + req.GET.should.equal "foo" => "bar", "quux" => "b;la" + req.POST.should.be.empty + req.params.should.equal "foo" => "bar", "quux" => "b;la" + end + should "limit the keys from the GET query string" do env = Rack::MockRequest.env_for("/?foo=bar") -- cgit v1.2.3-24-ge0c7 From 2ab24cf3590fc1fa6cf551841ef97edf2cdf1ebf Mon Sep 17 00:00:00 2001 From: Stephen Best Date: Sat, 10 Aug 2013 16:24:33 +0100 Subject: ShowException only serves HTML Accept header contains text/html Rather than be concerned with whether a request is an asynchronous browser request or not it is better to simply consider the Accept header and only serve HTML to clients that specifically ask for it. This way you will not find your pure JSON API application splitting out HTML error messages to your console when using curl :) --- lib/rack/showexceptions.rb | 12 ++++++------ test/spec_showexceptions.rb | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/rack/showexceptions.rb b/lib/rack/showexceptions.rb index c91ca07c..39499d5e 100644 --- a/lib/rack/showexceptions.rb +++ b/lib/rack/showexceptions.rb @@ -28,12 +28,12 @@ module Rack env["rack.errors"].puts(exception_string) env["rack.errors"].flush - if prefers_plain_text?(env) - content_type = "text/plain" - body = [exception_string] - else + if accepts_html?(env) content_type = "text/html" body = pretty(env, e) + else + content_type = "text/plain" + body = [exception_string] end [500, @@ -42,8 +42,8 @@ module Rack body] end - def prefers_plain_text?(env) - env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" && (!env["HTTP_ACCEPT"] || !env["HTTP_ACCEPT"].include?("text/html")) + def accepts_html?(env) + env["HTTP_ACCEPT"] && env["HTTP_ACCEPT"].include?("text/html") end def dump_exception(exception) diff --git a/test/spec_showexceptions.rb b/test/spec_showexceptions.rb index bdd5ce5b..c5e2cb02 100644 --- a/test/spec_showexceptions.rb +++ b/test/spec_showexceptions.rb @@ -16,7 +16,7 @@ describe Rack::ShowExceptions do )) lambda{ - res = req.get("/") + res = req.get("/", "HTTP_ACCEPT" => "text/html") }.should.not.raise res.should.be.a.server_error @@ -26,7 +26,7 @@ describe Rack::ShowExceptions do res.should =~ /ShowExceptions/ end - it "responds with plain text on AJAX requests accepting anything but HTML" do + it "responds with plain text to requests not specifically accepting HTML" do res = nil req = Rack::MockRequest.new( @@ -35,7 +35,7 @@ describe Rack::ShowExceptions do )) lambda{ - res = req.get("/", "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest") + res = req.get( "/", "HTTP_ACCECPT" => "*/*") }.should.not.raise res.should.be.a.server_error @@ -47,7 +47,7 @@ describe Rack::ShowExceptions do res.body.should.include __FILE__ end - it "responds with HTML on AJAX requests accepting HTML" do + it "responds with HTML to requests specifically accepting HTML" do res = nil req = Rack::MockRequest.new( @@ -56,7 +56,7 @@ describe Rack::ShowExceptions do )) lambda{ - res = req.get("/", "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", "HTTP_ACCEPT" => "text/html") + res = req.get("/", "HTTP_ACCEPT" => "text/html") }.should.not.raise res.should.be.a.server_error @@ -79,7 +79,7 @@ describe Rack::ShowExceptions do ) lambda{ - res = req.get("/") + res = req.get("/", "HTTP_ACCEPT" => "text/html") }.should.not.raise res.should.be.a.server_error -- cgit v1.2.3-24-ge0c7 From 893a2c505a3a43b4f224b50ea06567dacb473cae Mon Sep 17 00:00:00 2001 From: Stephen Best Date: Sat, 10 Aug 2013 20:15:38 +0100 Subject: ShowExceptions minor refactoring * Load HTML exception template only if needed * Only #call is public * Enumerable body concern in one place --- lib/rack/showexceptions.rb | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/rack/showexceptions.rb b/lib/rack/showexceptions.rb index 39499d5e..0e8a5952 100644 --- a/lib/rack/showexceptions.rb +++ b/lib/rack/showexceptions.rb @@ -17,7 +17,6 @@ module Rack def initialize(app) @app = app - @template = ERB.new(TEMPLATE) end def call(env) @@ -33,15 +32,21 @@ module Rack body = pretty(env, e) else content_type = "text/plain" - body = [exception_string] + body = exception_string end - [500, - {"Content-Type" => content_type, - "Content-Length" => Rack::Utils.bytesize(body.join).to_s}, - body] + [ + 500, + { + "Content-Type" => content_type, + "Content-Length" => Rack::Utils.bytesize(body).to_s, + }, + [body], + ] end + private + def accepts_html?(env) env["HTTP_ACCEPT"] && env["HTTP_ACCEPT"].include?("text/html") end @@ -85,7 +90,7 @@ module Rack end }.compact - [@template.result(binding)] + ERB.new(TEMPLATE).result(binding) end def h(obj) # :nodoc: -- cgit v1.2.3-24-ge0c7 From b8cb4d1f2c37868d9fecb4202a364ca7879ef287 Mon Sep 17 00:00:00 2001 From: Stephen Best Date: Tue, 13 Aug 2013 08:18:52 +0100 Subject: Restore public API --- lib/rack/showexceptions.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rack/showexceptions.rb b/lib/rack/showexceptions.rb index 0e8a5952..d8127380 100644 --- a/lib/rack/showexceptions.rb +++ b/lib/rack/showexceptions.rb @@ -45,11 +45,14 @@ module Rack ] end - private + def prefers_plaintext?(env) + !accepts_html(env) + end def accepts_html?(env) env["HTTP_ACCEPT"] && env["HTTP_ACCEPT"].include?("text/html") end + private :accepts_html? def dump_exception(exception) string = "#{exception.class}: #{exception.message}\n" -- cgit v1.2.3-24-ge0c7 From ce99c0d6b804ebb5931ae5bbd8eb946364649eba Mon Sep 17 00:00:00 2001 From: Stephen Best Date: Fri, 10 Jan 2014 17:29:49 +0000 Subject: Undo template refactoring As this is orthoganol to HTML rendering change. --- lib/rack/showexceptions.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rack/showexceptions.rb b/lib/rack/showexceptions.rb index d8127380..d0d2d07a 100644 --- a/lib/rack/showexceptions.rb +++ b/lib/rack/showexceptions.rb @@ -17,6 +17,7 @@ module Rack def initialize(app) @app = app + @template = ERB.new(TEMPLATE) end def call(env) @@ -93,7 +94,7 @@ module Rack end }.compact - ERB.new(TEMPLATE).result(binding) + @template.result(binding) end def h(obj) # :nodoc: -- cgit v1.2.3-24-ge0c7 From 01428fe54f003996dc04811fe0216de2258a2e09 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 14:05:42 -0700 Subject: Flip to best_q_match, so we provide html to */* --- lib/rack/showexceptions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/showexceptions.rb b/lib/rack/showexceptions.rb index d0d2d07a..731aea49 100644 --- a/lib/rack/showexceptions.rb +++ b/lib/rack/showexceptions.rb @@ -51,7 +51,7 @@ module Rack end def accepts_html?(env) - env["HTTP_ACCEPT"] && env["HTTP_ACCEPT"].include?("text/html") + Rack::Utils.best_q_match(env["HTTP_ACCEPT"], %w[text/html]) end private :accepts_html? -- cgit v1.2.3-24-ge0c7 From 6f1a4a4873fbf2fd54f2582a9e388b96722e21de Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 14:24:32 -0700 Subject: correct typo and refactor tests for coverage Closes #592 --- test/spec_showexceptions.rb | 61 ++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/test/spec_showexceptions.rb b/test/spec_showexceptions.rb index c5e2cb02..6400fdb9 100644 --- a/test/spec_showexceptions.rb +++ b/test/spec_showexceptions.rb @@ -26,7 +26,7 @@ describe Rack::ShowExceptions do res.should =~ /ShowExceptions/ end - it "responds with plain text to requests not specifically accepting HTML" do + it "responds with HTML only to requests accepting HTML" do res = nil req = Rack::MockRequest.new( @@ -34,39 +34,32 @@ describe Rack::ShowExceptions do lambda{|env| raise RuntimeError, "It was never supposed to work" } )) - lambda{ - res = req.get( "/", "HTTP_ACCECPT" => "*/*") - }.should.not.raise - - res.should.be.a.server_error - res.status.should.equal 500 - - res.content_type.should.equal "text/plain" - - res.body.should.include "RuntimeError: It was never supposed to work\n" - res.body.should.include __FILE__ - end - - it "responds with HTML to requests specifically accepting HTML" do - res = nil - - req = Rack::MockRequest.new( - show_exceptions( - lambda{|env| raise RuntimeError, "It was never supposed to work" } - )) - - lambda{ - res = req.get("/", "HTTP_ACCEPT" => "text/html") - }.should.not.raise - - res.should.be.a.server_error - res.status.should.equal 500 - - res.content_type.should.equal "text/html" - - res.body.should.include "RuntimeError" - res.body.should.include "It was never supposed to work" - res.body.should.include Rack::Utils.escape_html(__FILE__) + [ + # Serve text/html when the client accepts text/html + ["text/html", ["/", "HTTP_ACCEPT" => "text/html"]], + ["text/html", ["/", "HTTP_ACCEPT" => "*/*"]], + # Serve text/plain when the client does not accept text/html + ["text/plain", ["/"]], + ["text/plain", ["/", "HTTP_ACCEPT" => "application/json"]] + ].each do |exmime, rargs| + lambda{ + res = req.get(*rargs) + }.should.not.raise + + res.should.be.a.server_error + res.status.should.equal 500 + + res.content_type.should.equal exmime + + res.body.should.include "RuntimeError" + res.body.should.include "It was never supposed to work" + + if exmime == "text/html" + res.body.should.include '' + else + res.body.should.not.include '' + end + end end it "handles exceptions without a backtrace" do -- cgit v1.2.3-24-ge0c7 From b3e7a7c3d7c6236efd0b38301bf7aeb43a4c19ee Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 14:42:20 -0700 Subject: Gracefully handle cycles in parameters Might close 632, pending more information. --- lib/rack/utils.rb | 6 +++++- test/spec_utils.rb | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 264b6b62..32868bdb 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -533,7 +533,11 @@ module Rack hash.keys.each do |key| value = hash[key] if value.kind_of?(self.class) - hash[key] = value.to_params_hash + if value.object_id == self.object_id + hash[key] = hash + else + hash[key] = value.to_params_hash + end elsif value.kind_of?(Array) value.map! {|x| x.kind_of?(self.class) ? x.to_params_hash : x} end diff --git a/test/spec_utils.rb b/test/spec_utils.rb index de12f83d..f6acb436 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -123,6 +123,17 @@ describe Rack::Utils do Rack::Utils.parse_query(",foo=bar;,", ";,").should.equal "foo" => "bar" end + should "not create infinite loops with cycle structures" do + ex = { "foo" => nil } + ex["foo"] = ex + + params = Rack::Utils::KeySpaceConstrainedParams.new + params['foo'] = params + lambda { + params.to_params_hash.should.equal ex + }.should.not.raise + end + should "parse nested query strings correctly" do Rack::Utils.parse_nested_query("foo"). should.equal "foo" => nil -- cgit v1.2.3-24-ge0c7 From e8ba98b474ca1480f805420394d51f6670da49e6 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 15:18:39 -0700 Subject: Fix showexceptions specs on 1.8. Need travis back. --- test/spec_showexceptions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/spec_showexceptions.rb b/test/spec_showexceptions.rb index 6400fdb9..7d50c59f 100644 --- a/test/spec_showexceptions.rb +++ b/test/spec_showexceptions.rb @@ -36,11 +36,11 @@ describe Rack::ShowExceptions do [ # Serve text/html when the client accepts text/html - ["text/html", ["/", "HTTP_ACCEPT" => "text/html"]], - ["text/html", ["/", "HTTP_ACCEPT" => "*/*"]], + ["text/html", ["/", {"HTTP_ACCEPT" => "text/html"}]], + ["text/html", ["/", {"HTTP_ACCEPT" => "*/*"}]], # Serve text/plain when the client does not accept text/html ["text/plain", ["/"]], - ["text/plain", ["/", "HTTP_ACCEPT" => "application/json"]] + ["text/plain", ["/", {"HTTP_ACCEPT" => "application/json"}]] ].each do |exmime, rargs| lambda{ res = req.get(*rargs) -- cgit v1.2.3-24-ge0c7 From 4582f345bfc854556c7d5a54294193f322104f99 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 15:20:00 -0700 Subject: Fix cycle tests on 1.8.7 --- test/spec_utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec_utils.rb b/test/spec_utils.rb index f6acb436..a7894b45 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -130,7 +130,7 @@ describe Rack::Utils do params = Rack::Utils::KeySpaceConstrainedParams.new params['foo'] = params lambda { - params.to_params_hash.should.equal ex + params.to_params_hash.to_s.should.equal ex.to_s }.should.not.raise end -- cgit v1.2.3-24-ge0c7 From f467f1b8c21127473b9e6277608d06f9e81e101e Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 15:20:44 -0700 Subject: Fix URI parsing on 1.8.7, also address perf --- lib/rack/mock.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/rack/mock.rb b/lib/rack/mock.rb index fada5bb2..3c02c1fe 100644 --- a/lib/rack/mock.rb +++ b/lib/rack/mock.rb @@ -77,10 +77,16 @@ module Rack body.close if body.respond_to?(:close) end + # For historical reasons, we're pinning to RFC 2396. It's easier for users + # and we get support from ruby 1.8 to 2.2 using this method. + def self.parse_uri_rfc2396(uri) + @parser ||= defined?(URI::RFC2396_Parser) ? URI::RFC2396_Parser.new : URI + @parser.parse(uri) + end + # Return the Rack environment used for a request to +uri+. def self.env_for(uri="", opts={}) - parser = URI::Parser.new - uri = parser.parse(uri) + uri = parse_uri_rfc2396(uri) uri.path = "/#{uri.path}" unless uri.path[0] == ?/ env = DEFAULT_ENV.dup -- cgit v1.2.3-24-ge0c7 From 7d20fa6c1ea894937d2326bfec0549cf53c95f21 Mon Sep 17 00:00:00 2001 From: Max Cantor Date: Fri, 27 Jun 2014 15:16:58 -0400 Subject: whitespace --- lib/rack/server.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index 37483afb..4e268b04 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -1,7 +1,10 @@ require 'optparse' + module Rack + class Server + class Options def parse!(args) options = {} @@ -364,4 +367,5 @@ module Rack end end + end -- cgit v1.2.3-24-ge0c7 From 52a80a79e27d171af3ab7e604f9bc211bc9e3e80 Mon Sep 17 00:00:00 2001 From: Max Cantor Date: Fri, 27 Jun 2014 15:44:16 -0400 Subject: Give @middleware a more semantic name. Had to put the class method definitions in an eigenclass wrapper to use the 'alias' keyword sanely. It wouldn't be necessary if old behavior of the middleware was unsupported, but that would be too invasive for just a small clarity change. --- lib/rack/server.rb | 45 +++++++++++++++++++++++++++------------------ test/spec_server.rb | 10 +++++----- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index 4e268b04..711071ca 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -205,29 +205,37 @@ module Rack @app ||= options[:builder] ? build_app_from_string : build_app_and_options_from_config end - def self.logging_middleware - lambda { |server| - server.server.name =~ /CGI/ ? nil : [Rack::CommonLogger, $stderr] - } - end + class << self + def logging_middleware + lambda { |server| + server.server.name =~ /CGI/ ? nil : [Rack::CommonLogger, $stderr] + } + end - def self.middleware - @middleware ||= begin - m = Hash.new {|h,k| h[k] = []} - m["deployment"].concat [ - [Rack::ContentLength], - [Rack::Chunked], - logging_middleware - ] - m["development"].concat m["deployment"] + [[Rack::ShowExceptions], [Rack::Lint]] - m + def default_middleware_by_environment + @default_middleware_by_environment ||= begin + m = Hash.new {|h,k| h[k] = []} + m["deployment"].concat [ + [Rack::ContentLength], + [Rack::Chunked], + logging_middleware + ] + m["development"].concat m["deployment"] + [[Rack::ShowExceptions], [Rack::Lint]] + m + end end + + # Aliased for backwards-compatibility + alias :middleware :default_middleware_by_environment end - def middleware - self.class.middleware + def default_middleware_by_environment + self.class.default_middleware_by_environment end + # Aliased for backwards-compatibility + alias :middleware :default_middleware_by_environment + def start &blk if options[:warn] $-w = true @@ -307,7 +315,8 @@ module Rack end def build_app(app) - middleware[options[:environment]].reverse_each do |middleware| + middlewares = default_middleware_by_environment[options[:environment]] + middlewares.reverse_each do |middleware| middleware = middleware.call(self) if middleware.respond_to?(:call) next unless middleware klass, *args = middleware diff --git a/test/spec_server.rb b/test/spec_server.rb index 44d4bcbb..59391681 100644 --- a/test/spec_server.rb +++ b/test/spec_server.rb @@ -30,14 +30,14 @@ describe Rack::Server do should "not include Rack::Lint in deployment or none environments" do server = Rack::Server.new(:app => 'foo') - server.middleware['deployment'].flatten.should.not.include(Rack::Lint) - server.middleware['none'].flatten.should.not.include(Rack::Lint) + server.default_middleware_by_environment['deployment'].flatten.should.not.include(Rack::Lint) + server.default_middleware_by_environment['none'].flatten.should.not.include(Rack::Lint) end should "not include Rack::ShowExceptions in deployment or none environments" do server = Rack::Server.new(:app => 'foo') - server.middleware['deployment'].flatten.should.not.include(Rack::ShowExceptions) - server.middleware['none'].flatten.should.not.include(Rack::ShowExceptions) + server.default_middleware_by_environment['deployment'].flatten.should.not.include(Rack::ShowExceptions) + server.default_middleware_by_environment['none'].flatten.should.not.include(Rack::ShowExceptions) end should "support CGI" do @@ -53,7 +53,7 @@ describe Rack::Server do should "not force any middleware under the none configuration" do server = Rack::Server.new(:app => 'foo') - server.middleware['none'].should.be.empty + server.default_middleware_by_environment['none'].should.be.empty end should "use a full path to the pidfile" do -- cgit v1.2.3-24-ge0c7 From 704be37eeafa748141008418d7fcae49bcbae421 Mon Sep 17 00:00:00 2001 From: Max Cantor Date: Fri, 27 Jun 2014 15:56:50 -0400 Subject: Simplify default middleware construction. - Removed concat, the DRYness isn't worth the loss of clarity. - Removed ||=, no need to memoize such a small operation. - Removed the array-default hash usage; if this behavior is supported, we should add a test for it. --- lib/rack/server.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index 711071ca..872b0798 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -213,16 +213,21 @@ module Rack end def default_middleware_by_environment - @default_middleware_by_environment ||= begin - m = Hash.new {|h,k| h[k] = []} - m["deployment"].concat [ + { + "deployment" => [ [Rack::ContentLength], [Rack::Chunked], logging_middleware - ] - m["development"].concat m["deployment"] + [[Rack::ShowExceptions], [Rack::Lint]] - m - end + ], + "development" => [ + [Rack::ContentLength], + [Rack::Chunked], + logging_middleware, + [Rack::ShowExceptions], + [Rack::Lint] + ], + "none" => [] + } end # Aliased for backwards-compatibility @@ -317,6 +322,7 @@ module Rack def build_app(app) middlewares = default_middleware_by_environment[options[:environment]] middlewares.reverse_each do |middleware| + # these 2 lines specifically for Rack::Server.logging_middleware middleware = middleware.call(self) if middleware.respond_to?(:call) next unless middleware klass, *args = middleware -- cgit v1.2.3-24-ge0c7 From a11a76ec70528479e45554f2d93c735cc2fda4dc Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 13 Jul 2014 16:19:51 -0700 Subject: remove incorrect comment in PR 706 --- lib/rack/server.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index 872b0798..efd89c27 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -322,7 +322,6 @@ module Rack def build_app(app) middlewares = default_middleware_by_environment[options[:environment]] middlewares.reverse_each do |middleware| - # these 2 lines specifically for Rack::Server.logging_middleware middleware = middleware.call(self) if middleware.respond_to?(:call) next unless middleware klass, *args = middleware -- cgit v1.2.3-24-ge0c7 From 167b6480235ff00ed5f355698bf00ec2f250f72e Mon Sep 17 00:00:00 2001 From: James Tucker Date: Mon, 14 Jul 2014 10:59:47 -0700 Subject: ParameterTypeError for parse_nested_query Inherits from TypeError, so existing code that checks types by is_a? and friends should still work fine. This should enable users to be more confident that they are only catching this error case, not other exceptional conditions. Closes #524 --- lib/rack/utils.rb | 18 +++++++++++++++--- test/spec_utils.rb | 6 +++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 32868bdb..27ac956f 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -22,6 +22,10 @@ module Rack # applications adopted from all kinds of Ruby libraries. module Utils + # ParameterTypeError is the error that is raised when incoming structural + # parameters (parsed by parse_nested_query) contain conflicting types. + class ParameterTypeError < TypeError; end + # URI escapes. (CGI style space to +) def escape(s) URI.encode_www_form_component(s) @@ -87,6 +91,11 @@ module Rack end module_function :parse_query + # parse_nested_query expands a query string into structural types. Supported + # types are Arrays, Hashes and basic value types. It is possible to supply + # query strings with parameters of conflicting types, in this case a + # ParameterTypeError is raised. Users are encouraged to return a 400 in this + # case. def parse_nested_query(qs, d = nil) params = KeySpaceConstrainedParams.new @@ -100,6 +109,9 @@ module Rack end module_function :parse_nested_query + # normalize_params recursively expands parameters into structural types. If + # the structural types represented by two different parameter names are in + # conflict, a ParameterTypeError is raised. def normalize_params(params, name, v = nil) name =~ %r(\A[\[\]]*([^\[\]]+)\]*) k = $1 || '' @@ -113,12 +125,12 @@ module Rack params[name] = v elsif after == "[]" params[k] ||= [] - raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) + raise ParameterTypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) params[k] << v elsif after =~ %r(^\[\]\[([^\[\]]+)\]$) || after =~ %r(^\[\](.+)$) child_key = $1 params[k] ||= [] - raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) + raise ParameterTypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array) if params_hash_type?(params[k].last) && !params[k].last.key?(child_key) normalize_params(params[k].last, child_key, v) else @@ -126,7 +138,7 @@ module Rack end else params[k] ||= params.class.new - raise TypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params_hash_type?(params[k]) + raise ParameterTypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params_hash_type?(params[k]) params[k] = normalize_params(params[k], after, v) end diff --git a/test/spec_utils.rb b/test/spec_utils.rb index a7894b45..6391f951 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -213,15 +213,15 @@ describe Rack::Utils do should.equal "x" => {"y" => [{"z" => "1", "w" => "a"}, {"z" => "2", "w" => "3"}]} lambda { Rack::Utils.parse_nested_query("x[y]=1&x[y]z=2") }. - should.raise(TypeError). + should.raise(Rack::Utils::ParameterTypeError). message.should.equal "expected Hash (got String) for param `y'" lambda { Rack::Utils.parse_nested_query("x[y]=1&x[]=1") }. - should.raise(TypeError). + should.raise(Rack::Utils::ParameterTypeError). message.should.match(/expected Array \(got [^)]*\) for param `x'/) lambda { Rack::Utils.parse_nested_query("x[y]=1&x[y][][w]=2") }. - should.raise(TypeError). + should.raise(Rack::Utils::ParameterTypeError). message.should.equal "expected Array (got String) for param `y'" end -- cgit v1.2.3-24-ge0c7 From c25dede78d02bbcd9f836352be01ed24c28884a4 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 18 Oct 2013 19:46:12 +0000 Subject: README: update to add yahns to the list of servers yahns will eventually support more than Rack, but for now it only knows Rack. ref: http://yahns.yhbt.net/README Signed-off-by: James Tucker --- README.rdoc | 1 + 1 file changed, 1 insertion(+) diff --git a/README.rdoc b/README.rdoc index b6f383dc..d5bd6d2e 100644 --- a/README.rdoc +++ b/README.rdoc @@ -33,6 +33,7 @@ These web servers include Rack handlers in their distributions: * Unicorn * unixrack * uWSGI +* yahns * Zbatery Any valid Rack app will run the same on all these handlers, without -- cgit v1.2.3-24-ge0c7 From b51f3036fbdb911eac6b5cf97ebf9388fb1f14a9 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 6 Nov 2013 03:02:48 +0000 Subject: builder: avoid to_app on every request when using map By calling to_app immediately after initializing the per-map Rack::Builder instances. Otherwise, benefits of warmup and web servers taking advantage of CoW are lost. This passes tests, and is lightly tested and I have not verified this for any negative consequences or incompatibilities. Signed-off-by: James Tucker --- lib/rack/builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb index a4f6c575..bda3be27 100644 --- a/lib/rack/builder.rb +++ b/lib/rack/builder.rb @@ -157,7 +157,7 @@ module Rack def generate_map(default_app, mapping) mapped = default_app ? {'/' => default_app} : {} - mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b) } + mapping.each { |r,b| mapped[r] = self.class.new(default_app, &b).to_app } URLMap.new(mapped) end end -- cgit v1.2.3-24-ge0c7 From 895beec0622d3cafdc5fbae20d665c6d5f6c8e7c Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 12 Nov 2013 20:58:46 +0000 Subject: chunked: do not chunk on pre-HTTP/1.0 clients Ancient HTTP clients which predate HTTP/1.0 may not set HTTP_VERSION at all, and those do not support chunking. RFC 1945 describes HTTP/0.9 as well as HTTP/1.0 Signed-off-by: James Tucker --- lib/rack/chunked.rb | 13 ++++++++++++- test/spec_chunked.rb | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/rack/chunked.rb b/lib/rack/chunked.rb index a400756a..ea221fa9 100644 --- a/lib/rack/chunked.rb +++ b/lib/rack/chunked.rb @@ -39,11 +39,22 @@ module Rack @app = app end + # pre-HTTP/1.0 (informally "HTTP/0.9") HTTP requests did not have + # a version (nor response headers) + def chunkable_version?(ver) + case ver + when "HTTP/1.0", nil, "HTTP/0.9" + false + else + true + end + end + def call(env) status, headers, body = @app.call(env) headers = HeaderHash.new(headers) - if env['HTTP_VERSION'] == 'HTTP/1.0' || + if ! chunkable_version?(env['HTTP_VERSION']) || STATUS_WITH_NO_ENTITY_BODY.include?(status) || headers['Content-Length'] || headers['Transfer-Encoding'] diff --git a/test/spec_chunked.rb b/test/spec_chunked.rb index 12f21581..0a6d9ff1 100644 --- a/test/spec_chunked.rb +++ b/test/spec_chunked.rb @@ -64,6 +64,22 @@ describe Rack::Chunked do body.join.should.equal 'Hello World!' end + should 'not modify response when client is ancient, pre-HTTP/1.0' do + app = lambda { |env| [200, {"Content-Type" => "text/plain"}, ['Hello', ' ', 'World!']] } + check = lambda do + status, headers, body = chunked(app).call(@env.dup) + status.should.equal 200 + headers.should.not.include 'Transfer-Encoding' + body.join.should.equal 'Hello World!' + end + + @env.delete('HTTP_VERSION') # unicorn will do this on pre-HTTP/1.0 requests + check.call + + @env['HTTP_VERSION'] = 'HTTP/0.9' # not sure if this happens in practice + check.call + end + should 'not modify response when Transfer-Encoding header already present' do app = lambda { |env| [200, {"Content-Type" => "text/plain", 'Transfer-Encoding' => 'identity'}, ['Hello', ' ', 'World!']] -- cgit v1.2.3-24-ge0c7 From 2f3d6655218c967cf1b20600f701349bdb418bfc Mon Sep 17 00:00:00 2001 From: Lenny Marks Date: Thu, 27 Mar 2014 16:34:42 -0400 Subject: Record Tempfiles from multipart form data in env[rack.tempfiles] To facilitate cleanup without depending on garbage collection. --- lib/rack/multipart/parser.rb | 7 ++++--- test/spec_request.rb | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index fa47fd16..00e59ba7 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -16,10 +16,10 @@ module Rack content_length = env['CONTENT_LENGTH'] content_length = content_length.to_i if content_length - new($1, io, content_length) + new($1, io, content_length, env) end - def initialize(boundary, io, content_length) + def initialize(boundary, io, content_length, env) @buf = "" if @buf.respond_to? :force_encoding @@ -31,6 +31,7 @@ module Rack @io = io @content_length = content_length @boundary_size = Utils.bytesize(@boundary) + EOL.size + @env = env if @content_length @content_length -= @boundary_size @@ -112,7 +113,7 @@ module Rack filename = get_filename(head) if filename - body = Tempfile.new("RackMultipart") + (@env['rack.tempfiles'] ||= []) << body = Tempfile.new("RackMultipart") body.binmode if body.respond_to?(:binmode) end diff --git a/test/spec_request.rb b/test/spec_request.rb index 10394d54..e5ec254e 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -748,6 +748,31 @@ EOF req.POST["mean"][:tempfile].read.should.equal "--AaB03xha" end + should "record tempfiles from multipart form data in env[rack.tempfiles]" do + input = < "multipart/form-data, boundary=AaB03x", + "CONTENT_LENGTH" => input.size, + :input => input) + req = Rack::Request.new(env) + req.params + env['rack.tempfiles'].size.should.equal(2) + end + should "detect invalid multipart form data" do input = < Date: Thu, 27 Mar 2014 16:44:30 -0400 Subject: Rack::TempfileReaper - should do nothing (i.e. not bomb out) without env[rack.tempfiles] - should close env[rack.tempfiles] when body is closed - should initialize env[rack.tempfiles] when not already present - should append env[rack.tempfiles] when already present --- lib/rack.rb | 1 + lib/rack/tempfile_reaper.rb | 22 ++++++++++++++++ test/spec_tempfile_reaper.rb | 63 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 lib/rack/tempfile_reaper.rb create mode 100644 test/spec_tempfile_reaper.rb diff --git a/lib/rack.rb b/lib/rack.rb index 57119df3..341514c5 100644 --- a/lib/rack.rb +++ b/lib/rack.rb @@ -53,6 +53,7 @@ module Rack autoload :ShowExceptions, "rack/showexceptions" autoload :ShowStatus, "rack/showstatus" autoload :Static, "rack/static" + autoload :TempfileReaper, "rack/tempfile_reaper" autoload :URLMap, "rack/urlmap" autoload :Utils, "rack/utils" autoload :Multipart, "rack/multipart" diff --git a/lib/rack/tempfile_reaper.rb b/lib/rack/tempfile_reaper.rb new file mode 100644 index 00000000..1500b06a --- /dev/null +++ b/lib/rack/tempfile_reaper.rb @@ -0,0 +1,22 @@ +require 'rack/body_proxy' + +module Rack + + # Middleware tracks and cleans Tempfiles created throughout a request (i.e. Rack::Multipart) + # Ideas/strategy based on posts by Eric Wong and Charles Oliver Nutter + # https://groups.google.com/forum/#!searchin/rack-devel/temp/rack-devel/brK8eh-MByw/sw61oJJCGRMJ + class TempfileReaper + def initialize(app) + @app = app + end + + def call(env) + env['rack.tempfiles'] ||= [] + status, headers, body = @app.call(env) + body_proxy = BodyProxy.new(body) do + env['rack.tempfiles'].each { |f| f.close! } unless env['rack.tempfiles'].nil? + end + [status, headers, body_proxy] + end + end +end diff --git a/test/spec_tempfile_reaper.rb b/test/spec_tempfile_reaper.rb new file mode 100644 index 00000000..ac39d878 --- /dev/null +++ b/test/spec_tempfile_reaper.rb @@ -0,0 +1,63 @@ +require 'rack/tempfile_reaper' +require 'rack/lint' +require 'rack/mock' + +describe Rack::TempfileReaper do + class MockTempfile + attr_reader :closed + + def initialize + @closed = false + end + + def close! + @closed = true + end + end + + before do + @env = Rack::MockRequest.env_for + end + + def call(app) + Rack::Lint.new(Rack::TempfileReaper.new(app)).call(@env) + end + + should 'do nothing (i.e. not bomb out) without env[rack.tempfiles]' do + app = lambda { |_| [200, {}, ['Hello, World!']] } + response = call(app) + response[2].close + response[0].should.equal(200) + end + + should 'close env[rack.tempfiles] when body is closed' do + tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new + @env['rack.tempfiles'] = [ tempfile1, tempfile2 ] + app = lambda { |_| [200, {}, ['Hello, World!']] } + call(app)[2].close + tempfile1.closed.should.equal true + tempfile2.closed.should.equal true + end + + should 'initialize env[rack.tempfiles] when not already present' do + tempfile = MockTempfile.new + app = lambda do |env| + env['rack.tempfiles'] << tempfile + [200, {}, ['Hello, World!']] + end + call(app)[2].close + tempfile.closed.should.equal true + end + + should 'append env[rack.tempfiles] when already present' do + tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new + @env['rack.tempfiles'] = [ tempfile1 ] + app = lambda do |env| + env['rack.tempfiles'] << tempfile2 + [200, {}, ['Hello, World!']] + end + call(app)[2].close + tempfile1.closed.should.equal true + tempfile2.closed.should.equal true + end +end -- cgit v1.2.3-24-ge0c7 From ccc542f19fb8a9b1946471ffef6f0f38e74f4fc5 Mon Sep 17 00:00:00 2001 From: Lenny Marks Date: Thu, 27 Mar 2014 17:24:21 -0400 Subject: Enable cleanup of Tempfiles from multipart form data by default --- lib/rack/server.rb | 6 ++++-- test/spec_server.rb | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index efd89c27..d51a95eb 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -217,14 +217,16 @@ module Rack "deployment" => [ [Rack::ContentLength], [Rack::Chunked], - logging_middleware + logging_middleware, + [Rack::TempfileReaper] ], "development" => [ [Rack::ContentLength], [Rack::Chunked], logging_middleware, [Rack::ShowExceptions], - [Rack::Lint] + [Rack::Lint], + [Rack::TempfileReaper] ], "none" => [] } diff --git a/test/spec_server.rb b/test/spec_server.rb index 59391681..9a81875a 100644 --- a/test/spec_server.rb +++ b/test/spec_server.rb @@ -40,6 +40,11 @@ describe Rack::Server do server.default_middleware_by_environment['none'].flatten.should.not.include(Rack::ShowExceptions) end + should "include Rack::TempfileReaper in deployment environment" do + server = Rack::Server.new(:app => 'foo') + server.middleware['deployment'].flatten.should.include(Rack::TempfileReaper) + end + should "support CGI" do begin o, ENV["REQUEST_METHOD"] = ENV["REQUEST_METHOD"], 'foo' -- cgit v1.2.3-24-ge0c7 From b0a19a2d034e50b65c90af778f1f8ad79c329cf2 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 15 Jul 2014 12:07:20 -0300 Subject: Use latest 2.1 on Travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 588bf0ea..4bea79b1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,7 @@ rvm: - 1.9.2 - 1.9.3 - 2.0.0 - - 2.1.0 + - 2.1 - ruby-head - rbx - jruby -- cgit v1.2.3-24-ge0c7 From d2e42932d41f460f0271afc3fb1b7568a3fe9d91 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 15 Jul 2014 12:08:05 -0300 Subject: Fix rbx settings for Travis --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4bea79b1..9a079a3e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,12 +11,12 @@ rvm: - 2.0.0 - 2.1 - ruby-head - - rbx + - rbx-2 - jruby - ree matrix: allow_failures: - - rvm: rbx + - rvm: rbx-2 notifications: email: false irc: "irc.freenode.org#rack" -- cgit v1.2.3-24-ge0c7 From 4a02eceb729d6901cec85e74cc0296cc9a08017f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 15 Jul 2014 12:29:53 -0300 Subject: Remove rbx from Travis' allow_failures --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9a079a3e..aa2eca43 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,9 +14,6 @@ rvm: - rbx-2 - jruby - ree -matrix: - allow_failures: - - rvm: rbx-2 notifications: email: false irc: "irc.freenode.org#rack" -- cgit v1.2.3-24-ge0c7 From f667205a8f45cf11c22d8e6eb50d09c8df3bef07 Mon Sep 17 00:00:00 2001 From: Rafael Mendonça França Date: Fri, 18 Jul 2014 15:27:36 -0300 Subject: default_middleware_by_environment should always returns empty array for unknown keys The current behaviour break rails server for any environment different of development. This behaviour was removed at 704be37e with a warning that if this behavior is supported tests need to be added. The tests are already there and they were not failing because "none" environment were added to the hash. This environment is not a valid environment and it is on the tests exactly to assert the behaviour that default_middleware_by_environment always return an empty array for unknown keys. --- lib/rack/server.rb | 34 +++++++++++++++++----------------- test/spec_server.rb | 7 ++++++- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/lib/rack/server.rb b/lib/rack/server.rb index d51a95eb..d2f0b954 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -213,23 +213,23 @@ module Rack end def default_middleware_by_environment - { - "deployment" => [ - [Rack::ContentLength], - [Rack::Chunked], - logging_middleware, - [Rack::TempfileReaper] - ], - "development" => [ - [Rack::ContentLength], - [Rack::Chunked], - logging_middleware, - [Rack::ShowExceptions], - [Rack::Lint], - [Rack::TempfileReaper] - ], - "none" => [] - } + m = Hash.new {|h,k| h[k] = []} + m["deployment"] = [ + [Rack::ContentLength], + [Rack::Chunked], + logging_middleware, + [Rack::TempfileReaper] + ] + m["development"] = [ + [Rack::ContentLength], + [Rack::Chunked], + logging_middleware, + [Rack::ShowExceptions], + [Rack::Lint], + [Rack::TempfileReaper] + ] + + m end # Aliased for backwards-compatibility diff --git a/test/spec_server.rb b/test/spec_server.rb index 9a81875a..08530dfa 100644 --- a/test/spec_server.rb +++ b/test/spec_server.rb @@ -40,6 +40,11 @@ describe Rack::Server do server.default_middleware_by_environment['none'].flatten.should.not.include(Rack::ShowExceptions) end + should "always return an empty array for unknown environments" do + server = Rack::Server.new(:app => 'foo') + server.default_middleware_by_environment['production'].should.equal [] + end + should "include Rack::TempfileReaper in deployment environment" do server = Rack::Server.new(:app => 'foo') server.middleware['deployment'].flatten.should.include(Rack::TempfileReaper) @@ -72,7 +77,7 @@ describe Rack::Server do FileUtils.rm pidfile server = Rack::Server.new( :app => app, - :environment => 'none', + :environment => 'production', :pid => pidfile, :Port => TCPServer.open('127.0.0.1', 0){|s| s.addr[1] }, :Host => '127.0.0.1', -- cgit v1.2.3-24-ge0c7 From 96fd002b917ac504e2416f48d280c02538e54b03 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 11:49:49 -0700 Subject: Undo test that falsely exemplifies production env --- test/spec_server.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec_server.rb b/test/spec_server.rb index 08530dfa..01b4f562 100644 --- a/test/spec_server.rb +++ b/test/spec_server.rb @@ -77,7 +77,7 @@ describe Rack::Server do FileUtils.rm pidfile server = Rack::Server.new( :app => app, - :environment => 'production', + :environment => 'none', :pid => pidfile, :Port => TCPServer.open('127.0.0.1', 0){|s| s.addr[1] }, :Host => '127.0.0.1', -- cgit v1.2.3-24-ge0c7 From 1aff92c9ac533dac8cc0486443d1d88cb78ad5cc Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 12:45:26 -0700 Subject: multipart content-type match now case insensitive Closes #676 This may not be 100% to spec, but it is important for compatibility and should not cause regressions. --- lib/rack/multipart.rb | 2 +- test/spec_multipart.rb | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb index d67ff051..7a44c4d4 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -9,7 +9,7 @@ module Rack EOL = "\r\n" MULTIPART_BOUNDARY = "AaB03x" - MULTIPART = %r|\Amultipart/.*boundary=\"?([^\";,]+)\"?|n + MULTIPART = %r|\Amultipart/.*boundary=\"?([^\";,]+)\"?|ni TOKEN = /[^\s()<>,;:\\"\/\[\]?=]+/ CONDISP = /Content-Disposition:\s*#{TOKEN}\s*/i DISPPARM = /;\s*(#{TOKEN})=("(?:\\"|[^"])*"|#{TOKEN})/ diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 069dc4d2..8b453bea 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -502,4 +502,28 @@ contents\r params["file"][:filename].should.equal('long' * 100) end + should "support mixed case metadata" do + file = multipart_file(:text) + data = File.open(file, 'rb') { |io| io.read } + + type = "Multipart/Form-Data; Boundary=AaB03x" + length = data.respond_to?(:bytesize) ? data.bytesize : data.size + + e = { "CONTENT_TYPE" => type, + "CONTENT_LENGTH" => length.to_s, + :input => StringIO.new(data) } + + env = Rack::MockRequest.env_for("/", e) + params = Rack::Multipart.parse_multipart(env) + params["submit-name"].should.equal "Larry" + params["submit-name-with-content"].should.equal "Berry" + params["files"][:type].should.equal "text/plain" + params["files"][:filename].should.equal "file1.txt" + params["files"][:head].should.equal "Content-Disposition: form-data; " + + "name=\"files\"; filename=\"file1.txt\"\r\n" + + "Content-Type: text/plain\r\n" + params["files"][:name].should.equal "files" + params["files"][:tempfile].read.should.equal "contents" + end + end -- cgit v1.2.3-24-ge0c7 From 2683d278913529400d13fd32b2a85e9eaa7f272c Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 12:48:47 -0700 Subject: Rack::Multipart::UploadedFile has file extensions Tempfiles created for file uploads now have file extensions. This enables certain use cases for users that are using file extensions as heuristics. Closes #690 --- lib/rack/multipart/uploaded_file.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rack/multipart/uploaded_file.rb b/lib/rack/multipart/uploaded_file.rb index 11932b17..1b56ad75 100644 --- a/lib/rack/multipart/uploaded_file.rb +++ b/lib/rack/multipart/uploaded_file.rb @@ -11,7 +11,7 @@ module Rack raise "#{path} file does not exist" unless ::File.exist?(path) @content_type = content_type @original_filename = ::File.basename(path) - @tempfile = Tempfile.new(@original_filename) + @tempfile = Tempfile.new([@original_filename, ::File.extname(path)]) @tempfile.set_encoding(Encoding::BINARY) if @tempfile.respond_to?(:set_encoding) @tempfile.binmode if binary FileUtils.copy_file(path, @tempfile.path) @@ -31,4 +31,4 @@ module Rack end end end -end \ No newline at end of file +end -- cgit v1.2.3-24-ge0c7 From 28ba782f06d97085862f72227815d492fcfc83c9 Mon Sep 17 00:00:00 2001 From: Rafael Mendonça França Date: Fri, 18 Jul 2014 17:56:51 -0300 Subject: Fix media_type_params when Content-Type parameters contains quoted-strings According RFC 2616, the Content-Type parameters value can be even a token or a quoted-string: media-type = type "/" subtype *( ";" parameter ) type = token subtype = token parameter = attribute "=" value attribute = token value = token | quoted-string quoted-string = ( <"> *(qdtext | quoted-pair ) <"> ) qdtext = >" quoted-pair = "\" CHAR --- lib/rack/request.rb | 2 +- test/spec_request.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 5a446acd..ac94026c 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -52,7 +52,7 @@ module Rack return {} if content_type.nil? Hash[*content_type.split(/\s*[;,]\s*/)[1..-1]. collect { |s| s.split('=', 2) }. - map { |k,v| [k.downcase, v] }.flatten] + map { |k,v| [k.downcase, v.gsub(/\A"|"\z/, "")] }.flatten] end # The character set of the request body if a "charset" media type diff --git a/test/spec_request.rb b/test/spec_request.rb index e5ec254e..09a308a6 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -611,7 +611,7 @@ describe Rack::Request do should "handle multiple media type parameters" do req = Rack::Request.new \ Rack::MockRequest.env_for("/", - "CONTENT_TYPE" => 'text/plain; foo=BAR,baz=bizzle dizzle;BLING=bam') + "CONTENT_TYPE" => 'text/plain; foo=BAR,baz=bizzle dizzle;BLING=bam;blong="boo";zump="zoo\"o"') req.should.not.be.form_data req.media_type_params.should.include 'foo' req.media_type_params['foo'].should.equal 'BAR' @@ -620,6 +620,8 @@ describe Rack::Request do req.media_type_params.should.not.include 'BLING' req.media_type_params.should.include 'bling' req.media_type_params['bling'].should.equal 'bam' + req.media_type_params['blong'].should.equal 'boo' + req.media_type_params['zump'].should.equal 'zoo\"o' end should "parse with junk before boundry" do -- cgit v1.2.3-24-ge0c7 From 586cfba23fc0365b252fd0f16317c7d3cf7211cd Mon Sep 17 00:00:00 2001 From: Rafael Mendonça França Date: Fri, 18 Jul 2014 17:25:49 -0300 Subject: Raise specific exception if the parameters are invalid There are some cases where we try to parse the parameters but it fails with ArgumentError. 1. When the parameters come from the query string and contains invalid UTF-8 characters. In this case we raise ArgumentError when trying to match the key portion of parameters with a regexp. 2. When the parameters come from the request body and the query string contains invalid % encoded string. In this case we raise ArgumentError when calling URI.decode_www_form_component. Now both cases raise a InvalidParameterError what inherit from TypeError and we can catch this exception to show a bad request for users instead of an internal server error. --- lib/rack/utils.rb | 7 +++++++ test/spec_request.rb | 12 ++++++++++++ test/spec_utils.rb | 6 ++++++ 3 files changed, 25 insertions(+) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 27ac956f..7f30a55a 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -26,6 +26,11 @@ module Rack # parameters (parsed by parse_nested_query) contain conflicting types. class ParameterTypeError < TypeError; end + # InvalidParameterError is the error that is raised when incoming structural + # parameters (parsed by parse_nested_query) contain invalid format or byte + # sequence. + class InvalidParameterError < TypeError; end + # URI escapes. (CGI style space to +) def escape(s) URI.encode_www_form_component(s) @@ -106,6 +111,8 @@ module Rack end return params.to_params_hash + rescue ArgumentError => e + raise InvalidParameterError, e.message end module_function :parse_nested_query diff --git a/test/spec_request.rb b/test/spec_request.rb index e5ec254e..2b4b3c0b 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -177,6 +177,18 @@ describe Rack::Request do req.params.should.equal req.GET.merge(req.POST) end + should "raise if input params has invalid %-encoding" do + mr = Rack::MockRequest.env_for("/?foo=quux", + "REQUEST_METHOD" => 'POST', + :input => "a%=1" + ) + req = Rack::Request.new mr + + lambda { req.POST }. + should.raise(Rack::Utils::InvalidParameterError). + message.should.equal "invalid %-encoding (a%)" + end + should "raise if rack.input is missing" do req = Rack::Request.new({}) lambda { req.POST }.should.raise(RuntimeError) diff --git a/test/spec_utils.rb b/test/spec_utils.rb index 6391f951..4b989db7 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -223,6 +223,12 @@ describe Rack::Utils do lambda { Rack::Utils.parse_nested_query("x[y]=1&x[y][][w]=2") }. should.raise(Rack::Utils::ParameterTypeError). message.should.equal "expected Array (got String) for param `y'" + + if RUBY_VERSION.to_f > 1.9 + lambda { Rack::Utils.parse_nested_query("foo%81E=1") }. + should.raise(Rack::Utils::InvalidParameterError). + message.should.equal "invalid byte sequence in UTF-8" + end end should "build query strings correctly" do -- cgit v1.2.3-24-ge0c7 From 2754224570978756fdbaf3f1842a229d3ab95913 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 15:28:20 -0700 Subject: UrlMap: Enable case-insensitive domain matching Closes #668. Supersedes #668, because it dropped port matching. --- lib/rack/urlmap.rb | 20 +++++++++++++++++--- test/spec_urlmap.rb | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/lib/rack/urlmap.rb b/lib/rack/urlmap.rb index d301ce9b..df9e7d6d 100644 --- a/lib/rack/urlmap.rb +++ b/lib/rack/urlmap.rb @@ -48,9 +48,10 @@ module Rack sPort = env['SERVER_PORT'] @mapping.each do |host, location, match, app| - unless hHost == host \ - || sName == host \ - || (!host && (hHost == sName || hHost == sName+':'+sPort)) + unless casecmp?(hHost, host) \ + || casecmp?(sName, host) \ + || (!host && (casecmp?(hHost, sName) || + casecmp?(hHost, sName+':'+sPort))) next end @@ -71,6 +72,19 @@ module Rack env['PATH_INFO'] = path env['SCRIPT_NAME'] = script_name end + + private + def casecmp?(v1, v2) + # if both nil, or they're the same string + return true if v1 == v2 + + # if either are nil... (but they're not the same) + return false if v1.nil? + return false if v2.nil? + + # otherwise check they're not case-insensitive the same + v1.casecmp(v2).zero? + end end end diff --git a/test/spec_urlmap.rb b/test/spec_urlmap.rb index 316c7254..2ef41cdc 100644 --- a/test/spec_urlmap.rb +++ b/test/spec_urlmap.rb @@ -210,4 +210,27 @@ describe Rack::URLMap do res["X-PathInfo"].should.equal "/http://example.org/bar" res["X-ScriptName"].should.equal "" end + + should "not be case sensitive with hosts" do + map = Rack::Lint.new(Rack::URLMap.new("http://example.org/" => lambda { |env| + [200, + { "Content-Type" => "text/plain", + "X-Position" => "root", + "X-PathInfo" => env["PATH_INFO"], + "X-ScriptName" => env["SCRIPT_NAME"] + }, [""]]} + )) + + res = Rack::MockRequest.new(map).get("http://example.org/") + res.should.be.ok + res["X-Position"].should.equal "root" + res["X-PathInfo"].should.equal "/" + res["X-ScriptName"].should.equal "" + + res = Rack::MockRequest.new(map).get("http://EXAMPLE.ORG/") + res.should.be.ok + res["X-Position"].should.equal "root" + res["X-PathInfo"].should.equal "/" + res["X-ScriptName"].should.equal "" + end end -- cgit v1.2.3-24-ge0c7 From 837b0e6ccf737b90c20977c8d6ac8bb5fbac2e3e Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 15:41:42 -0700 Subject: correct weird case regression from #714 --- lib/rack/request.rb | 11 ++++++++++- test/spec_request.rb | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index ac94026c..52ea652c 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -52,7 +52,7 @@ module Rack return {} if content_type.nil? Hash[*content_type.split(/\s*[;,]\s*/)[1..-1]. collect { |s| s.split('=', 2) }. - map { |k,v| [k.downcase, v.gsub(/\A"|"\z/, "")] }.flatten] + map { |k,v| [k.downcase, strip_doublequotes(v)] }.flatten] end # The character set of the request body if a "charset" media type @@ -383,5 +383,14 @@ module Rack [attribute, quality] end end + + private + def strip_doublequotes(s) + if s[0] == ?" && s[-1] == ?" + s[1..-2] + else + s + end + end end end diff --git a/test/spec_request.rb b/test/spec_request.rb index b2257c7c..8a2b4760 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -623,7 +623,7 @@ describe Rack::Request do should "handle multiple media type parameters" do req = Rack::Request.new \ Rack::MockRequest.env_for("/", - "CONTENT_TYPE" => 'text/plain; foo=BAR,baz=bizzle dizzle;BLING=bam;blong="boo";zump="zoo\"o"') + "CONTENT_TYPE" => 'text/plain; foo=BAR,baz=bizzle dizzle;BLING=bam;blong="boo";zump="zoo\"o";weird=lol"') req.should.not.be.form_data req.media_type_params.should.include 'foo' req.media_type_params['foo'].should.equal 'BAR' @@ -634,6 +634,7 @@ describe Rack::Request do req.media_type_params['bling'].should.equal 'bam' req.media_type_params['blong'].should.equal 'boo' req.media_type_params['zump'].should.equal 'zoo\"o' + req.media_type_params['weird'].should.equal 'lol"' end should "parse with junk before boundry" do -- cgit v1.2.3-24-ge0c7 From 975ccac7e56dcd765cb102016b97ef13d15feba8 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 15:43:21 -0700 Subject: Fix parent type API regression introduced in #713 --- lib/rack/utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 7f30a55a..53303995 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -29,7 +29,7 @@ module Rack # InvalidParameterError is the error that is raised when incoming structural # parameters (parsed by parse_nested_query) contain invalid format or byte # sequence. - class InvalidParameterError < TypeError; end + class InvalidParameterError < ArgumentError; end # URI escapes. (CGI style space to +) def escape(s) -- cgit v1.2.3-24-ge0c7 From 83155c586521455b43a9b05523b0e6d5318ba086 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 15:58:59 -0700 Subject: multipart/form-data with files with no input name Closes #695. This is not technically specification correct, but it is included for compatibility with bad clients. --- lib/rack/multipart/parser.rb | 4 ++++ test/multipart/filename_and_no_name | 6 ++++++ test/spec_multipart.rb | 12 ++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 test/multipart/filename_and_no_name diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 00e59ba7..22f9734b 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -112,6 +112,10 @@ module Rack filename = get_filename(head) + if name.nil? || name.empty? && filename + name = filename + end + if filename (@env['rack.tempfiles'] ||= []) << body = Tempfile.new("RackMultipart") body.binmode if body.respond_to?(:binmode) diff --git a/test/multipart/filename_and_no_name b/test/multipart/filename_and_no_name new file mode 100644 index 00000000..00d58153 --- /dev/null +++ b/test/multipart/filename_and_no_name @@ -0,0 +1,6 @@ +--AaB03x +Content-Disposition: form-data; filename="file1.txt" +Content-Type: text/plain + +contents +--AaB03x-- diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 8b453bea..2acb6e0d 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -153,6 +153,18 @@ describe Rack::Multipart do params["files"][:tempfile].read.should.equal "contents" end + should "parse multipart upload with text file with no name field" do + env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_and_no_name)) + params = Rack::Multipart.parse_multipart(env) + params["file1.txt"][:type].should.equal "text/plain" + params["file1.txt"][:filename].should.equal "file1.txt" + params["file1.txt"][:head].should.equal "Content-Disposition: form-data; " + + "filename=\"file1.txt\"\r\n" + + "Content-Type: text/plain\r\n" + params["file1.txt"][:name].should.equal "file1.txt" + params["file1.txt"][:tempfile].read.should.equal "contents" + end + should "parse multipart upload with nested parameters" do env = Rack::MockRequest.env_for("/", multipart_fixture(:nested)) params = Rack::Multipart.parse_multipart(env) -- cgit v1.2.3-24-ge0c7 From 9269d22c56762076e3b775ea65ec1d40494d2402 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 18 Jul 2014 16:07:27 -0700 Subject: support empty string multipart filename Closes #702. Enables support for IE11. --- lib/rack/multipart/parser.rb | 4 ++++ test/multipart/ie11 | 6 ++++++ test/spec_multipart.rb | 22 +++++++++++++++++++--- 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 test/multipart/ie11 diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 22f9734b..324c7d8c 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -116,6 +116,10 @@ module Rack name = filename end + if filename && filename.empty? + filename = name + end + if filename (@env['rack.tempfiles'] ||= []) << body = Tempfile.new("RackMultipart") body.binmode if body.respond_to?(:binmode) diff --git a/test/multipart/ie11 b/test/multipart/ie11 new file mode 100644 index 00000000..6c3da47f --- /dev/null +++ b/test/multipart/ie11 @@ -0,0 +1,6 @@ +--AaB03x +Content-Disposition: form-data; name="files"; filename="" +Content-Type: text/plain + +contents +--AaB03x-- diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 2acb6e0d..959a5cff 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -230,12 +230,15 @@ describe Rack::Multipart do params["files"][:tempfile].read.should.equal "contents" end - should "not include file params if no file was selected" do + # n.b. this case used to be "do not include", but because ie11 now does this + # for selected files, that can no longer be done. It was decided that not + # losing data is better, and no browser is documented with this behavior. + should "include file params if no file was selected" do env = Rack::MockRequest.env_for("/", multipart_fixture(:none)) params = Rack::Multipart.parse_multipart(env) params["submit-name"].should.equal "Larry" - params["files"].should.equal nil - params.keys.should.not.include "files" + params["files"].should.not.equal nil + params.keys.should.include "files" end should "parse multipart/mixed" do @@ -267,6 +270,19 @@ describe Rack::Multipart do params["files"][:tempfile].read.should.equal "contents" end + should "parse IE11 multipart upload" do + env = Rack::MockRequest.env_for("/", multipart_fixture(:ie11)) + params = Rack::Multipart.parse_multipart(env) + params["files"][:type].should.equal "text/plain" + params["files"][:filename].should.equal "files" + params["files"][:head].should.equal "Content-Disposition: form-data; " + + "name=\"files\"; " + + 'filename=""' + + "\r\nContent-Type: text/plain\r\n" + params["files"][:name].should.equal "files" + params["files"][:tempfile].read.should.equal "contents" + end + should "parse filename and modification param" do env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_and_modification_param)) params = Rack::Multipart.parse_multipart(env) -- cgit v1.2.3-24-ge0c7 From 5a9ffeb77c613e3611262f1182284f1d7df393a8 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sat, 19 Jul 2014 21:24:11 -0700 Subject: Revert "support empty string multipart filename" This reverts commit 9269d22c56762076e3b775ea65ec1d40494d2402. --- lib/rack/multipart/parser.rb | 4 ---- test/multipart/ie11 | 6 ------ test/spec_multipart.rb | 22 +++------------------- 3 files changed, 3 insertions(+), 29 deletions(-) delete mode 100644 test/multipart/ie11 diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 324c7d8c..22f9734b 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -116,10 +116,6 @@ module Rack name = filename end - if filename && filename.empty? - filename = name - end - if filename (@env['rack.tempfiles'] ||= []) << body = Tempfile.new("RackMultipart") body.binmode if body.respond_to?(:binmode) diff --git a/test/multipart/ie11 b/test/multipart/ie11 deleted file mode 100644 index 6c3da47f..00000000 --- a/test/multipart/ie11 +++ /dev/null @@ -1,6 +0,0 @@ ---AaB03x -Content-Disposition: form-data; name="files"; filename="" -Content-Type: text/plain - -contents ---AaB03x-- diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 959a5cff..2acb6e0d 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -230,15 +230,12 @@ describe Rack::Multipart do params["files"][:tempfile].read.should.equal "contents" end - # n.b. this case used to be "do not include", but because ie11 now does this - # for selected files, that can no longer be done. It was decided that not - # losing data is better, and no browser is documented with this behavior. - should "include file params if no file was selected" do + should "not include file params if no file was selected" do env = Rack::MockRequest.env_for("/", multipart_fixture(:none)) params = Rack::Multipart.parse_multipart(env) params["submit-name"].should.equal "Larry" - params["files"].should.not.equal nil - params.keys.should.include "files" + params["files"].should.equal nil + params.keys.should.not.include "files" end should "parse multipart/mixed" do @@ -270,19 +267,6 @@ describe Rack::Multipart do params["files"][:tempfile].read.should.equal "contents" end - should "parse IE11 multipart upload" do - env = Rack::MockRequest.env_for("/", multipart_fixture(:ie11)) - params = Rack::Multipart.parse_multipart(env) - params["files"][:type].should.equal "text/plain" - params["files"][:filename].should.equal "files" - params["files"][:head].should.equal "Content-Disposition: form-data; " + - "name=\"files\"; " + - 'filename=""' + - "\r\nContent-Type: text/plain\r\n" - params["files"][:name].should.equal "files" - params["files"][:tempfile].read.should.equal "contents" - end - should "parse filename and modification param" do env = Rack::MockRequest.env_for("/", multipart_fixture(:filename_and_modification_param)) params = Rack::Multipart.parse_multipart(env) -- cgit v1.2.3-24-ge0c7 From 9a92526024358aaa0b3e20a145f8e2da2c409dee Mon Sep 17 00:00:00 2001 From: Fran Casas Date: Fri, 1 Aug 2014 08:57:21 +0200 Subject: Implement full Logger interface on NullLogger NullLogger implements only a small subset of the Logger public interface. In case you need to hack with your loggers a bit more than usual the NullLogger is useless since it will raise NoMethodError all the time. --- lib/rack/nulllogger.rb | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/rack/nulllogger.rb b/lib/rack/nulllogger.rb index 77fb637d..2d5a2c97 100644 --- a/lib/rack/nulllogger.rb +++ b/lib/rack/nulllogger.rb @@ -9,10 +9,29 @@ module Rack @app.call(env) end - def info(progname = nil, &block); end + def info(progname = nil, &block); end def debug(progname = nil, &block); end - def warn(progname = nil, &block); end + def warn(progname = nil, &block); end def error(progname = nil, &block); end def fatal(progname = nil, &block); end + def unknown(progname = nil, &block); end + def info? ; end + def debug? ; end + def warn? ; end + def error? ; end + def fatal? ; end + def level ; end + def progname ; end + def datetime_format ; end + def formatter ; end + def sev_threshold ; end + def level=(level); end + def progname=(progname); end + def datetime_format=(datetime_format); end + def formatter=(formatter); end + def sev_threshold=(sev_threshold); end + def close ; end + def add(severity, message = nil, progname = nil, &block); end + def <<(msg); end end end -- cgit v1.2.3-24-ge0c7 From 0f8cd0544cf40ef91a55d7762bcbaf3a360f97f9 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 3 Aug 2014 13:42:49 -0300 Subject: Fix yet another body close bug in Rack::Deflater --- lib/rack/deflater.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rack/deflater.rb b/lib/rack/deflater.rb index 2e55f97e..36cfb7af 100644 --- a/lib/rack/deflater.rb +++ b/lib/rack/deflater.rb @@ -58,9 +58,9 @@ module Rack when "identity" [status, headers, body] when nil - body.close if body.respond_to?(:close) message = "An acceptable encoding for the requested resource #{request.fullpath} could not be found." - [406, {"Content-Type" => "text/plain", "Content-Length" => message.length.to_s}, [message]] + bp = Rack::BodyProxy.new([message]) { body.close if body.respond_to?(:close) } + [406, {"Content-Type" => "text/plain", "Content-Length" => message.length.to_s}, bp] end end -- cgit v1.2.3-24-ge0c7 From 12528d4567d8e6c1c7e9422fee6cd8b43c4389bf Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 3 Aug 2014 13:53:20 -0300 Subject: Rack::ETag correctly marks etags as Weak Closes #681. --- lib/rack/etag.rb | 2 +- test/spec_etag.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rack/etag.rb b/lib/rack/etag.rb index 99a1a4c0..fefe671f 100644 --- a/lib/rack/etag.rb +++ b/lib/rack/etag.rb @@ -28,7 +28,7 @@ module Rack body = Rack::BodyProxy.new(new_body) do original_body.close if original_body.respond_to?(:close) end - headers['ETag'] = %("#{digest}") if digest + headers['ETag'] = %(W/"#{digest}") if digest end unless headers['Cache-Control'] diff --git a/test/spec_etag.rb b/test/spec_etag.rb index b8b8b637..c075d9d0 100644 --- a/test/spec_etag.rb +++ b/test/spec_etag.rb @@ -21,13 +21,13 @@ describe Rack::ETag do should "set ETag if none is set if status is 200" do app = lambda { |env| [200, {'Content-Type' => 'text/plain'}, ["Hello, World!"]] } response = etag(app).call(request) - response[1]['ETag'].should.equal "\"65a8e27d8879283831b664bd8b7f0ad4\"" + response[1]['ETag'].should.equal "W/\"65a8e27d8879283831b664bd8b7f0ad4\"" end should "set ETag if none is set if status is 201" do app = lambda { |env| [201, {'Content-Type' => 'text/plain'}, ["Hello, World!"]] } response = etag(app).call(request) - response[1]['ETag'].should.equal "\"65a8e27d8879283831b664bd8b7f0ad4\"" + response[1]['ETag'].should.equal "W/\"65a8e27d8879283831b664bd8b7f0ad4\"" end should "set Cache-Control to 'max-age=0, private, must-revalidate' (default) if none is set" do -- cgit v1.2.3-24-ge0c7 From c34b5790af6733571522adf7cc91e9e85c4a3b64 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Sun, 3 Aug 2014 14:15:10 -0300 Subject: build_nested_query includes integer values Patch from @spastorino Closes #557 --- lib/rack/utils.rb | 6 +++--- test/spec_utils.rb | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 53303995..d5a19914 100644 --- a/lib/rack/utils.rb +++ b/lib/rack/utils.rb @@ -179,11 +179,11 @@ module Rack value.map { |k, v| build_nested_query(v, prefix ? "#{prefix}[#{escape(k)}]" : escape(k)) }.join("&") - when String + when nil + prefix + else raise ArgumentError, "value must be a Hash" if prefix.nil? "#{prefix}=#{escape(value)}" - else - prefix end end module_function :build_nested_query diff --git a/test/spec_utils.rb b/test/spec_utils.rb index 4b989db7..53311fd2 100644 --- a/test/spec_utils.rb +++ b/test/spec_utils.rb @@ -248,6 +248,8 @@ describe Rack::Utils do Rack::Utils.build_nested_query("foo" => "1", "bar" => "2"). should.be equal_query_to("foo=1&bar=2") + Rack::Utils.build_nested_query("foo" => 1, "bar" => 2). + should.be equal_query_to("foo=1&bar=2") Rack::Utils.build_nested_query("my weird field" => "q1!2\"'w$5&7/z8)?"). should.be equal_query_to("my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F") -- cgit v1.2.3-24-ge0c7