rack.hijack response header check is case-insensitive?

44 views
Skip to first unread message

Eric Wong

unread,
May 11, 2016, 1:04:53 AM5/11/16
to rack-...@googlegroups.com
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 :*<

James Tucker

unread,
May 11, 2016, 1:06:29 AM5/11/16
to Rack Development

It is case sensitive. Would welcome a spec bug fix patch to declare it so.

--

---
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+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Eric Wong

unread,
May 11, 2016, 10:28:04 PM5/11/16
to rack-...@googlegroups.com
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-q...@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 <jftu...@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

Eric Wong

unread,
May 11, 2016, 10:31:43 PM5/11/16
to rack-...@googlegroups.com
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

Eric Wong

unread,
Nov 1, 2016, 8:11:56 PM11/1/16
to rack-...@googlegroups.com
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.

Aaron Patterson

unread,
Nov 4, 2016, 8:22:19 PM11/4/16
to Eric Wong, rack-...@googlegroups.com
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/
signature.asc
Reply all
Reply to author
Forward
0 new messages