diff --git a/lib/WebGUI/Asset/File.pm b/lib/WebGUI/Asset/File.pm index f1b49f8b1..47dc5addf 100644 --- a/lib/WebGUI/Asset/File.pm +++ b/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'; } diff --git a/lib/WebGUI/Content/Asset.pm b/lib/WebGUI/Content/Asset.pm index f1d4929a4..999ff8103 100644 --- a/lib/WebGUI/Content/Asset.pm +++ b/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; } diff --git a/lib/WebGUI/Session/Response.pm b/lib/WebGUI/Session/Response.pm index 19b2641a7..11d516e3c 100644 --- a/lib/WebGUI/Session/Response.pm +++ b/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 be set in the config file and then +some middleware or reverse-proxy in front of L 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) or trigger a stream (C) depending on how L is configured. +If C is set in the config file, C 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, doing L 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; diff --git a/share/site.psgi b/share/site.psgi index 9c553b73a..85709bb60 100644 --- a/share/site.psgi +++ b/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; diff --git a/t/PSGI/Http.t b/t/PSGI/Http.t new file mode 100644 index 000000000..dc4ea874f --- /dev/null +++ b/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); + +}; + diff --git a/t/Session/Http.t b/t/Session/Http.t index a97c676e7..a036499a4 100644 --- a/t/Session/Http.t +++ b/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(''); ####################################################