From 307f95e0da6d2a6312f6ce364eb100326657b5ca Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Mon, 21 Jul 2025 12:32:23 +0100 Subject: [PATCH 01/27] Updated file column to remove erroneous line --- lib/GADS/Column/File.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index e4f7df746..752618db4 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -35,7 +35,6 @@ has override_types => ( lazy => 1, builder => sub { my $self = shift; - return [] unless $self->has_options; $self->options->{override_types} || []; }, trigger => sub { $_[0]->reset_options }, From 214031d1be48a19ff396725f4db63d9011449159 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 23 Jul 2025 10:18:28 +0100 Subject: [PATCH 02/27] Updated to allow for undef when creating field --- lib/GADS/Column/File.pm | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 752618db4..63689eb62 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -38,6 +38,13 @@ has override_types => ( $self->options->{override_types} || []; }, trigger => sub { $_[0]->reset_options }, + default => sub { [] }, + # Coerce the value to an empty array if it is not defined + coerce => sub { + my $value = shift; + return [] if !$value; + $value; + }, ); has filesize => ( From 8f7f995b3014c1e9ee4bb077abaa1cac2b501f55 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 23 Jul 2025 10:55:26 +0100 Subject: [PATCH 03/27] Updated filecheck for undef values --- lib/GADS/Filecheck.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index d462c8fa2..bdb22c22d 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -52,7 +52,8 @@ sub check_upload sub create_regex { my ($self, $allowed_data) = @_; - return (undef, undef) unless @$allowed_data; + # If we try to make this an array, it will bork if it's undef! + return (undef, undef) unless $allowed_data; my $mimeRegex = '^(' . join('|', map { '\Q' . $_->{name} .'\E' } @$allowed_data) . ')'; my $filetypeRegex = '^(' . join('|', map { '\Q'.$_->{extension}.'\E' } @$allowed_data) . ')$'; From 3928142a894bcdb0e09d19bd4434a42ac8ae42b0 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 23 Jul 2025 13:46:22 +0100 Subject: [PATCH 04/27] Updated rename button to fix where it wasn't working on fields with existing values --- src/frontend/components/button/lib/rename-button.ts | 1 + views/edit.tt | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/frontend/components/button/lib/rename-button.ts b/src/frontend/components/button/lib/rename-button.ts index f1a004082..46b7138d3 100644 --- a/src/frontend/components/button/lib/rename-button.ts +++ b/src/frontend/components/button/lib/rename-button.ts @@ -111,6 +111,7 @@ class RenameButton { .split('.') .slice(0, -1) .join('.'); + console.log("Original file name:", original); $(`#current-${id}`) .addClass('hidden') .attr('aria-hidden', 'true'); diff --git a/views/edit.tt b/views/edit.tt index bbbd49709..0c3de8e13 100755 --- a/views/edit.tt +++ b/views/edit.tt @@ -879,9 +879,11 @@ '
    '; FOREACH file IN editvalue.files; '
  • '; + '
    '; + '
    '; ''; INCLUDE fields/sub/checkbox.tt - id = "file_checkbox_" _ col.id _ "_" _ file.id + id = "file-" _ file.id name = field label = "Include file." value = file.id @@ -895,10 +897,13 @@ ''; ''; 'Current file name: '; - ''; + ''; file.name | html; - '.'; + ''; + ''; ''; + '
    '; + '
    '; '
  • '; END; ''; From c55f348cd5f51b07e585ba1c0228e889981d1d16 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 25 Jul 2025 12:08:59 +0100 Subject: [PATCH 05/27] Updated options to allow for read-only options and updated filecheck --- lib/GADS.pm | 6 ++++-- lib/GADS/Column.pm | 21 +++++++++++++++------ lib/GADS/Column/Autocur.pm | 7 ++++++- lib/GADS/Column/Calc.pm | 6 +++++- lib/GADS/Column/Curval.pm | 11 ++++++++++- lib/GADS/Column/Date.pm | 7 ++++++- lib/GADS/Column/Daterange.pm | 6 +++++- lib/GADS/Column/File.pm | 14 ++++++-------- lib/GADS/Column/Intgr.pm | 6 +++++- lib/GADS/Column/Person.pm | 9 ++++++++- lib/GADS/Filecheck.pm | 5 +++-- 11 files changed, 73 insertions(+), 25 deletions(-) diff --git a/lib/GADS.pm b/lib/GADS.pm index c69d289bc..804394763 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -3730,8 +3730,10 @@ prefix '/:layout_name' => sub { multivalue remember link_parent_id topic_id width aggregate group_display/); $column->type(param 'type') unless param('id'); # Can't change type as it would require DBIC resultsets to be removed and re-added - $column->$_(param $_) - foreach @{$column->option_names}; + foreach (@{$column->user_options}) { + print STDERR "Setting user option $_\n"; + $column->$_(param $_) + } $column->display_fields(param 'display_fields'); # Set the layout in the GADS::Filter object, in case the write # doesn't success, in which case the filter will need to be diff --git a/lib/GADS/Column.pm b/lib/GADS/Column.pm index a0bf21a92..2e3014420 100644 --- a/lib/GADS/Column.pm +++ b/lib/GADS/Column.pm @@ -248,14 +248,14 @@ sub reset_options # Force each option to build now to capture its value, otherwise if it # hasn't already been built then the options hash will be lost and it will # use its default value - $self->$_ foreach @{$self->option_names}; + $self->$_ foreach keys %{$self->option_names}; $self->clear_options; } sub _build_options { my $self = shift; my $options = {}; - foreach my $option_name (@{$self->option_names}) + foreach my $option_name (keys %{$self->option_names}) { $options->{$option_name} = $self->$option_name; } @@ -264,8 +264,17 @@ sub _build_options has option_names => ( is => 'ro', - isa => ArrayRef, - default => sub { [] }, + isa => HashRef, + default => sub { {} }, +); + +has user_options => ( + is => 'lazy', + isa => ArrayRef[Str], + builder => sub { + my $self = shift; + [grep { $self->option_names->{$_} } keys %{$self->option_names}]; + }, ); has ordering => ( @@ -1673,7 +1682,7 @@ sub import_hash notice __x"Update: filter from {old} to {new} for {name}", old => $self->filter->as_json, new => $values->{filter}, name => $self->name if $report && $self->filter->changed; - foreach my $option (@{$self->option_names}) + foreach my $option (keys %{$self->option_names}) { notice __x"Update: {option} from {old} to {new} for {name}", option => $option, old => $self->$option, new => $values->{$option}, name => $self->name @@ -1726,7 +1735,7 @@ sub export_hash }; } $return->{display_fields} = \@display_fields; - foreach my $option (@{$self->option_names}) + foreach my $option (keys %{$self->option_names}) { $return->{$option} = $self->$option; } diff --git a/lib/GADS/Column/Autocur.pm b/lib/GADS/Column/Autocur.pm index 1af3316e1..b7635e138 100644 --- a/lib/GADS/Column/Autocur.pm +++ b/lib/GADS/Column/Autocur.pm @@ -29,7 +29,12 @@ extends 'GADS::Column::Curcommon'; with 'GADS::Role::Curcommon::RelatedField'; has '+option_names' => ( - default => sub { [qw/override_permissions limit_rows/] }, + default => sub { + +{ + override_permissions => 1, + limit_rows => 1, + } + }, ); has '+multivalue' => ( diff --git a/lib/GADS/Column/Calc.pm b/lib/GADS/Column/Calc.pm index 0fab15a86..0f0a9fd18 100644 --- a/lib/GADS/Column/Calc.pm +++ b/lib/GADS/Column/Calc.pm @@ -33,7 +33,11 @@ has '+type' => ( ); has '+option_names' => ( - default => sub { [qw/show_in_edit/] }, + default => sub { + +{ + show_in_edit => 1, + } + }, ); has show_in_edit => ( diff --git a/lib/GADS/Column/Curval.pm b/lib/GADS/Column/Curval.pm index 5165cfb80..d9c5017cd 100644 --- a/lib/GADS/Column/Curval.pm +++ b/lib/GADS/Column/Curval.pm @@ -30,7 +30,16 @@ extends 'GADS::Column::Curcommon'; with 'GADS::Role::Curcommon::CurvalMulti'; has '+option_names' => ( - default => sub { [qw/override_permissions value_selector show_add delete_not_used limit_rows show_view_all/] }, + default => sub { + +{ + override_permissions => 1, + value_selector => 1, + show_add => 1, + delete_not_used => 1, + limit_rows => 1, + show_view_all => 1, + } + }, ); has show_view_all => ( diff --git a/lib/GADS/Column/Date.pm b/lib/GADS/Column/Date.pm index c9e1f69e4..c671ab26c 100644 --- a/lib/GADS/Column/Date.pm +++ b/lib/GADS/Column/Date.pm @@ -46,7 +46,12 @@ has '+has_multivalue_plus' => ( ); has '+option_names' => ( - default => sub { [qw/show_datepicker default_today/] }, + default => sub { + +{ + show_datepicker => 1, + default_today => 1, + } + }, ); has show_datepicker => ( diff --git a/lib/GADS/Column/Daterange.pm b/lib/GADS/Column/Daterange.pm index 5f0dd26e5..e06224d91 100644 --- a/lib/GADS/Column/Daterange.pm +++ b/lib/GADS/Column/Daterange.pm @@ -45,7 +45,11 @@ has '+has_multivalue_plus' => ( ); has '+option_names' => ( - default => sub { [qw/show_datepicker/] }, + default => sub { + +{ + show_datepicker => 1, + } + }, ); sub _build_retrieve_fields diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 63689eb62..063dba05f 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -26,7 +26,11 @@ use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; extends 'GADS::Column'; has '+option_names' => ( - default => sub { [qw/override_types/] } + default => sub { + +{ + override_types => 0, + } + } ); has override_types => ( @@ -35,16 +39,10 @@ has override_types => ( lazy => 1, builder => sub { my $self = shift; + return [] unless $self->has_options; $self->options->{override_types} || []; }, trigger => sub { $_[0]->reset_options }, - default => sub { [] }, - # Coerce the value to an empty array if it is not defined - coerce => sub { - my $value = shift; - return [] if !$value; - $value; - }, ); has filesize => ( diff --git a/lib/GADS/Column/Intgr.pm b/lib/GADS/Column/Intgr.pm index c8324ae4c..edee0c5db 100644 --- a/lib/GADS/Column/Intgr.pm +++ b/lib/GADS/Column/Intgr.pm @@ -39,7 +39,11 @@ has '+return_type' => ( ); has '+option_names' => ( - default => sub { [qw/show_calculator/] }, + default => sub { + +{ + show_calculator => 1, + } + }, ); has show_calculator => ( diff --git a/lib/GADS/Column/Person.pm b/lib/GADS/Column/Person.pm index 214e8eedb..df9624061 100644 --- a/lib/GADS/Column/Person.pm +++ b/lib/GADS/Column/Person.pm @@ -73,7 +73,14 @@ has '+fixedvals' => ( ); has '+option_names' => ( - default => sub { [qw/default_to_login notify_on_selection notify_on_selection_message notify_on_selection_subject/] }, + default => sub { + +{ + default_to_login => 1, + notify_on_selection => 1, + notify_on_selection_subject => 1, + notify_on_selection_message => 1, + } + }, ); has '+can_multivalue' => ( diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index bdb22c22d..cee50e2a8 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -55,8 +55,9 @@ sub create_regex # If we try to make this an array, it will bork if it's undef! return (undef, undef) unless $allowed_data; - my $mimeRegex = '^(' . join('|', map { '\Q' . $_->{name} .'\E' } @$allowed_data) . ')'; - my $filetypeRegex = '^(' . join('|', map { '\Q'.$_->{extension}.'\E' } @$allowed_data) . ')$'; + # Putting in \Q and \E to escape any special characters in the mimetype or extension breaks this - I don't know why + my $mimeRegex = '^(' . join('|', map { $_->{name} } @$allowed_data) . ')'; + my $filetypeRegex = '^(' . join('|', map { $_->{extension} } @$allowed_data) . ')$'; return (qr/$mimeRegex/, qr/$filetypeRegex/); } From 42e24b4d8331040c3dba0cf54fc175618c2af50f Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 25 Jul 2025 13:22:35 +0100 Subject: [PATCH 06/27] Removed debug statement --- lib/GADS.pm | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/GADS.pm b/lib/GADS.pm index 804394763..4a61313c5 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -3730,10 +3730,8 @@ prefix '/:layout_name' => sub { multivalue remember link_parent_id topic_id width aggregate group_display/); $column->type(param 'type') unless param('id'); # Can't change type as it would require DBIC resultsets to be removed and re-added - foreach (@{$column->user_options}) { - print STDERR "Setting user option $_\n"; - $column->$_(param $_) - } + $column->$_(param $_) + foreach @{$column->user_options}; $column->display_fields(param 'display_fields'); # Set the layout in the GADS::Filter object, in case the write # doesn't success, in which case the filter will need to be From 0d82cfbc0572898ce6419c1eb24b1e92b067841b Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 25 Jul 2025 15:40:43 +0100 Subject: [PATCH 07/27] Updated add-extension script, and upload control --- bin/add-extension.pl | 2 +- src/frontend/js/lib/util/upload/UploadControl.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bin/add-extension.pl b/bin/add-extension.pl index 6cd8cc361..ff2d31791 100755 --- a/bin/add-extension.pl +++ b/bin/add-extension.pl @@ -72,6 +72,7 @@ # Make sure the user wants to add the data to the selected column. print "Do you want to add mime type $mimetype with extension $extension to ". $result->name ." (y/n): "; my $answer = ; +chomp $answer; # Do a check for y if ( $answer =~ /^y/i ) { @@ -79,7 +80,6 @@ 'name' => $mimetype, 'extension' => $extension, }; - $result->override_types([]) unless $result->override_types; my $types = $result->override_types; push(@$types, $type) unless grep { $_->{extension} eq $type->{extension} && $_->{name} eq $type->{name} } @$types; $result->override_types($types); diff --git a/src/frontend/js/lib/util/upload/UploadControl.ts b/src/frontend/js/lib/util/upload/UploadControl.ts index ec2178fad..455def8d4 100644 --- a/src/frontend/js/lib/util/upload/UploadControl.ts +++ b/src/frontend/js/lib/util/upload/UploadControl.ts @@ -71,7 +71,10 @@ class Uploader { request.open(this.method, this.url.toString()); request.onabort = () => reject('aborted'); request.onerror = () => reject('error'); - request.onprogress = (e: ProgressEvent) => this.onProgressCallback?.(e.loaded, e.total); + request.upload.onprogress = (e: ProgressEvent) => { + if(!e.lengthComputable) this.onProgressCallback?.(e.loaded, 0); + this.onProgressCallback?.(e.loaded, e.total); + } request.onreadystatechange = () => { if (request.readyState === 4) { if (request.status === 200) { From cdf7def1c89e4fa566346c8d1b05904f906f41ae Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 25 Jul 2025 16:48:40 +0100 Subject: [PATCH 08/27] Added changes to files --- lib/GADS/Column/File.pm | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 063dba05f..c1ce5b9df 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -23,6 +23,10 @@ use MIME::Base64 qw/decode_base64/; use Moo; use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; +use MIME::Base64 qw/encode_base64/; + +use Data::Dump qw/pp/; + extends 'GADS::Column'; has '+option_names' => ( @@ -84,15 +88,23 @@ sub validate { my ($self, $value, %options) = @_; return 1 if !$value; + print STDERR pp %options; + if ($value !~ /^[0-9]+$/ || !$self->schema->resultset('Fileval')->find($value)) { return 0 unless $options{fatal}; - # Check that the file type is allowed here as well - my $filecheck = GADS::Filecheck->instance; - $filecheck->check_upload($self->name, $self->content, extra_types => $self->override_types); error __x"'{int}' is not a valid file ID for '{col}'", int => $value, col => $self->name; } + + # Check that the file type is allowed here as well + my $filecheck = GADS::Filecheck->instance; + my $fileval = $self->schema->resultset('Fileval')->find($value); + my $content = encode_base64($fileval->content); + my $name = $fileval->name; + print STDERR "Checking upload for column " . $self->name . " with filename " . $name . "\n"; + $filecheck->check_upload($name, $content, extra_types => $self->override_types); + 1; } From 32a437d76ca4e52ae72d1a8b5ef5e06b2970ccf0 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 25 Jul 2025 16:51:19 +0100 Subject: [PATCH 09/27] Updated file check --- lib/GADS/Column/File.pm | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index c1ce5b9df..063dba05f 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -23,10 +23,6 @@ use MIME::Base64 qw/decode_base64/; use Moo; use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; -use MIME::Base64 qw/encode_base64/; - -use Data::Dump qw/pp/; - extends 'GADS::Column'; has '+option_names' => ( @@ -88,23 +84,15 @@ sub validate { my ($self, $value, %options) = @_; return 1 if !$value; - print STDERR pp %options; - if ($value !~ /^[0-9]+$/ || !$self->schema->resultset('Fileval')->find($value)) { return 0 unless $options{fatal}; + # Check that the file type is allowed here as well + my $filecheck = GADS::Filecheck->instance; + $filecheck->check_upload($self->name, $self->content, extra_types => $self->override_types); error __x"'{int}' is not a valid file ID for '{col}'", int => $value, col => $self->name; } - - # Check that the file type is allowed here as well - my $filecheck = GADS::Filecheck->instance; - my $fileval = $self->schema->resultset('Fileval')->find($value); - my $content = encode_base64($fileval->content); - my $name = $fileval->name; - print STDERR "Checking upload for column " . $self->name . " with filename " . $name . "\n"; - $filecheck->check_upload($name, $content, extra_types => $self->override_types); - 1; } From 2f8e5c54e260d339ab8338fa0d1eb691c43f5f0a Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 25 Jul 2025 16:51:51 +0100 Subject: [PATCH 10/27] Updated filecheck --- lib/GADS/Column/File.pm | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 063dba05f..091bb5d84 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -23,6 +23,8 @@ use MIME::Base64 qw/decode_base64/; use Moo; use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; +use MIME::Base64 qw/encode_base64/; + extends 'GADS::Column'; has '+option_names' => ( @@ -87,12 +89,18 @@ sub validate if ($value !~ /^[0-9]+$/ || !$self->schema->resultset('Fileval')->find($value)) { return 0 unless $options{fatal}; - # Check that the file type is allowed here as well - my $filecheck = GADS::Filecheck->instance; - $filecheck->check_upload($self->name, $self->content, extra_types => $self->override_types); error __x"'{int}' is not a valid file ID for '{col}'", int => $value, col => $self->name; } + + # Check that the file type is allowed here as well + my $filecheck = GADS::Filecheck->instance; + my $fileval = $self->schema->resultset('Fileval')->find($value); + my $content = encode_base64($fileval->content); + my $name = $fileval->name; + print STDERR "Checking upload for column " . $self->name . " with filename " . $name . "\n"; + $filecheck->check_upload($name, $content, extra_types => $self->override_types); + 1; } From f41243fc4568be186c8138aebeda2b6125a8e378 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Mon, 28 Jul 2025 11:59:53 +0100 Subject: [PATCH 11/27] Updated user configurable hash to more sensible values --- lib/GADS/Column.pm | 14 +++++++------- lib/GADS/Column/Autocur.pm | 12 +++++++----- lib/GADS/Column/Calc.pm | 8 ++++---- lib/GADS/Column/Curval.pm | 27 +++++++++++++++++++-------- lib/GADS/Column/Date.pm | 11 +++++++---- lib/GADS/Column/Daterange.pm | 7 ++++--- lib/GADS/Column/File.pm | 8 +++++--- lib/GADS/Column/Intgr.pm | 7 ++++--- lib/GADS/Column/Person.pm | 19 +++++++++++++------ 9 files changed, 70 insertions(+), 43 deletions(-) diff --git a/lib/GADS/Column.pm b/lib/GADS/Column.pm index 2e3014420..6691922f8 100644 --- a/lib/GADS/Column.pm +++ b/lib/GADS/Column.pm @@ -248,14 +248,14 @@ sub reset_options # Force each option to build now to capture its value, otherwise if it # hasn't already been built then the options hash will be lost and it will # use its default value - $self->$_ foreach keys %{$self->option_names}; + $self->$_ foreach map { $_->{name} } @{$self->option_names}; $self->clear_options; } sub _build_options { my $self = shift; my $options = {}; - foreach my $option_name (keys %{$self->option_names}) + foreach my $option_name (map {$_->{name}} @{$self->option_names}) { $options->{$option_name} = $self->$option_name; } @@ -264,8 +264,8 @@ sub _build_options has option_names => ( is => 'ro', - isa => HashRef, - default => sub { {} }, + isa => ArrayRef, + default => sub { [] }, ); has user_options => ( @@ -273,7 +273,7 @@ has user_options => ( isa => ArrayRef[Str], builder => sub { my $self = shift; - [grep { $self->option_names->{$_} } keys %{$self->option_names}]; + [map { $_->{name} } grep {$_->{user_configurable}} @{$self->option_names}]; }, ); @@ -1682,7 +1682,7 @@ sub import_hash notice __x"Update: filter from {old} to {new} for {name}", old => $self->filter->as_json, new => $values->{filter}, name => $self->name if $report && $self->filter->changed; - foreach my $option (keys %{$self->option_names}) + foreach my $option (map {$_->{name}} @{$self->option_names}) { notice __x"Update: {option} from {old} to {new} for {name}", option => $option, old => $self->$option, new => $values->{$option}, name => $self->name @@ -1735,7 +1735,7 @@ sub export_hash }; } $return->{display_fields} = \@display_fields; - foreach my $option (keys %{$self->option_names}) + foreach my $option (map {$_->{name}} @{$self->option_names}) { $return->{$option} = $self->$option; } diff --git a/lib/GADS/Column/Autocur.pm b/lib/GADS/Column/Autocur.pm index b7635e138..ece4906c7 100644 --- a/lib/GADS/Column/Autocur.pm +++ b/lib/GADS/Column/Autocur.pm @@ -29,11 +29,13 @@ extends 'GADS::Column::Curcommon'; with 'GADS::Role::Curcommon::RelatedField'; has '+option_names' => ( - default => sub { - +{ - override_permissions => 1, - limit_rows => 1, - } + default => sub { [{ + name => 'override_permissions', + user_configurable => 1, + }, { + name => 'limit_rows', + user_configurable => 1, + }] }, ); diff --git a/lib/GADS/Column/Calc.pm b/lib/GADS/Column/Calc.pm index 0f0a9fd18..b71682afc 100644 --- a/lib/GADS/Column/Calc.pm +++ b/lib/GADS/Column/Calc.pm @@ -33,10 +33,10 @@ has '+type' => ( ); has '+option_names' => ( - default => sub { - +{ - show_in_edit => 1, - } + default => sub {[{ + name => 'show_in_edit', + user_configurable => 1, + }] }, ); diff --git a/lib/GADS/Column/Curval.pm b/lib/GADS/Column/Curval.pm index d9c5017cd..ef9e56869 100644 --- a/lib/GADS/Column/Curval.pm +++ b/lib/GADS/Column/Curval.pm @@ -31,14 +31,25 @@ with 'GADS::Role::Curcommon::CurvalMulti'; has '+option_names' => ( default => sub { - +{ - override_permissions => 1, - value_selector => 1, - show_add => 1, - delete_not_used => 1, - limit_rows => 1, - show_view_all => 1, - } + [{ + name => 'override_permissions', + user_configurable => 1, + }, { + name => 'value_selector', + user_configurable => 1, + }, { + name =>'show_add', + user_configurable => 1, + }, { + name => 'delete_not_used', + user_configurable => 1, + }, { + name => 'limit_rows', + user_configurable => 1, + }, { + name => 'show_view_all', + user_configurable => 1, + }] }, ); diff --git a/lib/GADS/Column/Date.pm b/lib/GADS/Column/Date.pm index c671ab26c..241c76a68 100644 --- a/lib/GADS/Column/Date.pm +++ b/lib/GADS/Column/Date.pm @@ -47,10 +47,13 @@ has '+has_multivalue_plus' => ( has '+option_names' => ( default => sub { - +{ - show_datepicker => 1, - default_today => 1, - } + [{ + name => 'show_datepicker', + user_configurable => 1, + }, { + name => 'default_today', + user_configurable => 1, + }] }, ); diff --git a/lib/GADS/Column/Daterange.pm b/lib/GADS/Column/Daterange.pm index e06224d91..9dde3df19 100644 --- a/lib/GADS/Column/Daterange.pm +++ b/lib/GADS/Column/Daterange.pm @@ -46,9 +46,10 @@ has '+has_multivalue_plus' => ( has '+option_names' => ( default => sub { - +{ - show_datepicker => 1, - } + [{ + name =>'show_datepicker', + user_configurable => 1, + }] }, ); diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 091bb5d84..d4582d258 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -22,6 +22,7 @@ use Log::Report 'linkspace'; use MIME::Base64 qw/decode_base64/; use Moo; use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; +use GADS::Filecheck; use MIME::Base64 qw/encode_base64/; @@ -29,9 +30,10 @@ extends 'GADS::Column'; has '+option_names' => ( default => sub { - +{ - override_types => 0, - } + [+{ + name => 'override_types', + user_configurable => 0, + }] } ); diff --git a/lib/GADS/Column/Intgr.pm b/lib/GADS/Column/Intgr.pm index edee0c5db..44020d434 100644 --- a/lib/GADS/Column/Intgr.pm +++ b/lib/GADS/Column/Intgr.pm @@ -40,9 +40,10 @@ has '+return_type' => ( has '+option_names' => ( default => sub { - +{ - show_calculator => 1, - } + [{ + name => 'show_calculator', + user_configurable => 1, + }] }, ); diff --git a/lib/GADS/Column/Person.pm b/lib/GADS/Column/Person.pm index df9624061..83d7af860 100644 --- a/lib/GADS/Column/Person.pm +++ b/lib/GADS/Column/Person.pm @@ -74,12 +74,19 @@ has '+fixedvals' => ( has '+option_names' => ( default => sub { - +{ - default_to_login => 1, - notify_on_selection => 1, - notify_on_selection_subject => 1, - notify_on_selection_message => 1, - } + [{ + name => 'default_to_login', + user_configurable => 1, + }, { + name => 'notify_on_selection', + user_configurable => 1, + }, { + name => 'notify_on_selection_subject', + user_configurable => 1, + }, { + name => 'notify_on_selection_message', + user_configurable =>1, + }] }, ); From c7a322002a0498af8f83cab731115ae919f35cc0 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Mon, 28 Jul 2025 16:29:28 +0100 Subject: [PATCH 12/27] Added better progress handling for multiple files --- .../form-group/input/lib/documentComponent.ts | 45 +++++++------ .../js/lib/util/ProgressHandler/index.ts | 1 + .../util/ProgressHandler/lib/Progress.test.ts | 29 ++++++++ .../lib/util/ProgressHandler/lib/Progress.ts | 63 +++++++++++++++++ .../ProgressHandler/lib/ProgressHandler.ts | 67 +++++++++++++++++++ src/frontend/js/lib/util/common/Renderable.ts | 10 +++ .../js/lib/util/upload/UploadControl.test.ts | 2 +- .../js/lib/util/upload/UploadControl.ts | 17 ++--- 8 files changed, 203 insertions(+), 31 deletions(-) create mode 100644 src/frontend/js/lib/util/ProgressHandler/index.ts create mode 100644 src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts create mode 100644 src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts create mode 100644 src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts create mode 100644 src/frontend/js/lib/util/common/Renderable.ts diff --git a/src/frontend/components/form-group/input/lib/documentComponent.ts b/src/frontend/components/form-group/input/lib/documentComponent.ts index 5f8aa43d8..84a39da8e 100644 --- a/src/frontend/components/form-group/input/lib/documentComponent.ts +++ b/src/frontend/components/form-group/input/lib/documentComponent.ts @@ -9,6 +9,7 @@ import { fromJson } from 'util/common'; import { FileDropEvent } from 'util/filedrag'; import ErrorHandler from 'util/errorHandler'; import { isString } from 'util/typechecks'; +import { ProgressHandler } from 'util/ProgressHandler'; interface FileData { id: number | string; @@ -26,10 +27,13 @@ class DocumentComponent { readonly el: JQuery; readonly fileInput: JQuery; errors: (string|Error)[]; - handler: ErrorHandler; + errorHandler: ErrorHandler; + progressHandler: ProgressHandler; + constructor(el: JQuery | HTMLElement) { this.el = $(el); + this.progressHandler = new ProgressHandler(this.el[0]); this.el.closest('.fieldset').find('.rename').renameButton().on('rename', async (ev: RenameEvent) => { if (!ev) throw new Error("e is not a RenameEvent - this shouldn't happen!") const $target = $(ev.target); @@ -46,12 +50,12 @@ class DocumentComponent { const dropTarget = this.el.closest('.file-upload'); const columnId = this.el.closest('.linkspace-field')?.data('column-id') ?? 0; - this.handler = new ErrorHandler(dropTarget[0]); + this.errorHandler = new ErrorHandler(dropTarget[0]); if (dropTarget) { const dragOptions = { allowMultiple: true }; dropTarget.filedrag(dragOptions).on('fileDrop', ({ file }: FileDropEvent) => { - this.handler.clearErrors(); + this.errorHandler.clearErrors(); logging.info('File dropped', file); this.handleAjaxUpload(url, csrf_token, file, columnId); }); @@ -67,28 +71,31 @@ class DocumentComponent { const file = ev.target.files![0]; if (!file || file === undefined || !file.name) return; const formData = formdataMapper({ file, csrf_token, column_id: columnId }); - this.showContainer(); - upload(url, formData, 'POST', this.showProgress.bind(this)).then((data)=>{ + // this.showContainer(); + upload(url, formData, file.name, 'POST', this.showProgress.bind(this)).then((data)=>{ this.addFileToField({ id: data.id, name: data.filename }); }).catch((e) => { + console.log('Error uploading file', e); if(JSON.parse(e as string)?.message) e = JSON.parse(e as string).message; - this.handler.addError(e); + this.errorHandler.addError(e); }); }); } - showProgress(loaded: number, total: number) { + showProgress(name: string, loaded: number, total: number) { let uploadProgression = (loaded / total) * 100; if (uploadProgression == Infinity) { // This will occur when there is an error uploading the file or the file is empty uploadProgression = 100; } - this.el?.find('.progress-bar__container') - .css('width', undefined) - .removeClass('progress-bar__container--fail'); - this.el?.find('.progress-bar__percentage').html(uploadProgression === 100 ? 'complete' : `${uploadProgression}%`); - this.el?.find('.progress-bar__progress').css('width', `${uploadProgression}%`); + console.log(`Upload progress for ${name}: ${uploadProgression}%`); + this.progressHandler.updateProgress(name, uploadProgression); + // this.el?.find('.progress-bar__container') + // .css('width', undefined) + // .removeClass('progress-bar__container--fail'); + // this.el?.find('.progress-bar__percentage').html(uploadProgression === 100 ? 'complete' : `${uploadProgression}%`); + // this.el?.find('.progress-bar__progress').css('width', `${uploadProgression}%`); } handleAjaxUpload(uri: string, csrf_token: string, file: File, columnId: number) { @@ -98,24 +105,18 @@ class DocumentComponent { const fileData = formdataMapper({ file, csrf_token, column_id: columnId }); this.showContainer(); - upload(uri, fileData, 'POST', this.showProgress).then((data) => { + upload(uri, fileData, file.name, 'POST', this.showProgress.bind(this)).then((data) => { this.addFileToField({ id: data.id, name: data.filename }); - }).then( - () => { - this.hideException(); - const container = $(this.el.find('.progress-bar__container')); - container.hide(); - } - ).catch((e) => { + }).catch((e) => { const error = e instanceof Error ? e.message : e as string ?? e.toString(); // If the error is a string, try to parse it as JSON if(isString(error) && fromJson(error)) { const decodedError = fromJson(error) if( 'message' in decodedError && decodedError.message ) { - this.handler.addError(new Error(`Unable to upload ${file.name}: ${decodedError.message}`)); + this.errorHandler.addError(new Error(`Unable to upload ${file.name}: ${decodedError.message}`)); } } else { - this.handler.addError(new Error(`Unable to upload ${file.name}: ${error}`)); + this.errorHandler.addError(new Error(`Unable to upload ${file.name}: ${error}`)); } const container = $(this.el.find('.progress-bar__container')); container.hide(); diff --git a/src/frontend/js/lib/util/ProgressHandler/index.ts b/src/frontend/js/lib/util/ProgressHandler/index.ts new file mode 100644 index 000000000..edb27dfb1 --- /dev/null +++ b/src/frontend/js/lib/util/ProgressHandler/index.ts @@ -0,0 +1 @@ +export { ProgressHandler } from './lib/ProgressHandler'; diff --git a/src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts b/src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts new file mode 100644 index 000000000..a9c224e3c --- /dev/null +++ b/src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts @@ -0,0 +1,29 @@ +import { describe, it, expect } from '@jest/globals'; +import { Progress } from './Progress'; + +describe('Progress', () => { + it('should initialize with default values', () => { + const progress = new Progress(document.body, 'test-progress', 100); + expect(progress.value).toBe(0); + }); + + it('should update progress correctly', () => { + const progress = new Progress(document.body, 'test-progress', 100); + progress.value=50; + expect(progress.value).toBe(50); + }); + + it('should complete progress correctly', () => { + const progress = new Progress(document.body, 'test-progress', 100); + progress.value=100; + expect(progress.value).toBe(100); + }); + + it('should handle invalid updates gracefully', () => { + const progress = new Progress(document.body, 'test-progress', 100); + progress.value=-10; + expect(progress.value).toBe(0); // Should not go below 0 + progress.value=150; + expect(progress.value).toBe(100); // Should not exceed total + }); +}); \ No newline at end of file diff --git a/src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts b/src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts new file mode 100644 index 000000000..8596260db --- /dev/null +++ b/src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts @@ -0,0 +1,63 @@ +import { Renderable } from "../../common/Renderable"; + +/** + * A class to handle progress rendering in a parent HTML element. + */ +export class Progress implements Renderable { + private _value = 0; + /** + * Flag to indicate if the progress value has changed since the last render. + * This is used to avoid unnecessary re-rendering. + * @type {boolean} + * @private + * @default false + */ + private changed = false; + + /** + * Gets the current value of the progress. + * @returns {number} The current value of the progress. + */ + get value(): number { + return this._value; + } + + /** + * Sets the value of the progress. If the value is out of bounds (less than 0 or greater than total), + * it will be clamped to the nearest valid value. + * @param {number} newValue - The new value to set for the progress. + */ + set value(newValue: number) { + if (newValue < 0 || newValue > this.total) { + newValue = Math.max(0, Math.min(newValue, this.total)); + } + this._value = newValue; + this.changed = true; + } + + /** + * Create a new Progress instance. + * @param {HTMLElement} parent The parent HTML element where the progress bar will be rendered. + * @param {string} identifier The identifier for the progress bar, used for accessibility and querying. + * @param {number} total The total value that the progress bar represents. + */ + constructor(private readonly parent: HTMLElement, private readonly identifier: string, private readonly total: number) { } + + /** @inheritdoc */ + render() { + if (!this.changed) return; + this.changed = false; + const value = Math.round((this.value / this.total) * 100); + const progressbar = $(`[data-identifier="${this.identifier}"]`); + if (progressbar.length === 0) { + this.parent.insertAdjacentHTML('beforeend', + `
    +
    +
    `); + } else { + progressbar.attr('aria-valuenow', this.value.toString()); + progressbar.find('.progress-bar').css('width', `${value}%`); + progressbar.find('.progress-bar').text(`${value}%`); + } + } +} diff --git a/src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts b/src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts new file mode 100644 index 000000000..09f26d613 --- /dev/null +++ b/src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts @@ -0,0 +1,67 @@ +import { Progress } from "./Progress"; +import { Renderable } from "../../common/Renderable"; + +/** + * ProgressHandler is a class that manages multiple progress bars. + */ +export class ProgressHandler implements Renderable { + /** + * A record that holds all progress bars indexed by their identifiers. + * Each key is a string identifier, and the value is an instance of Progress. + */ + private bars: Record; + + /** + * + * @param parent The parent HTML element where the progress bars will be rendered. + */ + public constructor(private readonly parent: HTMLElement) { + this.bars = {}; + } + + /** + * Creates a new progress bar and adds it to the handler. + * @param {string} identifier A unique identifier for the progress bar. + * @param {number} total The total value for the progress bar, default is 100. + */ + public createBar(identifier: string, total: number=100) { + const bar = new Progress(this.parent, identifier, total); + this.bars[identifier] = bar; + this.render(); + } + + /** + * Update the value of an existing progress bar. + * @param identifier The unique identifier of the progress bar to update. + * @param value The new value to set for the progress bar. + */ + public updateProgress(identifier: string, value: number) { + if (this.bars[identifier]) { + this.bars[identifier].value = value; + } else { + console.warn(`Progress bar with identifier "${identifier}" does not exist.`); + } + this.render(); + } + + /** @inheritdoc */ + public render() { + Object.values(this.bars).forEach(bar => bar.render()); + } + + /** + * Remove a progress bar from the handler and the DOM. + * @param {string} identifier The unique identifier of the progress bar to remove. + */ + public remove(identifier: string) { + if (this.bars[identifier]) { + delete this.bars[identifier]; + const progressbar = $(`[data-identifier="${identifier}"]`); + if (progressbar.length > 0) { + progressbar.remove(); + } + } else { + console.warn(`Progress bar with identifier "${identifier}" does not exist.`); + } + } +} diff --git a/src/frontend/js/lib/util/common/Renderable.ts b/src/frontend/js/lib/util/common/Renderable.ts new file mode 100644 index 000000000..a291a9ca8 --- /dev/null +++ b/src/frontend/js/lib/util/common/Renderable.ts @@ -0,0 +1,10 @@ +/** + * Interface for renderable components. + * Classes implementing this interface should provide a render method. + */ +export interface Renderable { + /** + * Renders the component. + */ + render(): void; +} diff --git a/src/frontend/js/lib/util/upload/UploadControl.test.ts b/src/frontend/js/lib/util/upload/UploadControl.test.ts index 51d6faec0..4dfa88875 100644 --- a/src/frontend/js/lib/util/upload/UploadControl.test.ts +++ b/src/frontend/js/lib/util/upload/UploadControl.test.ts @@ -65,7 +65,7 @@ describe('UploadControl', () => { const method = 'POST'; const data = { key: 'value' }; const uploader = new Uploader(url, method); - uploader.onProgress((loaded, total) => { + uploader.onProgress((name, loaded, total) => { expect(loaded).toBe(1); expect(total).toBe(2); }); diff --git a/src/frontend/js/lib/util/upload/UploadControl.ts b/src/frontend/js/lib/util/upload/UploadControl.ts index ec2178fad..289824b80 100644 --- a/src/frontend/js/lib/util/upload/UploadControl.ts +++ b/src/frontend/js/lib/util/upload/UploadControl.ts @@ -5,7 +5,7 @@ import { fromJson } from "util/common"; * @param loaded The number of bytes that have been uploaded * @param total The total number of bytes to upload */ -type ProgressFunction = (loaded: number, total: number) => void; +type ProgressFunction = (name: string, loaded: number, total: number) => void; type RequestMethod = "PUT"|"POST"|"GET"|"DELETE"|"PATCH"; @@ -29,12 +29,14 @@ type XmlHttpRequestLike = { * Upload data to a server endpoint * @param url The endpoint to upload to * @param data The form data to upload + * @param name The name of the file to upload, if applicable * @param method The method to use, either POST, PUT, or GET * @param onProgress A callback that is called when the upload progress changes * @returns The JSON response from the server */ -async function upload(url: string | URL, data?: FormData | object, method: RequestMethod = 'POST', onProgress: ProgressFunction = () => { }): Promise { +async function upload(url: string | URL, data?: FormData | object, name?: string, method: RequestMethod = 'POST', onProgress: ProgressFunction = () => { }): Promise { const uploader = new Uploader(url, method); + uploader.name = name; uploader.onProgress(onProgress); return uploader.upload(data); } @@ -44,12 +46,10 @@ async function upload(url: string | URL, data?: FormData | object, */ class Uploader { private onProgressCallback?: ProgressFunction; - private url: string | URL; - private method: RequestMethod; + name: string; - constructor(url: string | URL, method: RequestMethod) { - this.url = url; - this.method = method; + constructor(private readonly url: string | URL, private readonly method: RequestMethod) { + console.log(`Uploader initialized with URL: ${url} and method: ${method}`); } /** @@ -66,12 +66,13 @@ class Uploader { * @returns A promise that resolves to the JSON response from the server */ async upload(data?: FormData | object): Promise { + if(!this.name) this.name = Math.random().toString(36).substring(2, 15); return new Promise((resolve, reject) => { const request: XmlHttpRequestLike & XMLHttpRequest = new XMLHttpRequest(); request.open(this.method, this.url.toString()); request.onabort = () => reject('aborted'); request.onerror = () => reject('error'); - request.onprogress = (e: ProgressEvent) => this.onProgressCallback?.(e.loaded, e.total); + request.onprogress = (e: ProgressEvent) => this.onProgressCallback?.(this.name!, e.loaded, e.total); request.onreadystatechange = () => { if (request.readyState === 4) { if (request.status === 200) { From c22f795a0ea5ff852be0de294ed3c1ee0e6ed03f Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Tue, 29 Jul 2025 09:43:44 +0100 Subject: [PATCH 13/27] Revert "Added better progress handling for multiple files" This reverts commit c7a322002a0498af8f83cab731115ae919f35cc0. --- .../form-group/input/lib/documentComponent.ts | 45 ++++++------- .../js/lib/util/ProgressHandler/index.ts | 1 - .../util/ProgressHandler/lib/Progress.test.ts | 29 -------- .../lib/util/ProgressHandler/lib/Progress.ts | 63 ----------------- .../ProgressHandler/lib/ProgressHandler.ts | 67 ------------------- src/frontend/js/lib/util/common/Renderable.ts | 10 --- .../js/lib/util/upload/UploadControl.test.ts | 2 +- .../js/lib/util/upload/UploadControl.ts | 17 +++-- 8 files changed, 31 insertions(+), 203 deletions(-) delete mode 100644 src/frontend/js/lib/util/ProgressHandler/index.ts delete mode 100644 src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts delete mode 100644 src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts delete mode 100644 src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts delete mode 100644 src/frontend/js/lib/util/common/Renderable.ts diff --git a/src/frontend/components/form-group/input/lib/documentComponent.ts b/src/frontend/components/form-group/input/lib/documentComponent.ts index 84a39da8e..5f8aa43d8 100644 --- a/src/frontend/components/form-group/input/lib/documentComponent.ts +++ b/src/frontend/components/form-group/input/lib/documentComponent.ts @@ -9,7 +9,6 @@ import { fromJson } from 'util/common'; import { FileDropEvent } from 'util/filedrag'; import ErrorHandler from 'util/errorHandler'; import { isString } from 'util/typechecks'; -import { ProgressHandler } from 'util/ProgressHandler'; interface FileData { id: number | string; @@ -27,13 +26,10 @@ class DocumentComponent { readonly el: JQuery; readonly fileInput: JQuery; errors: (string|Error)[]; - errorHandler: ErrorHandler; - progressHandler: ProgressHandler; - + handler: ErrorHandler; constructor(el: JQuery | HTMLElement) { this.el = $(el); - this.progressHandler = new ProgressHandler(this.el[0]); this.el.closest('.fieldset').find('.rename').renameButton().on('rename', async (ev: RenameEvent) => { if (!ev) throw new Error("e is not a RenameEvent - this shouldn't happen!") const $target = $(ev.target); @@ -50,12 +46,12 @@ class DocumentComponent { const dropTarget = this.el.closest('.file-upload'); const columnId = this.el.closest('.linkspace-field')?.data('column-id') ?? 0; - this.errorHandler = new ErrorHandler(dropTarget[0]); + this.handler = new ErrorHandler(dropTarget[0]); if (dropTarget) { const dragOptions = { allowMultiple: true }; dropTarget.filedrag(dragOptions).on('fileDrop', ({ file }: FileDropEvent) => { - this.errorHandler.clearErrors(); + this.handler.clearErrors(); logging.info('File dropped', file); this.handleAjaxUpload(url, csrf_token, file, columnId); }); @@ -71,31 +67,28 @@ class DocumentComponent { const file = ev.target.files![0]; if (!file || file === undefined || !file.name) return; const formData = formdataMapper({ file, csrf_token, column_id: columnId }); - // this.showContainer(); - upload(url, formData, file.name, 'POST', this.showProgress.bind(this)).then((data)=>{ + this.showContainer(); + upload(url, formData, 'POST', this.showProgress.bind(this)).then((data)=>{ this.addFileToField({ id: data.id, name: data.filename }); }).catch((e) => { - console.log('Error uploading file', e); if(JSON.parse(e as string)?.message) e = JSON.parse(e as string).message; - this.errorHandler.addError(e); + this.handler.addError(e); }); }); } - showProgress(name: string, loaded: number, total: number) { + showProgress(loaded: number, total: number) { let uploadProgression = (loaded / total) * 100; if (uploadProgression == Infinity) { // This will occur when there is an error uploading the file or the file is empty uploadProgression = 100; } - console.log(`Upload progress for ${name}: ${uploadProgression}%`); - this.progressHandler.updateProgress(name, uploadProgression); - // this.el?.find('.progress-bar__container') - // .css('width', undefined) - // .removeClass('progress-bar__container--fail'); - // this.el?.find('.progress-bar__percentage').html(uploadProgression === 100 ? 'complete' : `${uploadProgression}%`); - // this.el?.find('.progress-bar__progress').css('width', `${uploadProgression}%`); + this.el?.find('.progress-bar__container') + .css('width', undefined) + .removeClass('progress-bar__container--fail'); + this.el?.find('.progress-bar__percentage').html(uploadProgression === 100 ? 'complete' : `${uploadProgression}%`); + this.el?.find('.progress-bar__progress').css('width', `${uploadProgression}%`); } handleAjaxUpload(uri: string, csrf_token: string, file: File, columnId: number) { @@ -105,18 +98,24 @@ class DocumentComponent { const fileData = formdataMapper({ file, csrf_token, column_id: columnId }); this.showContainer(); - upload(uri, fileData, file.name, 'POST', this.showProgress.bind(this)).then((data) => { + upload(uri, fileData, 'POST', this.showProgress).then((data) => { this.addFileToField({ id: data.id, name: data.filename }); - }).catch((e) => { + }).then( + () => { + this.hideException(); + const container = $(this.el.find('.progress-bar__container')); + container.hide(); + } + ).catch((e) => { const error = e instanceof Error ? e.message : e as string ?? e.toString(); // If the error is a string, try to parse it as JSON if(isString(error) && fromJson(error)) { const decodedError = fromJson(error) if( 'message' in decodedError && decodedError.message ) { - this.errorHandler.addError(new Error(`Unable to upload ${file.name}: ${decodedError.message}`)); + this.handler.addError(new Error(`Unable to upload ${file.name}: ${decodedError.message}`)); } } else { - this.errorHandler.addError(new Error(`Unable to upload ${file.name}: ${error}`)); + this.handler.addError(new Error(`Unable to upload ${file.name}: ${error}`)); } const container = $(this.el.find('.progress-bar__container')); container.hide(); diff --git a/src/frontend/js/lib/util/ProgressHandler/index.ts b/src/frontend/js/lib/util/ProgressHandler/index.ts deleted file mode 100644 index edb27dfb1..000000000 --- a/src/frontend/js/lib/util/ProgressHandler/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { ProgressHandler } from './lib/ProgressHandler'; diff --git a/src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts b/src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts deleted file mode 100644 index a9c224e3c..000000000 --- a/src/frontend/js/lib/util/ProgressHandler/lib/Progress.test.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { describe, it, expect } from '@jest/globals'; -import { Progress } from './Progress'; - -describe('Progress', () => { - it('should initialize with default values', () => { - const progress = new Progress(document.body, 'test-progress', 100); - expect(progress.value).toBe(0); - }); - - it('should update progress correctly', () => { - const progress = new Progress(document.body, 'test-progress', 100); - progress.value=50; - expect(progress.value).toBe(50); - }); - - it('should complete progress correctly', () => { - const progress = new Progress(document.body, 'test-progress', 100); - progress.value=100; - expect(progress.value).toBe(100); - }); - - it('should handle invalid updates gracefully', () => { - const progress = new Progress(document.body, 'test-progress', 100); - progress.value=-10; - expect(progress.value).toBe(0); // Should not go below 0 - progress.value=150; - expect(progress.value).toBe(100); // Should not exceed total - }); -}); \ No newline at end of file diff --git a/src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts b/src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts deleted file mode 100644 index 8596260db..000000000 --- a/src/frontend/js/lib/util/ProgressHandler/lib/Progress.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { Renderable } from "../../common/Renderable"; - -/** - * A class to handle progress rendering in a parent HTML element. - */ -export class Progress implements Renderable { - private _value = 0; - /** - * Flag to indicate if the progress value has changed since the last render. - * This is used to avoid unnecessary re-rendering. - * @type {boolean} - * @private - * @default false - */ - private changed = false; - - /** - * Gets the current value of the progress. - * @returns {number} The current value of the progress. - */ - get value(): number { - return this._value; - } - - /** - * Sets the value of the progress. If the value is out of bounds (less than 0 or greater than total), - * it will be clamped to the nearest valid value. - * @param {number} newValue - The new value to set for the progress. - */ - set value(newValue: number) { - if (newValue < 0 || newValue > this.total) { - newValue = Math.max(0, Math.min(newValue, this.total)); - } - this._value = newValue; - this.changed = true; - } - - /** - * Create a new Progress instance. - * @param {HTMLElement} parent The parent HTML element where the progress bar will be rendered. - * @param {string} identifier The identifier for the progress bar, used for accessibility and querying. - * @param {number} total The total value that the progress bar represents. - */ - constructor(private readonly parent: HTMLElement, private readonly identifier: string, private readonly total: number) { } - - /** @inheritdoc */ - render() { - if (!this.changed) return; - this.changed = false; - const value = Math.round((this.value / this.total) * 100); - const progressbar = $(`[data-identifier="${this.identifier}"]`); - if (progressbar.length === 0) { - this.parent.insertAdjacentHTML('beforeend', - `
    -
    -
    `); - } else { - progressbar.attr('aria-valuenow', this.value.toString()); - progressbar.find('.progress-bar').css('width', `${value}%`); - progressbar.find('.progress-bar').text(`${value}%`); - } - } -} diff --git a/src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts b/src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts deleted file mode 100644 index 09f26d613..000000000 --- a/src/frontend/js/lib/util/ProgressHandler/lib/ProgressHandler.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { Progress } from "./Progress"; -import { Renderable } from "../../common/Renderable"; - -/** - * ProgressHandler is a class that manages multiple progress bars. - */ -export class ProgressHandler implements Renderable { - /** - * A record that holds all progress bars indexed by their identifiers. - * Each key is a string identifier, and the value is an instance of Progress. - */ - private bars: Record; - - /** - * - * @param parent The parent HTML element where the progress bars will be rendered. - */ - public constructor(private readonly parent: HTMLElement) { - this.bars = {}; - } - - /** - * Creates a new progress bar and adds it to the handler. - * @param {string} identifier A unique identifier for the progress bar. - * @param {number} total The total value for the progress bar, default is 100. - */ - public createBar(identifier: string, total: number=100) { - const bar = new Progress(this.parent, identifier, total); - this.bars[identifier] = bar; - this.render(); - } - - /** - * Update the value of an existing progress bar. - * @param identifier The unique identifier of the progress bar to update. - * @param value The new value to set for the progress bar. - */ - public updateProgress(identifier: string, value: number) { - if (this.bars[identifier]) { - this.bars[identifier].value = value; - } else { - console.warn(`Progress bar with identifier "${identifier}" does not exist.`); - } - this.render(); - } - - /** @inheritdoc */ - public render() { - Object.values(this.bars).forEach(bar => bar.render()); - } - - /** - * Remove a progress bar from the handler and the DOM. - * @param {string} identifier The unique identifier of the progress bar to remove. - */ - public remove(identifier: string) { - if (this.bars[identifier]) { - delete this.bars[identifier]; - const progressbar = $(`[data-identifier="${identifier}"]`); - if (progressbar.length > 0) { - progressbar.remove(); - } - } else { - console.warn(`Progress bar with identifier "${identifier}" does not exist.`); - } - } -} diff --git a/src/frontend/js/lib/util/common/Renderable.ts b/src/frontend/js/lib/util/common/Renderable.ts deleted file mode 100644 index a291a9ca8..000000000 --- a/src/frontend/js/lib/util/common/Renderable.ts +++ /dev/null @@ -1,10 +0,0 @@ -/** - * Interface for renderable components. - * Classes implementing this interface should provide a render method. - */ -export interface Renderable { - /** - * Renders the component. - */ - render(): void; -} diff --git a/src/frontend/js/lib/util/upload/UploadControl.test.ts b/src/frontend/js/lib/util/upload/UploadControl.test.ts index 4dfa88875..51d6faec0 100644 --- a/src/frontend/js/lib/util/upload/UploadControl.test.ts +++ b/src/frontend/js/lib/util/upload/UploadControl.test.ts @@ -65,7 +65,7 @@ describe('UploadControl', () => { const method = 'POST'; const data = { key: 'value' }; const uploader = new Uploader(url, method); - uploader.onProgress((name, loaded, total) => { + uploader.onProgress((loaded, total) => { expect(loaded).toBe(1); expect(total).toBe(2); }); diff --git a/src/frontend/js/lib/util/upload/UploadControl.ts b/src/frontend/js/lib/util/upload/UploadControl.ts index 289824b80..ec2178fad 100644 --- a/src/frontend/js/lib/util/upload/UploadControl.ts +++ b/src/frontend/js/lib/util/upload/UploadControl.ts @@ -5,7 +5,7 @@ import { fromJson } from "util/common"; * @param loaded The number of bytes that have been uploaded * @param total The total number of bytes to upload */ -type ProgressFunction = (name: string, loaded: number, total: number) => void; +type ProgressFunction = (loaded: number, total: number) => void; type RequestMethod = "PUT"|"POST"|"GET"|"DELETE"|"PATCH"; @@ -29,14 +29,12 @@ type XmlHttpRequestLike = { * Upload data to a server endpoint * @param url The endpoint to upload to * @param data The form data to upload - * @param name The name of the file to upload, if applicable * @param method The method to use, either POST, PUT, or GET * @param onProgress A callback that is called when the upload progress changes * @returns The JSON response from the server */ -async function upload(url: string | URL, data?: FormData | object, name?: string, method: RequestMethod = 'POST', onProgress: ProgressFunction = () => { }): Promise { +async function upload(url: string | URL, data?: FormData | object, method: RequestMethod = 'POST', onProgress: ProgressFunction = () => { }): Promise { const uploader = new Uploader(url, method); - uploader.name = name; uploader.onProgress(onProgress); return uploader.upload(data); } @@ -46,10 +44,12 @@ async function upload(url: string | URL, data?: FormData | object, */ class Uploader { private onProgressCallback?: ProgressFunction; - name: string; + private url: string | URL; + private method: RequestMethod; - constructor(private readonly url: string | URL, private readonly method: RequestMethod) { - console.log(`Uploader initialized with URL: ${url} and method: ${method}`); + constructor(url: string | URL, method: RequestMethod) { + this.url = url; + this.method = method; } /** @@ -66,13 +66,12 @@ class Uploader { * @returns A promise that resolves to the JSON response from the server */ async upload(data?: FormData | object): Promise { - if(!this.name) this.name = Math.random().toString(36).substring(2, 15); return new Promise((resolve, reject) => { const request: XmlHttpRequestLike & XMLHttpRequest = new XMLHttpRequest(); request.open(this.method, this.url.toString()); request.onabort = () => reject('aborted'); request.onerror = () => reject('error'); - request.onprogress = (e: ProgressEvent) => this.onProgressCallback?.(this.name!, e.loaded, e.total); + request.onprogress = (e: ProgressEvent) => this.onProgressCallback?.(e.loaded, e.total); request.onreadystatechange = () => { if (request.readyState === 4) { if (request.status === 200) { From 52d15b485bce85e228d288a6b74f4d7047d19e3e Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 11:43:12 +0100 Subject: [PATCH 14/27] Updated to remove file name check --- lib/GADS/Column/File.pm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index d4582d258..a795ec54b 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -100,8 +100,7 @@ sub validate my $fileval = $self->schema->resultset('Fileval')->find($value); my $content = encode_base64($fileval->content); my $name = $fileval->name; - print STDERR "Checking upload for column " . $self->name . " with filename " . $name . "\n"; - $filecheck->check_upload($name, $content, extra_types => $self->override_types); + $filecheck->check_upload($name, $content, extra_types => $self->override_types, check_name => 0); 1; } From 7d38d71f13f43aba604c810aa0a08fe46b475585 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 12:04:41 +0100 Subject: [PATCH 15/27] Fixed failing Jest test --- src/frontend/js/lib/util/upload/UploadControl.ts | 3 +++ src/frontend/testing/globals.definitions.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/frontend/js/lib/util/upload/UploadControl.ts b/src/frontend/js/lib/util/upload/UploadControl.ts index 455def8d4..21501e76e 100644 --- a/src/frontend/js/lib/util/upload/UploadControl.ts +++ b/src/frontend/js/lib/util/upload/UploadControl.ts @@ -23,6 +23,9 @@ type XmlHttpRequestLike = { readyState: number, status: number, responseText: string, + upload?: { + onprogress?: ((e: ProgressEvent) => void) | null + }; }; /** diff --git a/src/frontend/testing/globals.definitions.ts b/src/frontend/testing/globals.definitions.ts index 8de75912b..80e7bcaa9 100644 --- a/src/frontend/testing/globals.definitions.ts +++ b/src/frontend/testing/globals.definitions.ts @@ -56,6 +56,9 @@ export class MockXhr implements XmlHttpRequestLike { readyState: number = 4; status: number = 200; responseText: string = JSON.stringify({error: 0}); + upload = { + onprogress: jest.fn() + } } export interface ElementLike { From a7a1ede4a91096110d1d337100cbe36b6a015022 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 14:36:58 +0100 Subject: [PATCH 16/27] Updated for use of \Q and \E in regex --- lib/GADS/Filecheck.pm | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index cee50e2a8..6f4572d22 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -56,8 +56,11 @@ sub create_regex return (undef, undef) unless $allowed_data; # Putting in \Q and \E to escape any special characters in the mimetype or extension breaks this - I don't know why - my $mimeRegex = '^(' . join('|', map { $_->{name} } @$allowed_data) . ')'; - my $filetypeRegex = '^(' . join('|', map { $_->{extension} } @$allowed_data) . ')$'; + my @mimeRegexes = map { qr!\Q$_->{name}\E! } @$allowed_data; + my @filetypeRegexes = map { qr!\Q$_->{extension}\E! } @$allowed_data; + + my $mimeRegex = '^(' . join ('|', @mimeRegexes) . ')$'; + my $filetypeRegex = '^(' . join ('|', @filetypeRegexes) . ')$'; return (qr/$mimeRegex/, qr/$filetypeRegex/); } From 98262180f29dfd91fdd1a2a9f830a4bf1ee68c0f Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 14:47:48 +0100 Subject: [PATCH 17/27] Updated filecheck to use actual files --- lib/GADS/Column/File.pm | 6 +++--- lib/GADS/Filecheck.pm | 32 +++++--------------------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index a795ec54b..725db34f3 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -98,10 +98,10 @@ sub validate # Check that the file type is allowed here as well my $filecheck = GADS::Filecheck->instance; my $fileval = $self->schema->resultset('Fileval')->find($value); - my $content = encode_base64($fileval->content); my $name = $fileval->name; - $filecheck->check_upload($name, $content, extra_types => $self->override_types, check_name => 0); - + my $path = $fileval->file_to_id; + # Need to quote the path to ensure it is a string, rather than a file object + $filecheck->check_upload($name, "$path", extra_types => $self->override_types, check_name => 0); 1; } diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index 6f4572d22..6c0a020e0 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -26,12 +26,12 @@ sub is_image } sub check_upload -{ my ($self, $name, $content, %options) = @_; +{ my ($self, $name, $file, %options) = @_; my $check_name = $options{check_name} // 1; my $allowed_data = $options{extra_types}; - my $info = $magic->info_from_string($content); + my $info = $magic->info_from_filename($file); my ($mimeRegex, $filetypeRegex) = $self->create_regex($allowed_data); @@ -77,32 +77,10 @@ sub check_filename } sub check_file -{ my ($self, $upload, %options) = @_; +{ + my ($self, $upload, %options) = @_; - my $check_name = $options{check_name} // 1; - my $allowed_data = $options{extra_types}; - - my ($mimeRegex, $filetypeRegex) = $self->create_regex($allowed_data); - - my $info = $magic->info_from_filename($upload->tempname); - - my $allowed_bypass = $info->{mime_type} =~ $mimeRegex if $mimeRegex; - - error __x"Files of mimetype {mimetype} are not allowed", - mimetype => $info->{mime_type} - unless $allowed_bypass or $self->check_type($info->{mime_type}); - - $self->check_filename($upload->filename, $filetypeRegex); - - # As recommended at https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload - # Brackets have been added to this - above recommendations do not explicitly state that brackets are not allowed - Ticket #1695 - $self->check_name($upload->filename) - if($check_name); - - error __"Maximum file size is 50 MB" - if $upload->size > 50 * 1024 * 1024; - - return $info->{mime_type}; + return $self->check_upload($upload->filename, $upload->tempname, %options); } sub check_name { From 82b0eb90a1b6c3b535d79dc8a5e1a4a4897fe74e Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 15:06:08 +0100 Subject: [PATCH 18/27] Removed and modified comments --- lib/GADS/Column/File.pm | 2 -- lib/GADS/Filecheck.pm | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 725db34f3..03ab12bf6 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -24,8 +24,6 @@ use Moo; use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; use GADS::Filecheck; -use MIME::Base64 qw/encode_base64/; - extends 'GADS::Column'; has '+option_names' => ( diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index 6c0a020e0..8fcc06b64 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -52,10 +52,9 @@ sub check_upload sub create_regex { my ($self, $allowed_data) = @_; - # If we try to make this an array, it will bork if it's undef! + # We need to check the allowed data is defined rather than checking for an array return (undef, undef) unless $allowed_data; - # Putting in \Q and \E to escape any special characters in the mimetype or extension breaks this - I don't know why my @mimeRegexes = map { qr!\Q$_->{name}\E! } @$allowed_data; my @filetypeRegexes = map { qr!\Q$_->{extension}\E! } @$allowed_data; From 50d03b800372a33647da93a6d46e12bd8e2c8208 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 15:11:27 +0100 Subject: [PATCH 19/27] Re-added size check to upload check, and comments to check_name --- lib/GADS/Filecheck.pm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index 8fcc06b64..88da26e71 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -43,6 +43,8 @@ sub check_upload $self->check_filename($name, $filetypeRegex); + # As recommended at https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload + # Brackets have been added to this - above recommendations do not explicitly state that brackets are not allowed - Ticket #1695 $self->check_name($name) if($check_name); @@ -79,6 +81,9 @@ sub check_file { my ($self, $upload, %options) = @_; + error __"Maximum file size is 50 MB" + if $upload->size > 50 * 1024 * 1024; + return $self->check_upload($upload->filename, $upload->tempname, %options); } From 5a188dcd1c07fb60517de729b5b1f3af26bdafeb Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 15:44:01 +0100 Subject: [PATCH 20/27] Further changes to methods to better support filevals and uploads --- lib/GADS.pm | 4 ++-- lib/GADS/Column/File.pm | 4 +--- lib/GADS/Filecheck.pm | 31 ++++++++++++++++++++----------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/GADS.pm b/lib/GADS.pm index 4a61313c5..9f6ff5b0d 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -1703,7 +1703,7 @@ post '/file/:id?' => require_login sub { # File upload through the "manage files" interface if (my $upload = upload('file')) { - my $mimetype = $filecheck->check_file($upload); # Borks on invalid file type + my $mimetype = $filecheck->check_upload_old($upload); # Borks on invalid file type my $file; if (process( sub { $file = rset('Fileval')->create_with_file({ name => $upload->filename, @@ -1810,7 +1810,7 @@ post '/api/file/?' => require_login sub { config => config )->column($column_id); - my $mimetype = $filecheck->check_file($upload, check_name => 0, extra_types => $column->override_types); # Borks on invalid file type + my $mimetype = $filecheck->check_upload($upload, check_name => 0, extra_types => $column->override_types); # Borks on invalid file type my $filename = $upload->filename; # Remove any invalid characters from the new name - this will possibly be changed to an error going forward diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index 03ab12bf6..a2ea6fb1d 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -96,10 +96,8 @@ sub validate # Check that the file type is allowed here as well my $filecheck = GADS::Filecheck->instance; my $fileval = $self->schema->resultset('Fileval')->find($value); - my $name = $fileval->name; - my $path = $fileval->file_to_id; # Need to quote the path to ensure it is a string, rather than a file object - $filecheck->check_upload($name, "$path", extra_types => $self->override_types, check_name => 0); + $filecheck->check_file($fileval, extra_types => $self->override_types, check_name => 0); 1; } diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index 88da26e71..d9a3cd274 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -25,7 +25,7 @@ sub is_image $info->{mime_type} =~ m!^image/!; } -sub check_upload +sub _check_file { my ($self, $name, $file, %options) = @_; my $check_name = $options{check_name} // 1; @@ -51,6 +51,25 @@ sub check_upload return $info->{mime_type}; } +sub check_fileval { + my ($self, $fileval, %options) = @_; + + my $filename = $fileval->file_to_id; + + # Quote the path to ensure it is a string, rather than a file object + return $self->_check_file($fileval->name, "$filename", %options); +} + +sub check_upload +{ + my ($self, $upload, %options) = @_; + + error __"Maximum file size is 50 MB" + if $upload->size > 50 * 1024 * 1024; + + return $self->_check_file($upload->filename, $upload->tempname, %options); +} + sub create_regex { my ($self, $allowed_data) = @_; @@ -77,16 +96,6 @@ sub check_filename unless $extension_bypass or $self->check_extension($ext); } -sub check_file -{ - my ($self, $upload, %options) = @_; - - error __"Maximum file size is 50 MB" - if $upload->size > 50 * 1024 * 1024; - - return $self->check_upload($upload->filename, $upload->tempname, %options); -} - sub check_name { my ($self, $filename) = @_; error __x"The filename {name} is not allowed. Filenames can only contain alphanumeric characters and a single dot", From 9f93c5652fed59dbb297d5765da4b6d76adada5f Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 16:33:13 +0100 Subject: [PATCH 21/27] Fix for typo in Fileval file --- lib/GADS/Column/File.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index a2ea6fb1d..aed5486d4 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -97,7 +97,7 @@ sub validate my $filecheck = GADS::Filecheck->instance; my $fileval = $self->schema->resultset('Fileval')->find($value); # Need to quote the path to ensure it is a string, rather than a file object - $filecheck->check_file($fileval, extra_types => $self->override_types, check_name => 0); + $filecheck->check_fileval($fileval, extra_types => $self->override_types, check_name => 0); 1; } From 688519cbbe9ea75fef1ab2b38ed2db314829c64f Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 16:44:22 +0100 Subject: [PATCH 22/27] Fixed incorrect sub name in call --- lib/GADS.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/GADS.pm b/lib/GADS.pm index 9f6ff5b0d..bc113255c 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -1703,7 +1703,7 @@ post '/file/:id?' => require_login sub { # File upload through the "manage files" interface if (my $upload = upload('file')) { - my $mimetype = $filecheck->check_upload_old($upload); # Borks on invalid file type + my $mimetype = $filecheck->check_upload($upload); # Borks on invalid file type my $file; if (process( sub { $file = rset('Fileval')->create_with_file({ name => $upload->filename, From cd953595776c81f1d258481b8d58cb62a7b144e6 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 16:49:40 +0100 Subject: [PATCH 23/27] Updated errorhandler --- src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts index 580ad2838..dd9d620a2 100644 --- a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts +++ b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts @@ -1,3 +1,5 @@ +import { fromJson } from "util/common"; + export class ErrorHandler { private el: JQuery; private errors: string[] = []; @@ -18,6 +20,10 @@ export class ErrorHandler { addError(...errors: (string | Error)[]) { errors.forEach(error => { if (typeof error === 'string') { + const jsonError = fromJson(error); + if(jsonError) { + this.addError(jsonError); + } this.errors.push(error); } else if (error instanceof Error) { this.errors.push(error.message); From 22ad5eb29c49b4049df27f9e2b12b9b1748765b4 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 16:52:08 +0100 Subject: [PATCH 24/27] Further error checks --- src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts index dd9d620a2..351bfc716 100644 --- a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts +++ b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts @@ -25,7 +25,7 @@ export class ErrorHandler { this.addError(jsonError); } this.errors.push(error); - } else if (error instanceof Error) { + } else if (error instanceof Error || "message" in error) { this.errors.push(error.message); } else { console.warn('Unsupported error type:', error); From 3de0eb1715eb0de279df3b8dc141d777108a2708 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 17:00:59 +0100 Subject: [PATCH 25/27] Revert "Further error checks" This reverts commit 22ad5eb29c49b4049df27f9e2b12b9b1748765b4. --- src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts index 351bfc716..dd9d620a2 100644 --- a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts +++ b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts @@ -25,7 +25,7 @@ export class ErrorHandler { this.addError(jsonError); } this.errors.push(error); - } else if (error instanceof Error || "message" in error) { + } else if (error instanceof Error) { this.errors.push(error.message); } else { console.warn('Unsupported error type:', error); From 64fdcf9fd8ace13dfdfd904a19d66376412af82e Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Wed, 30 Jul 2025 17:01:16 +0100 Subject: [PATCH 26/27] Revert "Updated errorhandler" This reverts commit cd953595776c81f1d258481b8d58cb62a7b144e6. --- src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts index dd9d620a2..580ad2838 100644 --- a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts +++ b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts @@ -1,5 +1,3 @@ -import { fromJson } from "util/common"; - export class ErrorHandler { private el: JQuery; private errors: string[] = []; @@ -20,10 +18,6 @@ export class ErrorHandler { addError(...errors: (string | Error)[]) { errors.forEach(error => { if (typeof error === 'string') { - const jsonError = fromJson(error); - if(jsonError) { - this.addError(jsonError); - } this.errors.push(error); } else if (error instanceof Error) { this.errors.push(error.message); From d8cb21a6d4548ff5388eb369d2e8430803867943 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Thu, 31 Jul 2025 10:00:34 +0100 Subject: [PATCH 27/27] Reverted items showing as changes where not required --- bin/add-extension.pl | 2 +- lib/GADS.pm | 6 +- lib/GADS/Column.pm | 17 ++--- lib/GADS/Column/Autocur.pm | 9 +-- lib/GADS/Column/Calc.pm | 6 +- lib/GADS/Column/Curval.pm | 22 +------ lib/GADS/Column/Date.pm | 10 +-- lib/GADS/Column/Daterange.pm | 7 +- lib/GADS/Column/File.pm | 17 ++--- lib/GADS/Column/Intgr.pm | 7 +- lib/GADS/Column/Person.pm | 16 +---- lib/GADS/Filecheck.pm | 66 ++++++++++--------- .../components/button/lib/rename-button.ts | 1 - 13 files changed, 54 insertions(+), 132 deletions(-) diff --git a/bin/add-extension.pl b/bin/add-extension.pl index ff2d31791..6cd8cc361 100755 --- a/bin/add-extension.pl +++ b/bin/add-extension.pl @@ -72,7 +72,6 @@ # Make sure the user wants to add the data to the selected column. print "Do you want to add mime type $mimetype with extension $extension to ". $result->name ." (y/n): "; my $answer = ; -chomp $answer; # Do a check for y if ( $answer =~ /^y/i ) { @@ -80,6 +79,7 @@ 'name' => $mimetype, 'extension' => $extension, }; + $result->override_types([]) unless $result->override_types; my $types = $result->override_types; push(@$types, $type) unless grep { $_->{extension} eq $type->{extension} && $_->{name} eq $type->{name} } @$types; $result->override_types($types); diff --git a/lib/GADS.pm b/lib/GADS.pm index bc113255c..c69d289bc 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -1703,7 +1703,7 @@ post '/file/:id?' => require_login sub { # File upload through the "manage files" interface if (my $upload = upload('file')) { - my $mimetype = $filecheck->check_upload($upload); # Borks on invalid file type + my $mimetype = $filecheck->check_file($upload); # Borks on invalid file type my $file; if (process( sub { $file = rset('Fileval')->create_with_file({ name => $upload->filename, @@ -1810,7 +1810,7 @@ post '/api/file/?' => require_login sub { config => config )->column($column_id); - my $mimetype = $filecheck->check_upload($upload, check_name => 0, extra_types => $column->override_types); # Borks on invalid file type + my $mimetype = $filecheck->check_file($upload, check_name => 0, extra_types => $column->override_types); # Borks on invalid file type my $filename = $upload->filename; # Remove any invalid characters from the new name - this will possibly be changed to an error going forward @@ -3731,7 +3731,7 @@ prefix '/:layout_name' => sub { $column->type(param 'type') unless param('id'); # Can't change type as it would require DBIC resultsets to be removed and re-added $column->$_(param $_) - foreach @{$column->user_options}; + foreach @{$column->option_names}; $column->display_fields(param 'display_fields'); # Set the layout in the GADS::Filter object, in case the write # doesn't success, in which case the filter will need to be diff --git a/lib/GADS/Column.pm b/lib/GADS/Column.pm index 6691922f8..a0bf21a92 100644 --- a/lib/GADS/Column.pm +++ b/lib/GADS/Column.pm @@ -248,14 +248,14 @@ sub reset_options # Force each option to build now to capture its value, otherwise if it # hasn't already been built then the options hash will be lost and it will # use its default value - $self->$_ foreach map { $_->{name} } @{$self->option_names}; + $self->$_ foreach @{$self->option_names}; $self->clear_options; } sub _build_options { my $self = shift; my $options = {}; - foreach my $option_name (map {$_->{name}} @{$self->option_names}) + foreach my $option_name (@{$self->option_names}) { $options->{$option_name} = $self->$option_name; } @@ -268,15 +268,6 @@ has option_names => ( default => sub { [] }, ); -has user_options => ( - is => 'lazy', - isa => ArrayRef[Str], - builder => sub { - my $self = shift; - [map { $_->{name} } grep {$_->{user_configurable}} @{$self->option_names}]; - }, -); - has ordering => ( is => 'rw', isa => Maybe[Str], @@ -1682,7 +1673,7 @@ sub import_hash notice __x"Update: filter from {old} to {new} for {name}", old => $self->filter->as_json, new => $values->{filter}, name => $self->name if $report && $self->filter->changed; - foreach my $option (map {$_->{name}} @{$self->option_names}) + foreach my $option (@{$self->option_names}) { notice __x"Update: {option} from {old} to {new} for {name}", option => $option, old => $self->$option, new => $values->{$option}, name => $self->name @@ -1735,7 +1726,7 @@ sub export_hash }; } $return->{display_fields} = \@display_fields; - foreach my $option (map {$_->{name}} @{$self->option_names}) + foreach my $option (@{$self->option_names}) { $return->{$option} = $self->$option; } diff --git a/lib/GADS/Column/Autocur.pm b/lib/GADS/Column/Autocur.pm index ece4906c7..1af3316e1 100644 --- a/lib/GADS/Column/Autocur.pm +++ b/lib/GADS/Column/Autocur.pm @@ -29,14 +29,7 @@ extends 'GADS::Column::Curcommon'; with 'GADS::Role::Curcommon::RelatedField'; has '+option_names' => ( - default => sub { [{ - name => 'override_permissions', - user_configurable => 1, - }, { - name => 'limit_rows', - user_configurable => 1, - }] - }, + default => sub { [qw/override_permissions limit_rows/] }, ); has '+multivalue' => ( diff --git a/lib/GADS/Column/Calc.pm b/lib/GADS/Column/Calc.pm index b71682afc..0fab15a86 100644 --- a/lib/GADS/Column/Calc.pm +++ b/lib/GADS/Column/Calc.pm @@ -33,11 +33,7 @@ has '+type' => ( ); has '+option_names' => ( - default => sub {[{ - name => 'show_in_edit', - user_configurable => 1, - }] - }, + default => sub { [qw/show_in_edit/] }, ); has show_in_edit => ( diff --git a/lib/GADS/Column/Curval.pm b/lib/GADS/Column/Curval.pm index ef9e56869..5165cfb80 100644 --- a/lib/GADS/Column/Curval.pm +++ b/lib/GADS/Column/Curval.pm @@ -30,27 +30,7 @@ extends 'GADS::Column::Curcommon'; with 'GADS::Role::Curcommon::CurvalMulti'; has '+option_names' => ( - default => sub { - [{ - name => 'override_permissions', - user_configurable => 1, - }, { - name => 'value_selector', - user_configurable => 1, - }, { - name =>'show_add', - user_configurable => 1, - }, { - name => 'delete_not_used', - user_configurable => 1, - }, { - name => 'limit_rows', - user_configurable => 1, - }, { - name => 'show_view_all', - user_configurable => 1, - }] - }, + default => sub { [qw/override_permissions value_selector show_add delete_not_used limit_rows show_view_all/] }, ); has show_view_all => ( diff --git a/lib/GADS/Column/Date.pm b/lib/GADS/Column/Date.pm index 241c76a68..c9e1f69e4 100644 --- a/lib/GADS/Column/Date.pm +++ b/lib/GADS/Column/Date.pm @@ -46,15 +46,7 @@ has '+has_multivalue_plus' => ( ); has '+option_names' => ( - default => sub { - [{ - name => 'show_datepicker', - user_configurable => 1, - }, { - name => 'default_today', - user_configurable => 1, - }] - }, + default => sub { [qw/show_datepicker default_today/] }, ); has show_datepicker => ( diff --git a/lib/GADS/Column/Daterange.pm b/lib/GADS/Column/Daterange.pm index 9dde3df19..5f0dd26e5 100644 --- a/lib/GADS/Column/Daterange.pm +++ b/lib/GADS/Column/Daterange.pm @@ -45,12 +45,7 @@ has '+has_multivalue_plus' => ( ); has '+option_names' => ( - default => sub { - [{ - name =>'show_datepicker', - user_configurable => 1, - }] - }, + default => sub { [qw/show_datepicker/] }, ); sub _build_retrieve_fields diff --git a/lib/GADS/Column/File.pm b/lib/GADS/Column/File.pm index aed5486d4..e4f7df746 100644 --- a/lib/GADS/Column/File.pm +++ b/lib/GADS/Column/File.pm @@ -22,17 +22,11 @@ use Log::Report 'linkspace'; use MIME::Base64 qw/decode_base64/; use Moo; use MooX::Types::MooseLike::Base qw/Int Maybe ArrayRef/; -use GADS::Filecheck; extends 'GADS::Column'; has '+option_names' => ( - default => sub { - [+{ - name => 'override_types', - user_configurable => 0, - }] - } + default => sub { [qw/override_types/] } ); has override_types => ( @@ -89,15 +83,12 @@ sub validate if ($value !~ /^[0-9]+$/ || !$self->schema->resultset('Fileval')->find($value)) { return 0 unless $options{fatal}; + # Check that the file type is allowed here as well + my $filecheck = GADS::Filecheck->instance; + $filecheck->check_upload($self->name, $self->content, extra_types => $self->override_types); error __x"'{int}' is not a valid file ID for '{col}'", int => $value, col => $self->name; } - - # Check that the file type is allowed here as well - my $filecheck = GADS::Filecheck->instance; - my $fileval = $self->schema->resultset('Fileval')->find($value); - # Need to quote the path to ensure it is a string, rather than a file object - $filecheck->check_fileval($fileval, extra_types => $self->override_types, check_name => 0); 1; } diff --git a/lib/GADS/Column/Intgr.pm b/lib/GADS/Column/Intgr.pm index 44020d434..c8324ae4c 100644 --- a/lib/GADS/Column/Intgr.pm +++ b/lib/GADS/Column/Intgr.pm @@ -39,12 +39,7 @@ has '+return_type' => ( ); has '+option_names' => ( - default => sub { - [{ - name => 'show_calculator', - user_configurable => 1, - }] - }, + default => sub { [qw/show_calculator/] }, ); has show_calculator => ( diff --git a/lib/GADS/Column/Person.pm b/lib/GADS/Column/Person.pm index 83d7af860..214e8eedb 100644 --- a/lib/GADS/Column/Person.pm +++ b/lib/GADS/Column/Person.pm @@ -73,21 +73,7 @@ has '+fixedvals' => ( ); has '+option_names' => ( - default => sub { - [{ - name => 'default_to_login', - user_configurable => 1, - }, { - name => 'notify_on_selection', - user_configurable => 1, - }, { - name => 'notify_on_selection_subject', - user_configurable => 1, - }, { - name => 'notify_on_selection_message', - user_configurable =>1, - }] - }, + default => sub { [qw/default_to_login notify_on_selection notify_on_selection_message notify_on_selection_subject/] }, ); has '+can_multivalue' => ( diff --git a/lib/GADS/Filecheck.pm b/lib/GADS/Filecheck.pm index d9a3cd274..d462c8fa2 100644 --- a/lib/GADS/Filecheck.pm +++ b/lib/GADS/Filecheck.pm @@ -25,13 +25,13 @@ sub is_image $info->{mime_type} =~ m!^image/!; } -sub _check_file -{ my ($self, $name, $file, %options) = @_; +sub check_upload +{ my ($self, $name, $content, %options) = @_; my $check_name = $options{check_name} // 1; my $allowed_data = $options{extra_types}; - my $info = $magic->info_from_filename($file); + my $info = $magic->info_from_string($content); my ($mimeRegex, $filetypeRegex) = $self->create_regex($allowed_data); @@ -43,44 +43,19 @@ sub _check_file $self->check_filename($name, $filetypeRegex); - # As recommended at https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload - # Brackets have been added to this - above recommendations do not explicitly state that brackets are not allowed - Ticket #1695 $self->check_name($name) if($check_name); return $info->{mime_type}; } -sub check_fileval { - my ($self, $fileval, %options) = @_; - - my $filename = $fileval->file_to_id; - - # Quote the path to ensure it is a string, rather than a file object - return $self->_check_file($fileval->name, "$filename", %options); -} - -sub check_upload -{ - my ($self, $upload, %options) = @_; - - error __"Maximum file size is 50 MB" - if $upload->size > 50 * 1024 * 1024; - - return $self->_check_file($upload->filename, $upload->tempname, %options); -} - sub create_regex { my ($self, $allowed_data) = @_; - # We need to check the allowed data is defined rather than checking for an array - return (undef, undef) unless $allowed_data; - - my @mimeRegexes = map { qr!\Q$_->{name}\E! } @$allowed_data; - my @filetypeRegexes = map { qr!\Q$_->{extension}\E! } @$allowed_data; + return (undef, undef) unless @$allowed_data; - my $mimeRegex = '^(' . join ('|', @mimeRegexes) . ')$'; - my $filetypeRegex = '^(' . join ('|', @filetypeRegexes) . ')$'; + my $mimeRegex = '^(' . join('|', map { '\Q' . $_->{name} .'\E' } @$allowed_data) . ')'; + my $filetypeRegex = '^(' . join('|', map { '\Q'.$_->{extension}.'\E' } @$allowed_data) . ')$'; return (qr/$mimeRegex/, qr/$filetypeRegex/); } @@ -96,6 +71,35 @@ sub check_filename unless $extension_bypass or $self->check_extension($ext); } +sub check_file +{ my ($self, $upload, %options) = @_; + + my $check_name = $options{check_name} // 1; + my $allowed_data = $options{extra_types}; + + my ($mimeRegex, $filetypeRegex) = $self->create_regex($allowed_data); + + my $info = $magic->info_from_filename($upload->tempname); + + my $allowed_bypass = $info->{mime_type} =~ $mimeRegex if $mimeRegex; + + error __x"Files of mimetype {mimetype} are not allowed", + mimetype => $info->{mime_type} + unless $allowed_bypass or $self->check_type($info->{mime_type}); + + $self->check_filename($upload->filename, $filetypeRegex); + + # As recommended at https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload + # Brackets have been added to this - above recommendations do not explicitly state that brackets are not allowed - Ticket #1695 + $self->check_name($upload->filename) + if($check_name); + + error __"Maximum file size is 50 MB" + if $upload->size > 50 * 1024 * 1024; + + return $info->{mime_type}; +} + sub check_name { my ($self, $filename) = @_; error __x"The filename {name} is not allowed. Filenames can only contain alphanumeric characters and a single dot", diff --git a/src/frontend/components/button/lib/rename-button.ts b/src/frontend/components/button/lib/rename-button.ts index 46b7138d3..f1a004082 100644 --- a/src/frontend/components/button/lib/rename-button.ts +++ b/src/frontend/components/button/lib/rename-button.ts @@ -111,7 +111,6 @@ class RenameButton { .split('.') .slice(0, -1) .join('.'); - console.log("Original file name:", original); $(`#current-${id}`) .addClass('hidden') .attr('aria-hidden', 'true');