File assets should always give IO::File::WithPath objects to PSGI, instead of the current redirecting or streaming behavior. (#11688)

New API method:  WebGUI::Response::sendFile;  it, as appropriate, calls
setRedirect or setStreamedFile, depending on enableStreamingUploads config var.
setStreamedFile now kicks off the XSendfile process.
File.pm now uses this instead of trying to set both a redirect and a stream.
IO::File::WithPath blows up if a file doesn't exist so this raises an exception
now.
The http now no longer insist that '0' is not a valid filename to stream.
site.psgi, depending on enableStreamingUploads, enables either the Static
or XSendfile middleware.

lib/WebGUI/Asset/File.pm

@@ -619,9 +619,10 @@ sub www_view {
 		return sprintf($i18n->get("file not found"), $self->getUrl());
 	}
 
-    $session->response->setRedirect($self->getFileUrl) unless $session->config->get('enableStreamingUploads');
-    $session->response->setStreamedFile($self->getStorageLocation->getPath($self->filename));
-    $session->response->sendHeader;
+    # sendFile does either a redirect or starts a stream depending on how we're configured
+
+    $session->response->sendFile($self->getStorageLocation, $self->filename);
+
     return 'chunked';
 }
 

lib/WebGUI/Content/Asset.pm

@@ -176,19 +176,6 @@ sub handler {
     else {
         $output = dispatch($session, getRequestedAssetUrl($session));
     }
-
-    my $filename = $session->response->getStreamedFile();
-    if ((defined $filename) && ($config->get("enableStreamingUploads") eq "1")) {
-        my $ct = guess_media_type($filename);
-        my $oldContentType = $request->content_type($ct);
-        if ($request->sendfile($filename) ) {
-            return; # TODO - what should we return to indicate streaming?
-        } 
-        else {
-            $request->content_type($oldContentType);
-        }
-    }
-
     return $output;
 }
 

lib/WebGUI/Session/Response.pm

@@ -6,6 +6,8 @@ use warnings;
 use parent qw(Plack::Response);
 
 use IO::File::WithPath;
+use Class::C3;
+use LWP::MediaTypes;
 
 use Plack::Util::Accessor qw(session streaming writer streamer);
 
@@ -212,7 +214,7 @@ Returns the stored epoch date when the page as last modified.
 sub getLastModified {
     my $self = shift;
     return $self->{_http}{lastModified};
-}  
+}
 
 #
 #
@@ -304,15 +306,65 @@ sub getStreamedFile {
  
 =head2 setStreamedFile ( ) {
  
-Set a file to be streamed thru mod_perl.
- 
+Set a file to be streamed through mod_perl.
+Rrequires that C<enableStreamingUploads> be set in the config file and then
+some middleware or reverse-proxy in front of L<WebGUI> to catch the X-Sendfile
+headers and replace that with the file to be sent.
+
 =cut
 
 sub setStreamedFile {
     my $self = shift;
     my $fn = shift;
+
+    if( ! defined $fn or ! length $fn ) {
+        # t/Session/Http.t tests that it can call this with '' as an arg
+        # really, it looks like it is testing implementation rather than behavior but I don't know what behavior it's getting at so, voodoo
+        $self->body(undef);
+        $self->{_http}{streamlocation} = undef;
+        return;
+    }
+
     $self->{_http}{streamlocation} = $fn;
-    # $self->body( IO::File::WithPath->new( $fn ) );  # let Plack handle the streaming, or let Plack::Middleware::XSendfile punt it; we don't want to send a 302 header and send the file, too; should be one or the other, selectable
+
+    # return undef unless $self->session->config->get('enableStreamingUploads'); # throw an error?  handle gracefully? XX
+
+    my $fh = eval { IO::File::WithPath->new( $fn ) } or WebGUI::Error::InvalidFile->throw(
+        error => "Couldn't create an IO::File::WithPath object: $@ $!",
+        brokenFile => $fn,
+    );
+
+    $self->body( $fh );
+
+    1;
+
+}
+
+#
+#
+#
+
+=head2 sendFile ( ) {
+
+Either redirect (C<setRedirect()>) or trigger a stream (C<setStreamedFile()>) depending on how L<WebGUI> is configured.
+If C<enableStreamingUploads> is set in the config file, C<setStreamedFile()> is used.
+
+=cut
+
+#    $session->response->sendFile($self->getStorageLocation, $self->filename); 
+
+ 
+sub sendFile {
+    my $self = shift;
+    my $storage = shift;
+    my $filename = shift;
+    if( $self->session->config->get('enableStreamingUploads') ) {
+        $self->setStreamedFile( $storage->getPath($filename) );
+    }
+    else {
+         $self->setRedirect( $storage->getUrl($filename) );
+    }
+    1;
 }
 
 #
@@ -350,4 +402,39 @@ sub getCacheControl {
     return $self->{_http}{cacheControl} || 1;
 }   
 
+=head2 finalize ( ) 
+
+Subclasses Plack::Response C<finalize()>, doing L<WebGUI> specific finalization chores.
+
+=cut
+
+#
+#
+#
+
+sub finalize {
+
+    my $self = shift;
+
+    my $filename = $self->getStreamedFile();
+    if (defined $filename and $self->session->config->get("enableStreamingUploads") ) {
+        # at this point, $request->body contains an IO::File::WithPath object, and that's all Plack needs
+        my $ct = LWP::MediaTypes::guess_media_type($filename);
+        $self->content_type($ct);
+    }
+
+    # in the future, sendHeader's logic should be moved into here and sendHeader should vanish.
+    # the rest of WebGUI should essentially never need to call sendHeader explicitly but should
+    # just call methods in here to configure the response and then return back up to the top where
+    # WebGUI.pm calls finalize and returns the response object back to Plack.
+    # for now, I've added this extra (harmless) call to sendHeader to get started on removing the
+    # others.
+
+    $self->sendHeader(); # doesn't send the header, only fixes the response up based on options set
+
+    $self->next::method(@_);
+
+}
+
+
 1;

share/site.psgi

@@ -6,6 +6,7 @@ use WebGUI;
 builder {
     my $wg = WebGUI->new( config => $ENV{WEBGUI_CONFIG} );
     my $config = $wg->config;
+    my $streaming_uploads = $config->get('enableStreamingUploads'); # have to restart for changes to this to take effect
 
     enable 'Log4perl', category => $config->getFilename, conf => WebGUI::Paths->logConfig;
     enable 'SimpleContentFilter', filter => sub {
@@ -20,9 +21,13 @@ builder {
 
     # For PassThru, use Plack::Builder::mount
 
-    # Extras fallback (you should be using something else to serve static files in production)
+    # Serve "Extras"
+    # Plack::Middleware::Static is fallback (you should be using something else to serve static files in production,
+    # unless you're using the corona Plack server, then it doesn't matter nearly so much)
+
     my ( $extrasURL, $extrasPath ) = ( $config->get('extrasURL'), $config->get('extrasPath') );
-    enable 'Static', root => "$extrasPath/", path => sub {s{^\Q$extrasURL/}{}};
+    enable_if {   $streaming_uploads } 'XSendfile';
+    enable_if { ! $streaming_uploads } 'Static', root => "$extrasPath/", path => sub {s{^\Q$extrasURL/}{}};
 
     # Open/close the WebGUI::Session at the outer-most onion layer
     enable '+WebGUI::Middleware::Session', config => $config;

t/PSGI/Http.t

@@ -0,0 +1,68 @@
+use strict;
+use warnings;
+use Test::More tests => 7;
+
+use Plack::Test;
+use Plack::Util;
+use HTTP::Request::Common;
+use WebGUI::Paths;
+use WebGUI::Test;
+
+# test things about responses
+# this is like t/Session/Http.t but Plack specific
+
+SKIP: {
+    skip 'set WEBGUI_LIVE to enable these tests', 7 unless $ENV{WEBGUI_LIVE};
+
+    my $session         = WebGUI::Test->session;
+
+    my $prev_streaming_uploads = $session->config->get('enableStreamingUploads');
+
+    local $ENV{WEBGUI_CONFIG} = WebGUI::Test->file; # tell the share/site.psgi which site to load
+
+    #
+    # fire up a Plack to test streaming
+    #
+
+    $session->config->set('enableStreamingUploads', 1);
+
+    my $app = Plack::Util::load_psgi( WebGUI::Paths->defaultPSGI ); 
+
+    ok( $app, "created a PSGI app from app.psgi" );
+
+    test_psgi $app, sub {
+        my $cb = shift;
+        my $res = $cb->( GET "/root/import/gallery-templates/images/previous.gif" );
+        is $res->code, 200, 'enableStreamingUploads: 200 response';
+        is $res->header('Content-Type'), 'image/gif', '... content type is image/gif';
+        ok substr($res->content, 0, 100) =~ m/GIF89/, '... data contains the string GIF89';
+    };
+
+
+    #
+    # fire up another Plack to test non-streaming
+    #
+
+    $session->config->set('enableStreamingUploads', 0);
+
+    $app = Plack::Util::load_psgi( WebGUI::Paths->defaultPSGI );
+
+    my $redirect_url;
+
+    test_psgi $app, sub {
+        my $cb = shift;
+        my $res = $cb->( GET "/root/import/gallery-templates/images/previous.gif" );
+        is $res->code, 302, 'enableStreamingUploads: 302 response';
+        ok $res->header('Location'), '... Location header in response';
+        $res = $cb->(GET $res->header('Location') );
+        is $res->code, 200, '... following location, we get a 200 response code';
+    };
+
+    #
+    # put things back how they were
+    #
+
+    $session->config->set('enableStreamingUploads', $prev_streaming_uploads);
+
+};
+

t/Session/Http.t

@@ -66,11 +66,22 @@ $response->status('200');
 
 $http->setStreamedFile('');
 is($http->getStreamedFile, undef, 'set/get StreamedFile: false values return undef, empty string');
-$http->setStreamedFile(0);
+$http->setStreamedFile(undef);
 is($http->getStreamedFile, undef, 'set/get StreamedFile: false values return undef, empty string');
 
-$http->setStreamedFile('/home/streaming');
-is($http->getStreamedFile, '/home/streaming', 'set/get StreamedFile: set specific location and get it');
+my $actual_file = $session->config->get('uploadsPath') . '/9e/a3/9ea37e148e517d4ae3d6326f691d848f/previous.gif'; # arbitrary file that exactually exists and hopefully will continue for a while
+$http->setStreamedFile( $actual_file );
+is($http->getStreamedFile, $actual_file, 'set/get StreamedFile: set specific location and get it');
+
+do {
+    eval { 
+        $http->setStreamedFile( $actual_file . '_but_actually_not_an_actual_file_because_someone_appended_a_bunch_of_bloody_garbage_to_it' );
+    };
+    my $e = WebGUI::Error->caught("WebGUI::Error::InvalidFile");
+    my $errorMessage = $e->error;
+    ok($errorMessage =~ m/No such file or directory/, "set/get StreamedFile: setting a non-existant file blows stuff up but that's okay because it's handled gracefully" );
+};
+
 $http->setStreamedFile('');
 
 ####################################################