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 <la...@unity3d.com> wrote: > > > On Fri, Jun 5, 2015 at 8:26 PM, Bill Moseley <mose...@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 >> mose...@hank.org >> >> _______________________________________________ >> List: Catalyst@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@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 mose...@hank.org
_______________________________________________ List: Catalyst@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/