Bugzilla – Attachment 39670 Details for
Bug 14296
Introduce upload settings, add quota and translate alerts
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14296: Introduce upload settings, check size and translate alerts
Bug-14296-Introduce-upload-settings-check-size-and.patch (text/plain), 9.97 KB, created by
Marcel de Rooy
on 2015-05-29 11:11:25 UTC
(
hide
)
Description:
Bug 14296: Introduce upload settings, check size and translate alerts
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2015-05-29 11:11:25 UTC
Size:
9.97 KB
patch
obsolete
>From 8d756ca0b1c33e21cf781bd0bb4f92edec4c2806 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 29 May 2015 10:56:12 +0200 >Subject: [PATCH] Bug 14296: Introduce upload settings, check size and > translate alerts >Content-Type: text/plain; charset=utf-8 > >This patch includes: >[1] Add sub MaxUploadSize to module and add unit test. > Note that normally the table will still be empty, but a large > fallback is used. >[2] Check the max upload size for the user when submitting the form and > again when processing it. >[3] Make it possible to translate some alerts in upload.tt. Also it moves > the input[type=file] control to the template. > >Test plan: >[1] Upload a file. No restrictions. >[2] Run UploadedFiles.t. >[3] Add a record (sorry, interface is another report) via mysql into > upload_settings: type=filesize, count1=1000. >[4] Upload a file larger than 1000 bytes. Should not be possible. >[5] Upload a smaller file. >[6] Bonus: I tested the second size check by adding a return true in the > javascript validation after the first size check. >--- > C4/UploadedFiles.pm | 37 ++++++++++++-- > cataloguing/value_builder/upload.pl | 14 ++---- > .../en/modules/cataloguing/value_builder/upload.tt | 51 ++++++++++---------- > t/db_dependent/UploadedFiles.t | 42 +++++++++++++++- > 4 files changed, 105 insertions(+), 39 deletions(-) > >diff --git a/C4/UploadedFiles.pm b/C4/UploadedFiles.pm >index d8a5be4..6fdeae0 100644 >--- a/C4/UploadedFiles.pm >+++ b/C4/UploadedFiles.pm >@@ -59,6 +59,8 @@ use File::Basename; > > use C4::Context; > >+use constant MAX_DEFAULT_UPLOAD => 10000000; # relatively large fallback; >+ > sub _get_file_path { > my ($id, $dirname, $filename) = @_; > >@@ -142,12 +144,22 @@ sub UploadFile { > return; > } > >+ my $ue = C4::Context->userenv; >+ my $owner = $ue? $ue->{number}: undef; >+ my $max = C4::UploadedFiles::MaxUploadSize( $owner ); >+ > my $buffer; > my $data = ''; >- while($handle->read($buffer, 1024)) { >+ while( $handle->read($buffer, 1024) && length($data) <= $max ) { > $data .= $buffer; > } > $handle->close; >+ if( length($data) > $max ) { >+ # double check actually (see js); normally not possible, so we just >+ # rely on a general error message and a warn >+ warn "UploadFile: upload size exceeded after submit!"; >+ return; >+ } > > my $sha = new Digest::SHA; > $sha->add($data); >@@ -180,8 +192,6 @@ sub UploadFile { > VALUES (?,?, ?, ?, ?); > }; > my $sth = $dbh->prepare($query); >- my $ue = C4::Context->userenv; >- my $owner = $ue? $ue->{number}: undef; > if( $sth->execute($id, $filename, $dir, length($data), $owner ) ) { > return $id; > } >@@ -335,4 +345,25 @@ sub httpheaders { > '-attachment' => $file, ); > } > >+=head2 MaxUploadSize >+ >+ Returns max upload size for borrower or if the borrower has no explicit >+ user limit, the default max. If no records are found, falls back to a >+ constant. >+ >+=cut >+ >+sub MaxUploadSize { >+ my ( $user ) = @_; >+ my $dbh = C4::Context->dbh; >+ my $query = qq| >+SELECT count1 FROM upload_settings >+WHERE (borrowernumber=? OR borrowernumber IS NULL) AND >+ type='filesize' >+ORDER BY IFNULL(borrowernumber, 0) DESC; >+ |; >+ my ( $max ) = $dbh->selectrow_array( $query, undef, ( $user ) ); >+ return $max // MAX_DEFAULT_UPLOAD; >+} >+ > 1; >diff --git a/cataloguing/value_builder/upload.pl b/cataloguing/value_builder/upload.pl >index ee2145e..01428fb 100755 >--- a/cataloguing/value_builder/upload.pl >+++ b/cataloguing/value_builder/upload.pl >@@ -90,10 +90,9 @@ my $launcher = sub { > $id = C4::UploadedFiles::UploadFile($uploaded_file, $dir, $fh->handle); > my $OPACBaseURL = C4::Context->preference('OPACBaseURL') // ''; > $OPACBaseURL =~ s#/$##; >- if (!$OPACBaseURL) { >- $template->param(MissingURL => 1); >- } >- if($id && $OPACBaseURL) { >+ if( !$OPACBaseURL ) { >+ $template->param( MissingURL => 1 ); >+ } elsif( $id ) { > my $return = "$OPACBaseURL/cgi-bin/koha/opac-retrieve-file.pl?id=$id"; > $template->param( > success => 1, >@@ -116,11 +115,6 @@ my $launcher = sub { > } else { > my $upload_path = C4::Context->config('upload_path'); > if ($upload_path) { >- my $filefield = CGI::filefield( >- -name => 'uploaded_file', >- -size => 50, >- ); >- > my $dirs_tree = [ { > name => '/', > value => '/', >@@ -129,7 +123,7 @@ my $launcher = sub { > > $template->param( > dirs_tree => $dirs_tree, >- filefield => $filefield, >+ maxfilesize => C4::UploadedFiles::MaxUploadSize($loggedinuser), > ); > } else { > $template->param( error_upload_path_not_configured => 1 ); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt >index e08e424..2a74817 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/upload.tt >@@ -7,32 +7,33 @@ > <script type="text/javascript" src="[% interface %]/lib/jquery/jquery.js"></script> > <link rel="stylesheet" type="text/css" href="[% themelang %]/css/staff-global.css" /> > <script type="text/javascript"> >- function ValidateForm() { >- var filename = document.forms["UploadForm"]["uploaded_file"].value; >+//<![CDATA[ >+ function _(s) { return s; } // dummy for translation >+ >+ function ValidateForm() { >+ var filename = $('#uploaded_file').val(); >+ var files = $('#uploaded_file').prop('files'); > var selected = 0; >- var value; >- $('form').each(function() { >- value = $(this).find('input[type="radio"][name="dir"]:checked').val(); >- if (value) { >+ if( $('#UploadForm').find('input[type="radio"][name="dir"]:checked').length > 0 ) { > selected = 1; >- } >- }); >- if (!filename && !selected) { >- alert("Please select a file and its destination."); >- return false; >- } >- else if (!filename) { >- alert("Please select a file to upload."); >- return false; > } >- else if (!selected) { >- alert("Please select a file destination."); >- return false; >- } >- else { >- return true; >+ >+ if (!filename && !selected) { >+ alert( _("Please select a file and its destination.") ); >+ } else if (!filename || files.length<1 ) { >+ alert( _("Please select a file to upload.") ); >+ } else if (!selected) { >+ alert( _("Please select a file destination.") ); >+ } else if( files[0].size > [% maxfilesize %] ) { >+ // check file size in advance; server will do repeated check later >+ alert( _("The file size exceeds the maximum.") ); >+return true; >+ } else { >+ return true; > } >- } >+ return false; >+ } >+//]]> > </script> > > </head> >@@ -102,9 +103,9 @@ > [% IF (dangling) %] > <p class="error">Error: The URL has no file to retrieve.</p> > [% END %] >- <h2>Please select the file to upload : </h2> >- <form name="UploadForm" method="post" enctype="multipart/form-data" action="/cgi-bin/koha/cataloguing/plugin_launcher.pl" onsubmit="return ValidateForm()"> >- [% filefield %] >+ <h2>Please select the file to upload:</h2> >+ <form id="UploadForm" method="post" enctype="multipart/form-data" action="/cgi-bin/koha/cataloguing/plugin_launcher.pl" onsubmit="return ValidateForm()"> >+ <input type="file" name="uploaded_file" id="uploaded_file" /> > <h3>Choose where to upload the file</h3> > [% INCLUDE list_dirs dirs = dirs_tree %] > <input type="hidden" name="from_popup" value="1" /> >diff --git a/t/db_dependent/UploadedFiles.t b/t/db_dependent/UploadedFiles.t >index f0594c5..1d85970 100644 >--- a/t/db_dependent/UploadedFiles.t >+++ b/t/db_dependent/UploadedFiles.t >@@ -3,7 +3,7 @@ > use Modern::Perl; > use File::Temp qw/tempdir/; > use Test::CGI::Multipart; >-use Test::More tests => 20; >+use Test::More tests => 21; > use Test::Warn; > > use t::lib::Mocks; >@@ -11,6 +11,10 @@ use t::lib::Mocks; > use C4::Context; > use C4::UploadedFiles; > >+my $dbh = C4::Context->dbh; >+$dbh->{AutoCommit} = 0; >+$dbh->{RaiseError} = 1; >+ > # This simulates a multipart POST request with a file upload. > my $tcm = new Test::CGI::Multipart; > $tcm->upload_file( >@@ -70,3 +74,39 @@ mkdir $temp2.'/A/AA'; > mkdir $temp2.'/A/BB'; > $a= C4::UploadedFiles::finddirs( $temp2 ); > is( @{$a->[0]->{dirs}} == 2, 1, 'Finddir returns subdirectories' ); >+ >+#tests for MaxUploadSize >+subtest 'Tests for MaxUploadSize' => sub { >+ test_max(); >+}; >+ >+#end of game >+$dbh->rollback; >+ >+sub test_max { >+ $dbh->do("DELETE FROM upload_settings"); >+ my $max = C4::UploadedFiles::MaxUploadSize(); >+ is( $max, 10000000, 'MaxUploadSize is equal to fallback' ); >+ >+ $dbh->do("INSERT INTO upload_settings (type, count1) VALUES (?, ?)", >+ undef, ( 'filesize', 500 )); >+ $max = C4::UploadedFiles::MaxUploadSize(); >+ is( $max, 500, 'MaxUploadSize is equal to default' ); >+ >+ my $borr = $dbh->selectall_arrayref( >+ 'SELECT borrowernumber FROM borrowers LIMIT 2' ); >+ if( @$borr < 2 ) { >+ done_testing(2); >+ return; >+ } >+ my $b1 = $borr->[0]->[0]; >+ $dbh->do("INSERT INTO upload_settings (borrowernumber, type, count1) >+ VALUES (?, ?, ?)", undef, ( $b1, 'filesize', 1500 )); >+ $max = C4::UploadedFiles::MaxUploadSize($b1); >+ is( $max, 1500, 'MaxUploadSize is equal to user limit' ); >+ >+ my $b2 = $borr->[1]->[0]; >+ $max = C4::UploadedFiles::MaxUploadSize($b2); >+ is( $max, 500, 'MaxUploadSize is still equal to default for user 2' ); >+ done_testing(4); >+} >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14296
:
39669
| 39670 |
39766
|
39768