Is Rack partial hijack supported?

Is Rack partial hijack supported?

[Samuel Williams]

Hi Eric,

I’ve been testing Rack server conformance and behaviour and noticed that while unicorn advertises `env[‘rack.hijack?’]` as true, it fails to correctly stream the following response:

if env['rack.hijack?']
  callback = proc do |stream|
    stream.write "Hello World"
    stream.close
  end
  
  return [200, {'rack.hijack' => callback}, nil]
else
  [404, {}, []]
end

Am I misunderstanding something about how this should work or does Unicorn not support partial hijack?

Kind regards,
Samuel

Re: Is Rack partial hijack supported?

[Eric Wong]

Samuel Williams <samuel@oriontransfer.net> wrote:
> Hi Eric,
> 
> I’ve been testing Rack server conformance and behaviour and noticed that while unicorn advertises `env[‘rack.hijack?’]` as true, it fails to correctly stream the following response:
> 
> if env['rack.hijack?']
>   callback = proc do |stream|
>     stream.write "Hello World"
>     stream.close
>   end
>   
>   return [200, {'rack.hijack' => callback}, nil]
> else
>   [404, {}, []]
> end

That looks fine...

> Am I misunderstanding something about how this should work or does Unicorn not support partial hijack?

I wonder if it's Rack::Chunked being loaded by default w/
RACK_ENV=(development|deployment) being the problem.

What error do you see? (assuming you're using curl or similar
for testing).

Does using `-E none' or RACK_ENV=none fix it?

$ cat >hijack.ru <<EOF
use Rack::ContentType, 'text/plain'
run(lambda do |env|
  if env['rack.hijack?']
    callback = proc do |stream|
      stream.write "Hello World"
      stream.close
    end
    return [200, {'rack.hijack' => callback}, nil]
  else
    [404, {}, []]
  end
end)
EOF

# This works fine:
$ unicorn -E none hijack.ru
$ curl -Ssf http://0:8080/

# This fails since Rack::Chunk is loaded by default:
$ unicorn hijack.ru
$ curl -Ssf http://0:8080/
curl: (56) Illegal or missing hexadecimal sequence in chunked-encoding

So I think the hijack callback needs to do the chunking itself;
I'm not sure...

That said, I have no idea if rack.hijack gets used at all w/
unicorn; and I seem to recall there being a bit of confusion
on how hijack and middleware would interact...


(please reply-all for archival purposes, thanks)

Re: Is Rack partial hijack supported?

[Samuel Williams]

Thanks Eric.

> # This works fine:
> $ unicorn -E none hijack.ru
> $ curl -Ssf http://0:8080/
> 
> # This fails since Rack::Chunk is loaded by default:
> $ unicorn hijack.ru
> $ curl -Ssf http://0:8080/
> curl: (56) Illegal or missing hexadecimal sequence in chunked-encoding
> 
> So I think the hijack callback needs to do the chunking itself;
> I'm not sure...
> 
> That said, I have no idea if rack.hijack gets used at all w/
> unicorn; and I seem to recall there being a bit of confusion
> on how hijack and middleware would interact…

Using `-E none` solves the issue and I can proceed with the conformance checks, thanks! AFAIK, no other server behaves like this (using `Rack::Server` for default middleware) although I can understand why for Rack 2 that was a logical choice.

FYI, we have deprecated `Rack::Chunked` in Rack 3. The wire format should be the responsibility of the server, not the application/middleware. This middleware in particular was a blocker to support HTTP/2 which does chunked encoding using binary framing. In addition, `Rack::Server` has also been moved out of the Rack gem. The Rack gem should focus on Rack the interface, rather than Rack the (server) implementation. As such, the `default_middleware_by_environment` should be an internal detail of the implementation rather than the middleware/application.

My suggestion is that you implement chunked encoding directly in your server and only if it makes sense according to the response you get back from the application. For a hijacked response, given that you are already using `connection: close`, you can probably just directly pass the socket to the hijack proc. This will be compatible with websockets too.

Kind regards,
Samuel

Re: Is Rack partial hijack supported?

[Eric Wong]

Samuel Williams <samuel@oriontransfer.net> wrote:
> Using `-E none` solves the issue and I can proceed with the
> conformance checks, thanks! AFAIK, no other server behaves
> like this (using `Rack::Server` for default middleware)
> although I can understand why for Rack 2 that was a logical
> choice.

OK, good to know.  I think the defaults were copied from rack
0.9 or so...  I'm still trying to keep unicorn supported across
as many Rack versions as possible since it tends to be used by
legacy systems that rarely see updates.

I've always used `-E none' for my projects.

> FYI, we have deprecated `Rack::Chunked` in Rack 3. The wire
> format should be the responsibility of the server, not the
> application/middleware. This middleware in particular was a
> blocker to support HTTP/2 which does chunked encoding using
> binary framing. In addition, `Rack::Server` has also been
> moved out of the Rack gem. The Rack gem should focus on Rack
> the interface, rather than Rack the (server) implementation.
> As such, the `default_middleware_by_environment` should be an
> internal detail of the implementation rather than the
> middleware/application.

Couldn't Rack::Chunked be made a no-op for everything aside from
HTTP/1.1?  That would cause less problems for existing Rack <=2.x
users wanting to upgrade.  It's already a no-op for HTTP/1.0...
(cf. rack/chunked.rb::chunkable_version? )

> My suggestion is that you implement chunked encoding directly
> in your server and only if it makes sense according to the
> response you get back from the application. For a hijacked
> response, given that you are already using `connection:
> close`, you can probably just directly pass the socket to the
> hijack proc. This will be compatible with websockets too.

"Connection: close" alone isn't enough.  body.each {...} can
crash and leave truncated responses clients don't know about
(especially. with non-HTML/XML/JSON content that has no closing
tag).  Either chunked is required (but HTTP/1.1-only), or
relying on gzip for built-in error-checking w/ HTTP/1.0 clients.

Anyways I'll wait a bit for rack 3 to finalize since I'm busy
with other stuff atm.

Re: Is Rack partial hijack supported?

[Eric Wong]

Eric Wong <e@yhbt.net> wrote:
> Couldn't Rack::Chunked be made a no-op for everything aside from
> HTTP/1.1?  That would cause less problems for existing Rack <=2.x
> users wanting to upgrade.  It's already a no-op for HTTP/1.0...
> (cf. rack/chunked.rb::chunkable_version? )

I don't know if you pay attention to rack-devel, but I've posted
a proposal (patches || pull request) to undeprecate Rack::Chunked, here:
https://groups.google.com/d/msgid/rack-devel/20220901214536.5469-1-e%4080x24.org

Or if you don't want to deal with Google and want to view the whole thread +
automated patch application from hunk-header links:
https://public-inbox.org/rack-devel/20220901214536.5469-1-e@80x24.org/T/

> Anyways I'll wait a bit for rack 3 to finalize since I'm busy
> with other stuff atm.

Any ETA on Rack 3?