* [RFC/PATCH] lint: additional response checking/skipping for hijack @ 2013-01-23 0:20 Eric Wong 2013-01-28 22:01 ` James Tucker 0 siblings, 1 reply; 6+ messages in thread From: Eric Wong @ 2013-01-23 0:20 UTC (permalink / raw) To: rack-devel Not a serious patch for now, at least not all of it. I suspect middlewares will break badly if the body.each/body.close checks are enforced. --- lib/rack/lint.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb index 1bc2127..f895772 100644 --- a/lib/rack/lint.rb +++ b/lib/rack/lint.rb @@ -9,6 +9,7 @@ class Lint def initialize(app) @app = app @content_length = nil + @response_hijacked = false end # :stopdoc: @@ -47,6 +48,15 @@ def _call(env) ## and returns an Array of exactly three values: status, headers, @body = @app.call(env) + + # hijacked requests may not give a valid response, do not check them + if env.include?("rack.hijack_io") + # request hijacking implies response hijacking, this will ensure + # the response body raises if body.each or body.close gets called + @response_hijacked = true + return [ status, headers, self ] + end + ## The *status*, check_status status ## the *headers*, @@ -530,6 +540,7 @@ def check_hijack_response(headers, env) headers['rack.hijack'] = proc do |io| original_hijack.call HijackWrapper.new(io) end + @response_hijacked = true else ## ## The special response header <tt>rack.hijack</tt> must only be set @@ -636,6 +647,9 @@ def verify_content_length(bytes) ## === The Body def each + assert("server is not attempting to iterate hijacked response body") { + @response_hijacked == false + } @closed = false bytes = 0 @@ -683,6 +697,9 @@ def each end def close + assert("server is not attempting to close hijacked response") { + @response_hijacked == false + } @closed = true @body.close if @body.respond_to?(:close) end -- Eric Wong ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] lint: additional response checking/skipping for hijack 2013-01-23 0:20 [RFC/PATCH] lint: additional response checking/skipping for hijack Eric Wong @ 2013-01-28 22:01 ` James Tucker 2013-04-21 15:41 ` Tim Carey-Smith 0 siblings, 1 reply; 6+ messages in thread From: James Tucker @ 2013-01-28 22:01 UTC (permalink / raw) To: rack-devel [-- Attachment #1: Type: text/plain, Size: 2477 bytes --] I'd generally recommend that the hijacking app return valid stub data, something like: [200, {}, []]. On 22 January 2013 16:20, Eric Wong <normalperson@yhbt.net> wrote: > Not a serious patch for now, at least not all of it. > I suspect middlewares will break badly if the body.each/body.close > checks are enforced. > > --- > lib/rack/lint.rb | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb > index 1bc2127..f895772 100644 > --- a/lib/rack/lint.rb > +++ b/lib/rack/lint.rb > @@ -9,6 +9,7 @@ class Lint > def initialize(app) > @app = app > @content_length = nil > + @response_hijacked = false > end > > # :stopdoc: > @@ -47,6 +48,15 @@ def _call(env) > > ## and returns an Array of exactly three values: > status, headers, @body = @app.call(env) > + > + # hijacked requests may not give a valid response, do not check them > + if env.include?("rack.hijack_io") > + # request hijacking implies response hijacking, this will ensure > + # the response body raises if body.each or body.close gets called > + @response_hijacked = true > + return [ status, headers, self ] > + end > + > ## The *status*, > check_status status > ## the *headers*, > @@ -530,6 +540,7 @@ def check_hijack_response(headers, env) > headers['rack.hijack'] = proc do |io| > original_hijack.call HijackWrapper.new(io) > end > + @response_hijacked = true > else > ## > ## The special response header <tt>rack.hijack</tt> must only be > set > @@ -636,6 +647,9 @@ def verify_content_length(bytes) > > ## === The Body > def each > + assert("server is not attempting to iterate hijacked response > body") { > + @response_hijacked == false > + } > @closed = false > bytes = 0 > > @@ -683,6 +697,9 @@ def each > end > > def close > + assert("server is not attempting to close hijacked response") { > + @response_hijacked == false > + } > @closed = true > @body.close if @body.respond_to?(:close) > end > -- > Eric Wong > -- --- You received this message because you are subscribed to the Google Groups "Rack Development" group. To unsubscribe from this group, send email to rack-devel+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out. [-- Attachment #2: Type: text/html, Size: 3239 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] lint: additional response checking/skipping for hijack 2013-01-28 22:01 ` James Tucker @ 2013-04-21 15:41 ` Tim Carey-Smith 2013-04-23 1:37 ` James Tucker 0 siblings, 1 reply; 6+ messages in thread From: Tim Carey-Smith @ 2013-04-21 15:41 UTC (permalink / raw) To: rack-devel Could you update the examples on the original issue to behave in this way? This is a point of confusion, I think. Perhaps this could be added to the SPEC as well? On Jan 29, 2013, at 11:01 AM, James Tucker <jftucker@gmail.com> wrote: > I'd generally recommend that the hijacking app return valid stub data, > something like: [200, {}, []]. > > > On 22 January 2013 16:20, Eric Wong <normalperson@yhbt.net> wrote: > >> Not a serious patch for now, at least not all of it. >> I suspect middlewares will break badly if the body.each/body.close >> checks are enforced. >> >> --- >> lib/rack/lint.rb | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb >> index 1bc2127..f895772 100644 >> --- a/lib/rack/lint.rb >> +++ b/lib/rack/lint.rb >> @@ -9,6 +9,7 @@ class Lint >> def initialize(app) >> @app = app >> @content_length = nil >> + @response_hijacked = false >> end >> >> # :stopdoc: >> @@ -47,6 +48,15 @@ def _call(env) >> >> ## and returns an Array of exactly three values: >> status, headers, @body = @app.call(env) >> + >> + # hijacked requests may not give a valid response, do not check them >> + if env.include?("rack.hijack_io") >> + # request hijacking implies response hijacking, this will ensure >> + # the response body raises if body.each or body.close gets called >> + @response_hijacked = true >> + return [ status, headers, self ] >> + end >> + >> ## The *status*, >> check_status status >> ## the *headers*, >> @@ -530,6 +540,7 @@ def check_hijack_response(headers, env) >> headers['rack.hijack'] = proc do |io| >> original_hijack.call HijackWrapper.new(io) >> end >> + @response_hijacked = true >> else >> ## >> ## The special response header <tt>rack.hijack</tt> must only be >> set >> @@ -636,6 +647,9 @@ def verify_content_length(bytes) >> >> ## === The Body >> def each >> + assert("server is not attempting to iterate hijacked response >> body") { >> + @response_hijacked == false >> + } >> @closed = false >> bytes = 0 >> >> @@ -683,6 +697,9 @@ def each >> end >> >> def close >> + assert("server is not attempting to close hijacked response") { >> + @response_hijacked == false >> + } >> @closed = true >> @body.close if @body.respond_to?(:close) >> end >> -- >> Eric Wong >> > > -- > > --- > You received this message because you are subscribed to the Google Groups "Rack Development" group. > To unsubscribe from this group, send email to rack-devel+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out. > > -- --- 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/groups/opt_out. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] lint: additional response checking/skipping for hijack 2013-04-21 15:41 ` Tim Carey-Smith @ 2013-04-23 1:37 ` James Tucker 2013-04-23 1:48 ` Tim Carey-Smith 0 siblings, 1 reply; 6+ messages in thread From: James Tucker @ 2013-04-23 1:37 UTC (permalink / raw) To: rack-devel [-- Attachment #1: Type: text/plain, Size: 3721 bytes --] The examples do work this way. What are you referring to? On Sun, Apr 21, 2013 at 8:41 AM, Tim Carey-Smith <g@spork.in> wrote: > Could you update the examples on the original issue to behave in this way? > This is a point of confusion, I think. > > Perhaps this could be added to the SPEC as well? > > On Jan 29, 2013, at 11:01 AM, James Tucker <jftucker@gmail.com> wrote: > > > I'd generally recommend that the hijacking app return valid stub data, > > something like: [200, {}, []]. > > > > > > On 22 January 2013 16:20, Eric Wong <normalperson@yhbt.net> wrote: > > > >> Not a serious patch for now, at least not all of it. > >> I suspect middlewares will break badly if the body.each/body.close > >> checks are enforced. > >> > >> --- > >> lib/rack/lint.rb | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb > >> index 1bc2127..f895772 100644 > >> --- a/lib/rack/lint.rb > >> +++ b/lib/rack/lint.rb > >> @@ -9,6 +9,7 @@ class Lint > >> def initialize(app) > >> @app = app > >> @content_length = nil > >> + @response_hijacked = false > >> end > >> > >> # :stopdoc: > >> @@ -47,6 +48,15 @@ def _call(env) > >> > >> ## and returns an Array of exactly three values: > >> status, headers, @body = @app.call(env) > >> + > >> + # hijacked requests may not give a valid response, do not check > them > >> + if env.include?("rack.hijack_io") > >> + # request hijacking implies response hijacking, this will > ensure > >> + # the response body raises if body.each or body.close gets > called > >> + @response_hijacked = true > >> + return [ status, headers, self ] > >> + end > >> + > >> ## The *status*, > >> check_status status > >> ## the *headers*, > >> @@ -530,6 +540,7 @@ def check_hijack_response(headers, env) > >> headers['rack.hijack'] = proc do |io| > >> original_hijack.call HijackWrapper.new(io) > >> end > >> + @response_hijacked = true > >> else > >> ## > >> ## The special response header <tt>rack.hijack</tt> must only be > >> set > >> @@ -636,6 +647,9 @@ def verify_content_length(bytes) > >> > >> ## === The Body > >> def each > >> + assert("server is not attempting to iterate hijacked response > >> body") { > >> + @response_hijacked == false > >> + } > >> @closed = false > >> bytes = 0 > >> > >> @@ -683,6 +697,9 @@ def each > >> end > >> > >> def close > >> + assert("server is not attempting to close hijacked response") { > >> + @response_hijacked == false > >> + } > >> @closed = true > >> @body.close if @body.respond_to?(:close) > >> end > >> -- > >> Eric Wong > >> > > > > -- > > > > --- > > You received this message because you are subscribed to the Google > Groups "Rack Development" group. > > To unsubscribe from this group, send email to > rack-devel+unsubscribe@googlegroups.com. > > For more options, visit https://groups.google.com/groups/opt_out. > > > > > > -- > > --- > 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/groups/opt_out. > > > -- --- 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/groups/opt_out. [-- Attachment #2: Type: text/html, Size: 5484 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] lint: additional response checking/skipping for hijack 2013-04-23 1:37 ` James Tucker @ 2013-04-23 1:48 ` Tim Carey-Smith 2013-04-28 1:17 ` James Tucker 0 siblings, 1 reply; 6+ messages in thread From: Tim Carey-Smith @ 2013-04-23 1:48 UTC (permalink / raw) To: rack-devel The referenced hijack.ru app returns the IO object instead of returning [200, {}, []]. https://github.com/raggi/thin/blob/e04855459cb42fd98a0a483075f8337cafe6d949/example/hijack.ru#L15 Thanks, Tim On Apr 23, 2013, at 1:37 PM, James Tucker <jftucker@gmail.com> wrote: > The examples do work this way. > > What are you referring to? > > > On Sun, Apr 21, 2013 at 8:41 AM, Tim Carey-Smith <g@spork.in> wrote: > >> Could you update the examples on the original issue to behave in this way? >> This is a point of confusion, I think. >> >> Perhaps this could be added to the SPEC as well? >> >> On Jan 29, 2013, at 11:01 AM, James Tucker <jftucker@gmail.com> wrote: >> >>> I'd generally recommend that the hijacking app return valid stub data, >>> something like: [200, {}, []]. >>> >>> >>> On 22 January 2013 16:20, Eric Wong <normalperson@yhbt.net> wrote: >>> >>>> Not a serious patch for now, at least not all of it. >>>> I suspect middlewares will break badly if the body.each/body.close >>>> checks are enforced. >>>> >>>> --- >>>> lib/rack/lint.rb | 17 +++++++++++++++++ >>>> 1 file changed, 17 insertions(+) >>>> >>>> diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb >>>> index 1bc2127..f895772 100644 >>>> --- a/lib/rack/lint.rb >>>> +++ b/lib/rack/lint.rb >>>> @@ -9,6 +9,7 @@ class Lint >>>> def initialize(app) >>>> @app = app >>>> @content_length = nil >>>> + @response_hijacked = false >>>> end >>>> >>>> # :stopdoc: >>>> @@ -47,6 +48,15 @@ def _call(env) >>>> >>>> ## and returns an Array of exactly three values: >>>> status, headers, @body = @app.call(env) >>>> + >>>> + # hijacked requests may not give a valid response, do not check >> them >>>> + if env.include?("rack.hijack_io") >>>> + # request hijacking implies response hijacking, this will >> ensure >>>> + # the response body raises if body.each or body.close gets >> called >>>> + @response_hijacked = true >>>> + return [ status, headers, self ] >>>> + end >>>> + >>>> ## The *status*, >>>> check_status status >>>> ## the *headers*, >>>> @@ -530,6 +540,7 @@ def check_hijack_response(headers, env) >>>> headers['rack.hijack'] = proc do |io| >>>> original_hijack.call HijackWrapper.new(io) >>>> end >>>> + @response_hijacked = true >>>> else >>>> ## >>>> ## The special response header <tt>rack.hijack</tt> must only be >>>> set >>>> @@ -636,6 +647,9 @@ def verify_content_length(bytes) >>>> >>>> ## === The Body >>>> def each >>>> + assert("server is not attempting to iterate hijacked response >>>> body") { >>>> + @response_hijacked == false >>>> + } >>>> @closed = false >>>> bytes = 0 >>>> >>>> @@ -683,6 +697,9 @@ def each >>>> end >>>> >>>> def close >>>> + assert("server is not attempting to close hijacked response") { >>>> + @response_hijacked == false >>>> + } >>>> @closed = true >>>> @body.close if @body.respond_to?(:close) >>>> end >>>> -- >>>> Eric Wong >>>> >>> >>> -- >>> >>> --- >>> You received this message because you are subscribed to the Google >> Groups "Rack Development" group. >>> To unsubscribe from this group, send email to >> rack-devel+unsubscribe@googlegroups.com. >>> For more options, visit https://groups.google.com/groups/opt_out. >>> >>> >> >> -- >> >> --- >> 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/groups/opt_out. >> >> >> > > -- > > --- > 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/groups/opt_out. > > -- --- 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/groups/opt_out. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] lint: additional response checking/skipping for hijack 2013-04-23 1:48 ` Tim Carey-Smith @ 2013-04-28 1:17 ` James Tucker 0 siblings, 0 replies; 6+ messages in thread From: James Tucker @ 2013-04-28 1:17 UTC (permalink / raw) To: rack-devel [-- Attachment #1: Type: text/plain, Size: 5069 bytes --] Hah, so it does. The other doesn't. That's what I get for writing teeny examples. On Mon, Apr 22, 2013 at 6:48 PM, Tim Carey-Smith <g@spork.in> wrote: > The referenced hijack.ru app returns the IO object instead of returning > [200, {}, []]. > > > https://github.com/raggi/thin/blob/e04855459cb42fd98a0a483075f8337cafe6d949/example/hijack.ru#L15 > > Thanks, > Tim > > On Apr 23, 2013, at 1:37 PM, James Tucker <jftucker@gmail.com> wrote: > > > The examples do work this way. > > > > What are you referring to? > > > > > > On Sun, Apr 21, 2013 at 8:41 AM, Tim Carey-Smith <g@spork.in> wrote: > > > >> Could you update the examples on the original issue to behave in this > way? > >> This is a point of confusion, I think. > >> > >> Perhaps this could be added to the SPEC as well? > >> > >> On Jan 29, 2013, at 11:01 AM, James Tucker <jftucker@gmail.com> wrote: > >> > >>> I'd generally recommend that the hijacking app return valid stub data, > >>> something like: [200, {}, []]. > >>> > >>> > >>> On 22 January 2013 16:20, Eric Wong <normalperson@yhbt.net> wrote: > >>> > >>>> Not a serious patch for now, at least not all of it. > >>>> I suspect middlewares will break badly if the body.each/body.close > >>>> checks are enforced. > >>>> > >>>> --- > >>>> lib/rack/lint.rb | 17 +++++++++++++++++ > >>>> 1 file changed, 17 insertions(+) > >>>> > >>>> diff --git a/lib/rack/lint.rb b/lib/rack/lint.rb > >>>> index 1bc2127..f895772 100644 > >>>> --- a/lib/rack/lint.rb > >>>> +++ b/lib/rack/lint.rb > >>>> @@ -9,6 +9,7 @@ class Lint > >>>> def initialize(app) > >>>> @app = app > >>>> @content_length = nil > >>>> + @response_hijacked = false > >>>> end > >>>> > >>>> # :stopdoc: > >>>> @@ -47,6 +48,15 @@ def _call(env) > >>>> > >>>> ## and returns an Array of exactly three values: > >>>> status, headers, @body = @app.call(env) > >>>> + > >>>> + # hijacked requests may not give a valid response, do not check > >> them > >>>> + if env.include?("rack.hijack_io") > >>>> + # request hijacking implies response hijacking, this will > >> ensure > >>>> + # the response body raises if body.each or body.close gets > >> called > >>>> + @response_hijacked = true > >>>> + return [ status, headers, self ] > >>>> + end > >>>> + > >>>> ## The *status*, > >>>> check_status status > >>>> ## the *headers*, > >>>> @@ -530,6 +540,7 @@ def check_hijack_response(headers, env) > >>>> headers['rack.hijack'] = proc do |io| > >>>> original_hijack.call HijackWrapper.new(io) > >>>> end > >>>> + @response_hijacked = true > >>>> else > >>>> ## > >>>> ## The special response header <tt>rack.hijack</tt> must only > be > >>>> set > >>>> @@ -636,6 +647,9 @@ def verify_content_length(bytes) > >>>> > >>>> ## === The Body > >>>> def each > >>>> + assert("server is not attempting to iterate hijacked response > >>>> body") { > >>>> + @response_hijacked == false > >>>> + } > >>>> @closed = false > >>>> bytes = 0 > >>>> > >>>> @@ -683,6 +697,9 @@ def each > >>>> end > >>>> > >>>> def close > >>>> + assert("server is not attempting to close hijacked response") { > >>>> + @response_hijacked == false > >>>> + } > >>>> @closed = true > >>>> @body.close if @body.respond_to?(:close) > >>>> end > >>>> -- > >>>> Eric Wong > >>>> > >>> > >>> -- > >>> > >>> --- > >>> You received this message because you are subscribed to the Google > >> Groups "Rack Development" group. > >>> To unsubscribe from this group, send email to > >> rack-devel+unsubscribe@googlegroups.com. > >>> For more options, visit https://groups.google.com/groups/opt_out. > >>> > >>> > >> > >> -- > >> > >> --- > >> 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/groups/opt_out. > >> > >> > >> > > > > -- > > > > --- > > 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/groups/opt_out. > > > > > > -- > > --- > 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/groups/opt_out. > > > -- --- 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/groups/opt_out. [-- Attachment #2: Type: text/html, Size: 8121 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-28 1:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-23 0:20 [RFC/PATCH] lint: additional response checking/skipping for hijack Eric Wong 2013-01-28 22:01 ` James Tucker 2013-04-21 15:41 ` Tim Carey-Smith 2013-04-23 1:37 ` James Tucker 2013-04-23 1:48 ` Tim Carey-Smith 2013-04-28 1:17 ` James Tucker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).