content_length.rb incorrectly compares string with int, causing later exception

content_length.rb incorrectly compares string with int, causing later exception

[Larry Siden]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

I came across an issue that caused bytesize() to throw an exception because
it was being asked to return the size of nil when an HTTP response has
status is 204 (no content).

The test on line 16 of content_length.rb should prevent it from ever
reaching this code, but neglects to convert status (an instance of String)
to an int before checking for inclusion in the the Set instance named by the
constant STATUS_WITH_NO_ENTITY_BODY, which contains integers.

Please accept the following patch:

diff --git a/lib/rack/content_length.rb b/lib/rack/content_length.rb
index 1e56d43..ba72ef2 100644
--- a/lib/rack/content_length.rb
+++ b/lib/rack/content_length.rb
@@ -13,7 +13,7 @@ module Rack
       status, headers, body = @app.call(env)
       headers = HeaderHash.new(headers)

-      if !STATUS_WITH_NO_ENTITY_BODY.include?(status) &&
+      if !STATUS_WITH_NO_ENTITY_BODY.include?(status.to_i) &&
          !headers['Content-Length'] &&
          !headers['Transfer-Encoding'] &&
          (body.respond_to?(:to_ary) || body.respond_to?(:to_str))

I hope someone will contact me to let me know whether this has been accepted
and what release it will appear in.  Thank you and G-d bless.

Larry Siden, 734-926-9614, http://umich.edu/~lsiden

The United States is a nation of laws, badly written and randomly enforced.
--Frank Zappa 1940-1993

[-- Attachment #2: Type: text/html, Size: 1732 bytes --]

Re: content_length.rb incorrectly compares string with int, causing later exception

[Ryan Tomayko]

On Mon, Mar 8, 2010 at 8:20 PM, Larry Siden <lsiden@gmail.com> wrote:
> I came across an issue that caused bytesize() to throw an exception because
> it was being asked to return the size of nil when an HTTP response has
> status is 204 (no content).
> The test on line 16 of content_length.rb should prevent it from ever
> reaching this code, but neglects to convert status (an instance of String)
> to an int before checking for inclusion in the the Set instance named by the
> constant STATUS_WITH_NO_ENTITY_BODY, which contains integers.
> Please accept the following patch:
> diff --git a/lib/rack/content_length.rb b/lib/rack/content_length.rb
> index 1e56d43..ba72ef2 100644
> --- a/lib/rack/content_length.rb
> +++ b/lib/rack/content_length.rb
> @@ -13,7 +13,7 @@ module Rack
>        status, headers, body = @app.call(env)
>        headers = HeaderHash.new(headers)
>
> -      if !STATUS_WITH_NO_ENTITY_BODY.include?(status) &&
> +      if !STATUS_WITH_NO_ENTITY_BODY.include?(status.to_i) &&
>           !headers['Content-Length'] &&
>           !headers['Transfer-Encoding'] &&
>           (body.respond_to?(:to_ary) || body.respond_to?(:to_str))
> I hope someone will contact me to let me know whether this has been accepted
> and what release it will appear in.  Thank you and G-d bless.
> Larry Siden, 734-926-9614, http://umich.edu/~lsiden

Applied.

Thanks,
Ryan

Re: content_length.rb incorrectly compares string with int, causing later exception

[Larry Siden]

[-- Attachment #1.1: Type: text/plain, Size: 1757 bytes --]

Hey, I found out that what I previously submitted wasn't enough.  New
version is in attachment.  Please commit if acceptable.

Larry Siden, 734-926-9614, http://umich.edu/~lsiden

The United States is a nation of laws, badly written and randomly enforced.
--Frank Zappa 1940-1993


On Mon, Mar 8, 2010 at 11:20 PM, Larry Siden <lsiden@gmail.com> wrote:

> I came across an issue that caused bytesize() to throw an exception because
> it was being asked to return the size of nil when an HTTP response has
> status is 204 (no content).
>
> The test on line 16 of content_length.rb should prevent it from ever
> reaching this code, but neglects to convert status (an instance of String)
> to an int before checking for inclusion in the the Set instance named by the
> constant STATUS_WITH_NO_ENTITY_BODY, which contains integers.
>
> Please accept the following patch:
>
> diff --git a/lib/rack/content_length.rb b/lib/rack/content_length.rb
> index 1e56d43..ba72ef2 100644
> --- a/lib/rack/content_length.rb
> +++ b/lib/rack/content_length.rb
> @@ -13,7 +13,7 @@ module Rack
>        status, headers, body = @app.call(env)
>        headers = HeaderHash.new(headers)
>
> -      if !STATUS_WITH_NO_ENTITY_BODY.include?(status) &&
> +      if !STATUS_WITH_NO_ENTITY_BODY.include?(status.to_i) &&
>           !headers['Content-Length'] &&
>           !headers['Transfer-Encoding'] &&
>           (body.respond_to?(:to_ary) || body.respond_to?(:to_str))
>
> I hope someone will contact me to let me know whether this has been
> accepted and what release it will appear in.  Thank you and G-d bless.
>
> Larry Siden, 734-926-9614, http://umich.edu/~lsiden
>
> The United States is a nation of laws, badly written and randomly enforced.
> --Frank Zappa 1940-1993
>

[-- Attachment #1.2: Type: text/html, Size: 2424 bytes --]

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1641 bytes --]

diff -ur rack-rack-39ea53f/lib/rack/content_length.rb rack-1.1.0/lib/rack/content_length.rb
--- rack-rack-39ea53f/lib/rack/content_length.rb	2010-01-03 14:21:42.000000000 -0500
+++ rack-1.1.0/lib/rack/content_length.rb	2010-03-13 20:25:44.000000000 -0500
@@ -13,7 +13,7 @@
       status, headers, body = @app.call(env)
       headers = HeaderHash.new(headers)
 
-      if !STATUS_WITH_NO_ENTITY_BODY.include?(status) &&
+      if !STATUS_WITH_NO_ENTITY_BODY.include?(status.to_i) &&
          !headers['Content-Length'] &&
          !headers['Transfer-Encoding'] &&
          (body.respond_to?(:to_ary) || body.respond_to?(:to_str))
diff -ur rack-rack-39ea53f/lib/rack/handler/webrick.rb rack-1.1.0/lib/rack/handler/webrick.rb
--- rack-rack-39ea53f/lib/rack/handler/webrick.rb	2010-01-03 14:21:42.000000000 -0500
+++ rack-1.1.0/lib/rack/handler/webrick.rb	2010-03-13 20:35:26.000000000 -0500
@@ -1,10 +1,13 @@
 require 'webrick'
 require 'stringio'
 require 'rack/content_length'
+require 'rack/utils'
 
 module Rack
   module Handler
     class WEBrick < ::WEBrick::HTTPServlet::AbstractServlet
+			include Rack::Utils
+
       def self.run(app, options={})
         options[:BindAddress] = options.delete(:Host) if options[:Host]
         server = ::WEBrick::HTTPServer.new(options)
@@ -57,9 +60,11 @@
               }
             end
           }
-          body.each { |part|
-            res.body << part
-          }
+					if !STATUS_WITH_NO_ENTITY_BODY.include?(res.status.to_i) then
+						body.each { |part|
+							res.body << part
+						}
+					end
         ensure
           body.close  if body.respond_to? :close
         end

Re: content_length.rb incorrectly compares string with int, causing later exception

[Larry Siden]

[-- Attachment #1.1: Type: text/plain, Size: 2615 bytes --]

This patch to Rack fixes case where web service returns 204 ("Success, no
content") or any other response that does not need to provide a body
payload.  Without this, 'bytesize' will throw an exception.

I submitted this previous patch (http://gist.github.com/388475) on 3/13.
 I'm not sure it was ever followed up.  I had to tdebug same incident again,
thinking that my patch had already been committed (my fault for not checking
diffs).

Please commit this new attached patch (http://gist.github.com/388492) and
notify me.  This is a patch to Commit f10713c.

Thank you,

Larry Siden, 734-926-9614, http://umich.edu/~lsiden

The United States is a nation of laws, badly written and randomly enforced.
--Frank Zappa 1940-1993


On Sat, Mar 13, 2010 at 9:59 PM, Larry Siden <lsiden@gmail.com> wrote:

> Hey, I found out that what I previously submitted wasn't enough.  New
> version is in attachment.  Please commit if acceptable.
>
> Larry Siden, 734-926-9614, http://umich.edu/~lsiden
>
> The United States is a nation of laws, badly written and randomly enforced.
> --Frank Zappa 1940-1993
>
>
> On Mon, Mar 8, 2010 at 11:20 PM, Larry Siden <lsiden@gmail.com> wrote:
>
>> I came across an issue that caused bytesize() to throw an exception
>> because it was being asked to return the size of nil when an HTTP response
>> has status is 204 (no content).
>>
>> The test on line 16 of content_length.rb should prevent it from ever
>> reaching this code, but neglects to convert status (an instance of String)
>> to an int before checking for inclusion in the the Set instance named by the
>> constant STATUS_WITH_NO_ENTITY_BODY, which contains integers.
>>
>> Please accept the following patch:
>>
>> diff --git a/lib/rack/content_length.rb b/lib/rack/content_length.rb
>> index 1e56d43..ba72ef2 100644
>> --- a/lib/rack/content_length.rb
>> +++ b/lib/rack/content_length.rb
>> @@ -13,7 +13,7 @@ module Rack
>>        status, headers, body = @app.call(env)
>>        headers = HeaderHash.new(headers)
>>
>> -      if !STATUS_WITH_NO_ENTITY_BODY.include?(status) &&
>> +      if !STATUS_WITH_NO_ENTITY_BODY.include?(status.to_i) &&
>>           !headers['Content-Length'] &&
>>           !headers['Transfer-Encoding'] &&
>>           (body.respond_to?(:to_ary) || body.respond_to?(:to_str))
>>
>> I hope someone will contact me to let me know whether this has been
>> accepted and what release it will appear in.  Thank you and G-d bless.
>>
>> Larry Siden, 734-926-9614, http://umich.edu/~lsiden
>>
>> The United States is a nation of laws, badly written and randomly
>> enforced.
>> --Frank Zappa 1940-1993
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 3849 bytes --]

[-- Attachment #2: rack.f10713c.patch --]
[-- Type: application/octet-stream, Size: 1124 bytes --]