rack.hijack response header check is case-insensitive?

rack.hijack response header check is case-insensitive?

[Eric Wong]

The following snippet in lib/rack/handler/webrick.rb seems
to imply case-insensitivity by downcasing the comparison
to RACK_HIJACK (defined as "rack.hijack" in lib/rack.rb):

        status, headers, body = @app.call(env)
        begin
          res.status = status.to_i
          headers.each { |k, vs|
            next if k.downcase == RACK_HIJACK

            if k.downcase == "set-cookie"
              res.cookies.concat vs.split("\n")
            else

But I don't see SPEC mentioning case-insensitivity regarding
"rack." stuff...

Then a few lines down in the same method, it does this:

          io_lambda = headers[RACK_HIJACK]

But the server handler has no idea if "headers" here is the
case-insensitive Rack::Utils::HeaderHash or not.  Actually,
SPEC does not even require response headers to respond to a
#[] method, only #each.

I'm pretty sure it's not a real problem, since I doubt anybody
would want to capitalize anything starting with "rack.*".
At least I really hope not; one of the reasons I love Ruby
is capitalization is uncommon.  CamelCaseMakesMyEyesBleed :*<

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: rack.hijack response header check is case-insensitive?

[James Tucker]

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

It is case sensitive. Would welcome a spec bug fix patch to declare it so.
On May 10, 2016 10:04 PM, "Eric Wong" <e@80x24.org> wrote:

> The following snippet in lib/rack/handler/webrick.rb seems
> to imply case-insensitivity by downcasing the comparison
> to RACK_HIJACK (defined as "rack.hijack" in lib/rack.rb):
>
>         status, headers, body = @app.call(env)
>         begin
>           res.status = status.to_i
>           headers.each { |k, vs|
>             next if k.downcase == RACK_HIJACK
>
>             if k.downcase == "set-cookie"
>               res.cookies.concat vs.split("\n")
>             else
>
> But I don't see SPEC mentioning case-insensitivity regarding
> "rack." stuff...
>
> Then a few lines down in the same method, it does this:
>
>           io_lambda = headers[RACK_HIJACK]
>
> But the server handler has no idea if "headers" here is the
> case-insensitive Rack::Utils::HeaderHash or not.  Actually,
> SPEC does not even require response headers to respond to a
> #[] method, only #each.
>
> I'm pretty sure it's not a real problem, since I doubt anybody
> would want to capitalize anything starting with "rack.*".
> At least I really hope not; one of the reasons I love Ruby
> is capitalization is uncommon.  CamelCaseMakesMyEyesBleed :*<
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Rack Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rack-devel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

[PATCH] lint: clarify "rack.hijack" case-sensitivity in response

[Eric Wong]

Based on my inspection of the webrick handler, it was
ambiguous as to whether "rack.hijack" is supposed to
be case-sensitive or not. Thankfully James cleared it up:

http://mid.gmane.org/CABGa_T8ihnKWwguObGCQqF-qBA+_v1YwM1-2tv3y5ShbWo4scw@mail.gmail.com

Note: I manually reapplied the SPEC change in
commit 72185735ad0c3aea4e37ab66b0c370e42180df39
("Fixed link and rack.session's indentation in SPEC")
after regenerating it from lint.rb
---
  James Tucker <jftucker@gmail.com> wrote:
  > It is case sensitive. Would welcome a spec bug fix patch to declare it so.

  If you prefer to pull:

  The following changes since commit 9073125f71afd615091f575d74ec468a0b1b79bf:

    bumping version (2016-05-06 15:51:18 -0500)

  are available in the git repository at:

    git://80x24.org/rack.git lint-case-sens

  for you to fetch changes up to daef95d81b1c3b4dfe3e955d5f34d1be569d86b0:

    lint: clarify "rack.hijack" case-sensitivity in response (2016-05-12 02:06:36 +0000)

  ----------------------------------------------------------------
  Eric Wong (1):
        lint: clarify "rack.hijack" case-sensitivity in response

 SPEC             | 3 ++-
 lib/rack/lint.rb | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/SPEC b/SPEC
index 7e3af40..a5999b3 100644
--- a/SPEC
+++ b/SPEC
@@ -200,7 +200,8 @@ have been sent.
 In order to do this, an application may set the special header
 <tt>rack.hijack</tt> to an object that responds to <tt>call</tt>
 accepting an argument that conforms to the <tt>rack.hijack_io</tt>
-protocol.
+protocol.  Unlike normal response headers, <tt>rack.hijack</tt>
+is case-sensitive.
 After the headers have been sent, and this hijack callback has been
 called, the application is now responsible for the remaining lifecycle
 of the IO. The application is also responsible for maintaining HTTP
diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb
index 54d3782..6eb2476 100644
--- a/lib/rack/lint.rb
+++ b/lib/rack/lint.rb
@@ -573,7 +573,8 @@ def check_hijack_response(headers, env)
       ## In order to do this, an application may set the special header
       ## <tt>rack.hijack</tt> to an object that responds to <tt>call</tt>
       ## accepting an argument that conforms to the <tt>rack.hijack_io</tt>
-      ## protocol.
+      ## protocol.  Unlike normal response headers, <tt>rack.hijack</tt>
+      ## is case-sensitive.
       ##
       ## After the headers have been sent, and this hijack callback has been
       ## called, the application is now responsible for the remaining lifecycle
-- 
EW

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH] webrick: detect partial hijack without hash headers

[Eric Wong]

Response headers need not be a hash according to SPEC,
so grab the io_lambda the first time we iterate through
the headers and avoid an extra hash lookup.
---
  This is related to (but applies independently of) my lint
  clarification for case-sensitivity.

  The following changes since commit 9073125f71afd615091f575d74ec468a0b1b79bf:

    bumping version (2016-05-06 15:51:18 -0500)

  are available in the git repository at:

    git://80x24.org/rack.git webrick-header-each

  for you to fetch changes up to 2c95a6e5bc18ac860ec0f7f7614ffb4aa6ad817d:

    webrick: detect partial hijack without hash headers (2016-05-12 02:23:48 +0000)

  ----------------------------------------------------------------
  Eric Wong (1):
        webrick: detect partial hijack without hash headers

   lib/rack/handler/webrick.rb | 8 ++++----
   test/spec_webrick.rb        | 2 +-
   2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/rack/handler/webrick.rb b/lib/rack/handler/webrick.rb
index 95aa892..d0fcd21 100644
--- a/lib/rack/handler/webrick.rb
+++ b/lib/rack/handler/webrick.rb
@@ -86,10 +86,11 @@ def service(req, res)
         status, headers, body = @app.call(env)
         begin
           res.status = status.to_i
+          io_lambda = nil
           headers.each { |k, vs|
-            next if k.downcase == RACK_HIJACK
-
-            if k.downcase == "set-cookie"
+            if k == RACK_HIJACK
+              io_lambda = vs
+            elsif k.downcase == "set-cookie"
               res.cookies.concat vs.split("\n")
             else
               # Since WEBrick won't accept repeated headers,
@@ -98,7 +99,6 @@ def service(req, res)
             end
           }
 
-          io_lambda = headers[RACK_HIJACK]
           if io_lambda
             rd, wr = IO.pipe
             res.body = rd
diff --git a/test/spec_webrick.rb b/test/spec_webrick.rb
index 9ae6103..4a10c1c 100644
--- a/test/spec_webrick.rb
+++ b/test/spec_webrick.rb
@@ -171,7 +171,7 @@ def is_running?
     Rack::Lint.new(lambda{ |req|
       [
         200,
-        {"rack.hijack" => io_lambda},
+        [ [ "rack.hijack", io_lambda ] ],
         [""]
       ]
     })
-- 
EW

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] webrick: detect partial hijack without hash headers

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> Response headers need not be a hash according to SPEC,
> so grab the io_lambda the first time we iterate through
> the headers and avoid an extra hash lookup.
> ---
>   This is related to (but applies independently of) my lint
>   clarification for case-sensitivity.
> 
>   The following changes since commit 9073125f71afd615091f575d74ec468a0b1b79bf:
> 
>     bumping version (2016-05-06 15:51:18 -0500)
> 
>   are available in the git repository at:
> 
>     git://80x24.org/rack.git webrick-header-each
> 
>   for you to fetch changes up to 2c95a6e5bc18ac860ec0f7f7614ffb4aa6ad817d:
> 
>     webrick: detect partial hijack without hash headers (2016-05-12 02:23:48 +0000)

Ping?  I just got bitten by this.

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] webrick: detect partial hijack without hash headers

[Aaron Patterson]

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

On Wed, Nov 02, 2016 at 12:11:53AM +0000, Eric Wong wrote:
> Eric Wong <e@80x24.org> wrote:
> > Response headers need not be a hash according to SPEC,
> > so grab the io_lambda the first time we iterate through
> > the headers and avoid an extra hash lookup.
> > ---
> >   This is related to (but applies independently of) my lint
> >   clarification for case-sensitivity.
> > 
> >   The following changes since commit 9073125f71afd615091f575d74ec468a0b1b79bf:
> > 
> >     bumping version (2016-05-06 15:51:18 -0500)
> > 
> >   are available in the git repository at:
> > 
> >     git://80x24.org/rack.git webrick-header-each
> > 
> >   for you to fetch changes up to 2c95a6e5bc18ac860ec0f7f7614ffb4aa6ad817d:
> > 
> >     webrick: detect partial hijack without hash headers (2016-05-12 02:23:48 +0000)
> 
> Ping?  I just got bitten by this.

Sorry about that, I must have missed this.  I've applied the patch and
it should be in the next release.  Thank you!

-- 
Aaron Patterson
http://tenderlovemaking.com/

-- 

--- 
You received this message because you are subscribed to the Google Groups "Rack Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rack-devel+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]