From 5cb09ab21e8b3ddc6ec038c0aaafc48e3cafd693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:15:06 -0300 Subject: [PATCH 1/4] fix(tabutils): refuse a tabrestore row with a field after the last column tabrestore stripped the comma after each field, so a comma after the field of the last column left nothing to report. A row with one more empty field was restored as if it fit the columns. A last extra field of 0 also passed, because the check tested the rest of the line for truth. tabrestore now refuses a row whose last column field ends with a comma, or that has any text left, and rolls back the restore. --- xCAT-server/lib/xcat/plugins/tabutils.pm | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/tabutils.pm b/xCAT-server/lib/xcat/plugins/tabutils.pm index b77c6c464..cc0aab444 100644 --- a/xCAT-server/lib/xcat/plugins/tabutils.pm +++ b/xCAT-server/lib/xcat/plugins/tabutils.pm @@ -430,6 +430,7 @@ sub tabrestore my $origline = $line; #save for error reporting my %record; my $col; + my $separated; #a comma after the field of the last column starts one more field foreach $col (@colns) { if ($line =~ /^,/ or $line eq "") @@ -438,7 +439,7 @@ sub tabrestore if (!exists($auto_cols{$col})) { $record{$col} = undef; } - $line =~ s/^,//; + $separated = ($line =~ s/^,//); } elsif ($line =~ /^[^,]*"/) { # We have stuff in quotes... pain... @@ -501,7 +502,7 @@ sub tabrestore elsif ($offset eq length($line) or $nextchar eq ',') { #hit the end of the line or at least the end of the column $ent = substr($line, 0, $offset, ''); - $line =~ s/^,//; + $separated = ($line =~ s/^,//); chop $ent; $ent = substr($ent, 1); $ent =~ s/""/"/g; @@ -531,10 +532,10 @@ sub tabrestore { $record{$col} = $1; } - $line =~ s/^([^,]+)(,|$)//; + $separated = ($line =~ s/^([^,]+)(,|$)//) && $2 eq ','; } } - if ($line) + if ($line ne '' or $separated) { $rollback = 1; $cb->({ error => "Too many fields on line $linenumber: $origline | $line", errorcode => 4 }); From 1d7374c0e40c751fd18809f7dc0be21b2f75bd98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:16:49 -0300 Subject: [PATCH 2/4] test(tabutils): cover the field count of a tabrestore row tabrestore must restore a row with one field for each column, fewer fields, or a quoted field that holds a comma. It must refuse a row with an empty, 0 or text field after the last column, and roll back the restore. --- xCAT-test/unit/tabrestore_field_count.t | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 xCAT-test/unit/tabrestore_field_count.t diff --git a/xCAT-test/unit/tabrestore_field_count.t b/xCAT-test/unit/tabrestore_field_count.t new file mode 100644 index 000000000..ac9d13080 --- /dev/null +++ b/xCAT-test/unit/tabrestore_field_count.t @@ -0,0 +1,55 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use FindBin; +use lib "$FindBin::Bin/../lib", "$FindBin::Bin/../../perl-xCAT", "$FindBin::Bin/../../xCAT-server/lib/perl"; +use File::Path qw(make_path); +use File::Temp qw(tempdir); +use Test::More; +use XCAT::Test::File qw(repo_path); + +my $fixture = tempdir(CLEANUP => 1); +make_path("$fixture/db"); +local $ENV{XCATCFG} = "SQLite:$fixture/db"; +require xCAT::Table; +require(repo_path('xCAT-server/lib/xcat/plugins/tabutils.pm')); + +# Restores the rows into the site table, which has 4 columns, over a row that was there before. +sub restore { + my @rows = @_; + my $site = xCAT::Table->new('site', -create => 1); + $site->delEntries(); + $site->setAttribs({ key => 'before' }, { value => 'kept' }); + my @errors; + xCAT_plugin::tabutils::process_request({ command => ['tabrestore'], table => ['site'], data => [ '#key,value,comments,disable', @rows ] }, + sub { push @errors, map { ref($_) ? @$_ : $_ } grep { defined } $_[0]{error} }); + my %rows = map { ($_->{key} => join('|', map { $_ // '' } @{$_}{qw(value comments)})) } xCAT::Table->new('site')->getAllAttribs('key', 'value', 'comments'); + return (join("\n", @errors), \%rows); +} + +for my $case ( + [ 'one field for each column', '"k1","v1",,' ], + [ 'fewer fields than columns', '"k1","v1"' ], + [ 'a quoted field that holds a comma', '"k1","v1","a,b",', 'v1|a,b' ], + [ 'a quoted comment before the last field', '"k1","v1","note",', 'v1|note' ]) +{ + my ($label, $row, $want) = @$case; + my ($errors, $rows) = restore($row); + is($errors, '', "$label: the row is restored without an error"); + is_deeply($rows, { k1 => $want // 'v1|' }, "$label: the restore replaces the table with the row"); +} + +for my $case ( + [ 'an empty field after the last column', '"k1","v1",,,' ], + [ 'a field of 0 after the last column', '"k1","v1",,,0' ], + [ 'an empty field after a quoted last column', '"k1","v1",,"yes",' ], + [ 'an empty field after an unquoted last column', '"k1","v1",,yes,' ], + [ 'a text field after the last column', '"k1","v1",,,extra' ]) +{ + my ($label, $row) = @$case; + my ($errors, $rows) = restore('"k0","v0",,', $row); + like($errors, qr/Too many fields on line \d+: \Q$row\E/, "$label: the row is refused"); + is_deeply($rows, { before => 'kept|' }, "$label: the restore is rolled back"); +} + +done_testing(); From 06a3f486c7c54a32c1c24b6fe85883d1a5cbc0dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:37:25 -0300 Subject: [PATCH 3/4] fix(chtab): report a table that chtab -d cannot open chtab -d called delEntries on each table that it named without a check that the table opened. A table name that xCAT does not know made chtab die after it had deleted the rows of the tables named before it. The error was "Can't call method delEntries on an undefined value". The delete moves to xCAT::TableUtils->delete_rows, which opens every named table first. When a table does not open, it deletes no row and returns the table name. chtab -d reports that table with the error of the update form. --- perl-xCAT/xCAT/TableUtils.pm | 33 +++++++++++++++++++++++++++++++++ xCAT-server/sbin/chtab | 9 +++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/perl-xCAT/xCAT/TableUtils.pm b/perl-xCAT/xCAT/TableUtils.pm index dc18128da..04d45e691 100644 --- a/perl-xCAT/xCAT/TableUtils.pm +++ b/perl-xCAT/xCAT/TableUtils.pm @@ -2171,4 +2171,37 @@ sub rmnodegroups { $tabhd->setNodeAttribs($node, { groups => join(",", @newgroups) }); } +#----------------------------------------------------------------------------- + + +=head3 delete_rows + delete the rows that match the keys from each named table + + Arguments: + keys: a hash reference of key column values + names: the table names + Returns: + undef, or the name of the first table that does not open + Example: + my $missing = xCAT::TableUtils->delete_rows({ node => 'n1' }, 'nodehm', 'noderes'); + +=cut + +#----------------------------------------------------------------------------- + +sub delete_rows { + my ($class, $keys, @names) = @_; + + # Every table opens before the first delete, so a table that does not open leaves all of them unchanged. + my %tables; + foreach my $name (@names) { + $tables{$name} = xCAT::Table->new($name, -create => 1, -autocommit => 0) or return $name; + } + foreach my $name (@names) { + $tables{$name}->delEntries($keys); + $tables{$name}->commit; + } + return; +} + 1; diff --git a/xCAT-server/sbin/chtab b/xCAT-server/sbin/chtab index 18287fd35..56835c5a8 100755 --- a/xCAT-server/sbin/chtab +++ b/xCAT-server/sbin/chtab @@ -6,6 +6,7 @@ BEGIN } use lib "$::XCATROOT/lib/perl"; use xCAT::Table; +use xCAT::TableUtils; use xCAT::NodeRange; use Getopt::Long; @@ -97,10 +98,10 @@ if ($::DELETE) { usage; exit(1); } - for (@tables_to_del) { - $tables{$_} = xCAT::Table->new($_, -create => 1, -autocommit => 0); - $tables{$_}->delEntries(\%keyhash); - $tables{$_}->commit; + my $missing = xCAT::TableUtils->delete_rows(\%keyhash, @tables_to_del); + if (defined($missing)) { + print "Table $missing does not exist.\n"; + exit(1); } } else { From 67ada58599704ab68155cde48d04e6a5d75380f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:37:25 -0300 Subject: [PATCH 4/4] test(tableutils): cover the delete of rows from several tables delete_rows must delete the rows of the keys from every table that opens. When a named table does not open, first, last or between two tables, it must return that table and delete no row. --- xCAT-test/unit/tableutils_delete_rows.t | 47 +++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 xCAT-test/unit/tableutils_delete_rows.t diff --git a/xCAT-test/unit/tableutils_delete_rows.t b/xCAT-test/unit/tableutils_delete_rows.t new file mode 100644 index 000000000..2260b3fc3 --- /dev/null +++ b/xCAT-test/unit/tableutils_delete_rows.t @@ -0,0 +1,47 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use FindBin; +use lib "$FindBin::Bin/../../perl-xCAT"; +use File::Path qw(make_path); +use File::Temp qw(tempdir); +use Test::More; + +my $fixture = tempdir(CLEANUP => 1); +make_path("$fixture/db"); +local $ENV{XCATCFG} = "SQLite:$fixture/db"; +require xCAT::Table; +require xCAT::TableUtils; + +for my $table (qw(nodehm noderes)) { + xCAT::Table->new($table, -create => 1)->setAttribs({ node => $_ }, { comments => "row of $_" }) for qw(n1 n2 n3); +} + +sub nodes { + my ($table) = @_; + return join(',', sort map { $_->{node} } xCAT::Table->new($table)->getAllAttribs('node')); +} + +sub delete_rows { + my @args = @_; + my $missing = eval { xCAT::TableUtils->delete_rows(@args) }; + return ($missing, $@); +} + +my ($missing, $error) = delete_rows({ node => 'n1' }, 'nodehm', 'noderes'); +is($error, '', 'rows are deleted from tables that open'); +is($missing, undef, 'no table is reported when every table opens'); +is(nodes('nodehm'), 'n2,n3', 'the row of the key is deleted from the first table'); +is(nodes('noderes'), 'n2,n3', 'the row of the key is deleted from the last table'); + +for my $case ([ 'between two tables', 'nodehm', 'notatable', 'noderes' ], [ 'first', 'notatable', 'nodehm', 'noderes' ], + [ 'last', 'nodehm', 'noderes', 'notatable' ]) { + my ($label, @tables) = @$case; + ($missing, $error) = delete_rows({ node => 'n2' }, @tables); + is($error, '', "a missing table named $label does not die"); + is($missing, 'notatable', "a missing table named $label is returned"); + is(nodes('nodehm'), 'n2,n3', "nodehm keeps its rows when a missing table is named $label"); + is(nodes('noderes'), 'n2,n3', "noderes keeps its rows when a missing table is named $label"); +} + +done_testing();