[Catalyst] Configure psgi.input as optional?

[Bill Moseley]

I created a pull for checking the return for the psgi.input file:
https://github.com/perl-catalyst/catalyst-runtime/pull/100

I created a ticket for HTTP::Body:
https://rt.cpan.org/Ticket/Display.html?id=105021

I'll create a separate pull for a config setting to make creating the
psgi.input file optional when I have a bit of time.

On Fri, Jun 5, 2015 at 4:38 PM, Lasse Makholm <lasse at unity3d.com> wrote:

>
>
> On Fri, Jun 5, 2015 at 8:26 PM, Bill Moseley <moseley at hank.org> wrote:
>
>> Hi,
>>
>> Our app handles a lot of uploads, often quite large uploads.    As we
>> know, uploads (and any non-parsed body) gets written to temp files via
>> HTTP::Body.
>>
>> Now, Catalyst::Request also writes every body to a temp file (via
>> Stream::Buffered).   So, depending on the body size, can take up to twice
>> the space in temp files as the content length.
>>
>> If we are not going to use psgi.input and just use Catalyst's
>> (HTTP::Body's) temp files could we make the creation of psgi.input optional?
>>
>> I have a few other concerns about the current code.
>>
>> prepare_body in Catalyst::Request does not check the return code when it
>> writes to the psgi.input file.   HTTP::Body also fails to check return
>> codes when it writes.   This means if the partition where temp files are
>> created fills then Catalyst request will ignore this and continue as if
>> there's no problem.   I trust the risk here is recognized.
>>
>> I think the fix in Catalyst::Request is pretty simple by checking return
>> values:
>>
>>     # Check for definedness as you could read '0'
>>     while ( defined ( my $chunk = $self->read() ) ) {
>>         $self->prepare_body_chunk($chunk);
>>
>>         next unless $stream_buffer;
>>         $stream_buffer->print($chunk)
>>             || die sprintf "Failed to write %d bytes to psgi.input file:
>> $!", length( $chunk );
>>     }
>>
>> HTTP::Body needs similar changes.
>>
>
> On a related note, things like NFS mounted file systems tend to not
> guarantee that data is safely written to disk until all write() calls AND
> the final close() have completed successfully.
>
> Code that checks that write() succeeds but then fails to do the same for
> close() is still broken.
>
> Thanks for looking into this. Our app handles lots of large uploads too
> and I would love to see this fixed in HTTP::Body.
>
> /L
>
>
>>
>> See any problem with making the psgi.input file optional?   And any
>> reason not to throw an exception when writing to the temp file fails?
>>
>> BTW -- Plack::Handler::Apache2 sets psgi.input to the Apache request
>> object $r -- it would be handy to preserve this object for later use.
>>
>>
>> --
>> Bill Moseley
>> moseley at hank.org
>>
>> _______________________________________________
>> List: Catalyst at lists.scsys.co.uk
>> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
>> Searchable archive:
>> http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
>> Dev site: http://dev.catalyst.perl.org/
>>
>>
>
> _______________________________________________
> List: Catalyst at lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive:
> http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
>


-- 
Bill Moseley
moseley at hank.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/catalyst/attachments/20150606/73a1056c/attachment.htm>