Re^2: BackupPC or Data::Dumper playing foul...?

I'm pretty sure it is related to the Changes that you are referring to.
Simple code to play with and check the issue:

#!/usr/bin/perl

use strict;
use warnings;

use Data::Dumper;

my $newConf =  { 'FullKeepCnt' => [ 4 ] };

print "Original structure: ", Dumper( $newConf );

foreach my $var ( sort(keys(%$newConf)) ) {
    my $d1 = Data::Dumper->new([$newConf->{$var}], \[*value]);
    print "\nd1: ", Dumper( $d1 );
    my $value1 = $d1->Dump;
    print "\nvalue 1: $value1\n";

    my $d2 = Data::Dumper->new([$newConf->{$var}], [*value]);
    $d2->Useperl(0);
    print "\nd2: ", Dumper( $d2 );
    my $value2 = $d2->Dump;
    print "\nvalue2: $value2\n";
}
[download]

The output that matters is

value2: @main::value = (
                 4
               );
[download]

whereas the initial arrayref is what BackupPC needs and expects. (The list it gets back now via DumpXS actually ruins BackupPCs config files when using the web interface for editing...)

The pure Perl version returns a list, and so I guess that that's what it should be. My current understanding is that BackupPC did it wrongly for many years, and still does, but was able to get away with it because DumpXS within Data::Dumper was buggy. Now that Data::Dumper is corrected, BackupPCs needs to patch it's code.

Afaict, \[*value] works fine, both with DumpXS and pure Perl. But I'm not fully sure that that's 'the best' patch to be done, even if it works for me.

Comment on Re^2: BackupPC or Data::Dumper playing foul...? Select or Download Code

Replies are listed 'Best First'.
Re^3: BackupPC or Data::Dumper playing foul...? by hv (Prior) on Apr 08, 2022 at 20:41 UTC
Well this all depends on how the output from Dump (or Dumper) is going to be used, which you don't show; but yes, if the intent is to eval() the output to store in a scalar, passing `[value]` is exactly the wrong thing to do. Even if `\[value]` works for you now, it is an invalid parameter for Data::Dumper, so it's likely that another future version will actually check that and complain about it. The correct fix, as I mentioned in my original reply, is to remove the second parameter to new() altogether: just make it `Data::Dumper->new([$newConf->{$var}])`.	[reply] [d/l] [select]
Re^4: BackupPC or Data::Dumper playing foul...? by Krambambuli (Curate) on Apr 08, 2022 at 21:55 UTC
Took me a while to understand and check. Indeed, omitting the second argument entirely seems to be the correct patch. I'm not fully sure yet how the output is used; BackupPC's source is not necessarily easy to read/follow and I'm not at all 'fluent' in it. But as the changes to config values (done via CGI) are merged with the values read in from the old config files, as strings into executable config.pl files, there sure is some later eval() somewhere. Thank you!	[reply]
Re^5: BackupPC or Data::Dumper playing foul...? by dsheroh (Monsignor) on Apr 11, 2022 at 07:20 UTC
I haven't dug into the BackupPC code to verify the exact method used, but it has to be using either `do` or `eval`, because the config files aren't limited to variable assignments. If you hand-edit them, they're also able to run arbitrary perl code. The overall setup, for those unfamiliar with BackupPC, is that there's a main `config.pl` file containing a bunch of perl variable assignments representing the global configuration, and then each individual host that's backed up can, optionally, have a `[hostname].pl` file with additional variable assignments to override the global settings. Both `config.pl` and the `[hostname].pl` files can be edited through the CGI interface, which then uses Data::Dumper to overwrite the files with the new values.	[reply] [d/l] [select]