Hi Eric, > On 12 Mar 2021, at 13:00, Eric Wong <[email protected]> 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
0001-Allocate-a-new-request-for-each-client.patch
Description: Binary data
