[Claims] Check non-zero size of uploaded files.

perllib/FixMyStreet/App/Form/Claims.pm

@@ -17,6 +17,13 @@ has finished_action => ( is => 'ro' );
 
 has '+is_html5' => ( default => 1 );
 
+has upload_dir => ( is => 'ro', default => sub {
+    my $cfg = FixMyStreet->config('PHOTO_STORAGE_OPTIONS');
+    my $dir = $cfg ? $cfg->{UPLOAD_DIR} : FixMyStreet->config('UPLOAD_DIR');
+    $dir = path($dir, "claims_files")->absolute(FixMyStreet->path_to())->mkdir;
+    return $dir;
+});
+
 before _process_page_array => sub {
     my ($self, $pages) = @_;
     foreach my $page (@$pages) {
@@ -1145,23 +1152,19 @@ sub file_upload {
     my $c = $form->{c};
     my $saved_data = $form->saved_data;
 
-    my $receipts = $c->req->upload($field);
-    if ( $receipts ) {
-        my $cfg = FixMyStreet->config('PHOTO_STORAGE_OPTIONS');
-        my $dir = $cfg ? $cfg->{UPLOAD_DIR} : FixMyStreet->config('UPLOAD_DIR');
-        $dir = path($dir, "claims_files")->absolute(FixMyStreet->path_to())->mkdir;
-
-        FixMyStreet::PhotoStorage::base64_decode_upload($c, $receipts);
-        my ($p, $n, $ext) = fileparse($receipts->filename, qr/\.[^.]*/);
-        my $key = sha1_hex($receipts->slurp) . $ext;
-        my $out = $dir->child($key);
-        unless (copy($receipts->tempname, $out)) {
+    my $upload = $c->req->upload($field);
+    if ( $upload ) {
+        FixMyStreet::PhotoStorage::base64_decode_upload($c, $upload);
+        my ($p, $n, $ext) = fileparse($upload->filename, qr/\.[^.]*/);
+        my $key = sha1_hex($upload->slurp) . $ext;
+        my $out = $form->upload_dir->child($key);
+        unless (copy($upload->tempname, $out)) {
             $c->log->info('Couldn\'t copy temp file to destination: ' . $!);
             $c->stash->{photo_error} = _("Sorry, we couldn't save your file(s), please try again.");
             return;
         }
         # Then store the file hashes along with the original filenames for display
-        $saved_data->{$field} = { files => $key, filenames => [ $receipts->raw_basename ] };
+        $saved_data->{$field} = { files => $key, filenames => [ $upload->raw_basename ] };
     }
 }
 

perllib/FixMyStreet/App/Form/Field/FileIdUpload.pm

@@ -1,18 +1,53 @@
+=head1 NAME
+
+FixMyStreet::App::Form::Field::FileIdUpload - file upload field via ID
+
+=head1 SYNOPSIS
+
+Our Forms upload files by storing them in the upload directory and then using a
+hash of the contents as the ID to pass around in multi-step forms etc. Here we
+subclass the Upload field to validate the presence of the data in the various
+places it could be, not assuming an Upload object is supplied as the default.
+
+=head1 DESCRIPTION
+
+=cut
+
 package FixMyStreet::App::Form::Field::FileIdUpload;
 use Moose;
 
 extends 'HTML::FormHandler::Field::Upload';
 
+=head2 validate
+
+If we've explicitly said the field is not required, we don't perform any validation.
+
+Otherwise we check we've got something saved in either C<< $self->value >> (the field
+itself), C<< $self->form->saved_data->{$self->name} >> (saved from the previous step),
+or C<< $self->form->params >> (not uploaded, but ID saved in a previous step).
+
+We also check the file isn't zero in size at this point.
+
+=cut
+
 sub validate {
     my ($self) = @_;
 
     return if $self->tag_exists('required') && !$self->get_tag('required');
-    return $self->add_error($self->get_message('upload_file_not_found'))
-        unless ( defined $self->value && defined $self->value->{files}
+
+    my $key = ( defined $self->value && defined $self->value->{files}
                 && $self->value->{files} )
             || ( defined $self->form->saved_data->{$self->name}->{files}
                 && $self->form->saved_data->{$self->name}->{files} )
             || $self->form->params->{$self->name . '_fileid'};
+
+    return $self->add_error($self->get_message('upload_file_not_found'))
+        unless $key;
+
+    my $out = $self->form->upload_dir->child($key);
+    unless ($out->size) {
+        return $self->add_error($self->get_message('upload_file_too_small'));
+    }
 }
 
 __PACKAGE__->meta->make_immutable;

t/app/controller/claims.t

@@ -8,6 +8,7 @@ my $sample_file = path(__FILE__)->parent->child("sample.jpg");
 ok $sample_file->exists, "sample file $sample_file exists";
 my $sample_pdf = path(__FILE__)->parent->child("sample.pdf");
 ok $sample_pdf->exists, "sample file $sample_pdf exists";
+my $sample_blank = path(__FILE__)->parent(3)->child("fixtures", "blank.jpeg");
 
 my $mech = FixMyStreet::TestMech->new;
 
@@ -93,6 +94,10 @@ FixMyStreet::override_config {
             photos => [ $sample_file, undef, Content_Type => 'image/jpeg' ],
             photos2 => [ $sample_file, undef, Content_Type => 'image/jpeg' ],
         } }, "cause screen");
+        $mech->submit_form_ok({ with_fields => { registration => 'rego!', mileage => '20',
+            v5 => [ $sample_blank ], v5_in_name => 'Yes', damage_claim => 'No', vat_reg => 'No',
+        } }, "bad v5 file");
+        $mech->content_contains('File is too small');
         $mech->submit_form_ok({ with_fields => { registration => 'rego!', mileage => '20',
             v5 => [ $sample_pdf, undef, Content_Type => 'application/octet-stream', filename => 'v5.pdf' ],
             v5_in_name => 'Yes', insurer_address => 'insurer address', damage_claim => 'No', vat_reg => 'No',
@@ -105,6 +110,9 @@ FixMyStreet::override_config {
             tyre_damage => 'Yes', tyre_mileage => 20,
         } }, "damage details");
         $mech->content_contains('Review');
+        $mech->submit_form_ok({ form_number => 13 }, "Back to about vehicle page");
+        $mech->submit_form_ok({ with_fields => { continue => 'Continue' } });
+        $mech->submit_form_ok({ with_fields => { continue => 'Continue' } });
         $mech->submit_form_ok({ with_fields => { process => 'summary' } });
         $mech->content_contains('Claim submitted');