Hi Eric,

> On 12 Mar 2021, at 13:00, Eric Wong <normalperson@yhbt.net> wrote:
> 
> I was going to say I didn't have a preference and the
> current approach was fine...
> 
> However, I just now realized now that clobbering+replacing all
> of @request is required.
> 
> That's because env['rack.input'] is (Stream|Tee)Input,
> and that is lazily consumed and those objects keep state in
> @request (as the historically-named @parser)
> 
> If we're to make env safe to be shipped off to another thread,
> then @request still needs to stick around to maintain state
> of env['rack.input'] until it's all consumed.


Ah yeah, that’s a good point. I don’t think this affects us right
now so the existing patch still keeps us safe, but it would break this
case then indeed.

> It probably doesn't affect most apps out there that just decode
> forms via HTTP POST; but the streamed rack.input is something
> that's critical for projects that feed unicorn with PUTs via
> "curl -T"

Ah yeah. So do you think that on top of the current patch we’d need
something like the attached patch (which moves the @request allocation),
or would only the latter patch be needed then?

In the latter case there’s still a bunch of logic for Rack hijack around
then which might not be needed at that point, but I’m not entirely sure
how that would look like.

Cheers,

Dirkjan