ap/Bencode

Warnings thrown on 'undef' elements

Closed this issue · 5 comments

When passing a Perl structure containing 'undef' values to the 'bencode' subroutine like so:

use Bencode qw/bencode bdecode/;
use Data::Dumper;

my $x = [1,2,undef,4];
my $b = bencode($x);
warn $b;

my $c = bdecode($b);
warn Dumper($c);

warnings are generated and the result appears to be invalid Bencode.

Use of uninitialized value $data in pattern match (m//) at /home/mark/src/bif/.direnv/perl5/lib/perl5/Bencode.pm line 120.
Use of uninitialized value $data in concatenation (.) or string at /home/mark/src/bif/.direnv/perl5/lib/perl5/Bencode.pm line 121.
Use of uninitialized value $data in concatenation (.) or string at /home/mark/src/bif/.direnv/perl5/lib/perl5/Bencode.pm line 121.
li1ei2e:i4ee at x.pl line 11.
garbage at 7 at x.pl line 13.

I am aware that the original format doesn't cover undefined values, so you probably don't want to support round-tripping of 'undef'. But I would suggest either croaking or encoding a zero-length string in place of the undef (configurable by an option?) to be better than invalid output.

ap commented

[You can edit your own posts on GitHub, btw. I’ve applied your correction for you.]

ap commented

I am aware that the original format doesn't cover undefined values, so you probably don't want to support round-tripping of undef.

Well… can’t. 😊

I would suggest either croaking or encoding a zero-length string in place of the undef (configurable by an option?) to be better than invalid output.

You don’t even need to suggest that, it’s just a statement of fact. And your stated alternatives are the obvious ways of dealing with undefs.

Of those two options, since the module is already documented to croak on unhandled data types, croaking for undefs would be the lazier choice. Doing anything else will require adding interface. But adding it may be worthwhile; croaking may be too inconvenient.

What was your use case here? What code did you have to add because of this bug, what code would you have to write if undefs were treated as empty strings, and what code would have you have to write if they triggered exceptions?

My main use case is serializing simple data structures to hash them for comparison purposes. On one side the Bencode would be constructed by SQLite triggers. On the otherside by Perl. If bencode were to croak on undef then I couldn't use it at all.

My second use case is to inflate the Bencode and do a deeper comparison in the event that the hashes do not match when they should.

If any of my structures contain NULL/undef then Bencode is not actually the right format to use. So from that point of view it appears that croaking on undef is the right thing to do.

grr commented

The conversation just started and is still ongoing and you're already going to fork it? http://prepan.org/module/nYfNhvSqeC5
BTW, very poor namespace choice.

ap commented

The conversation just started and is still ongoing and you're already going to fork it? http://prepan.org/module/nYfNhvSqeC5

Heh. I don’t mind actually. It’s… amusingly pointless?

Why pick Bencode when it doesn’t support everything you need? And why not just switch to JSON upon finding out? Using the wrong tool on purpose is sometimes fun… I didn’t get the impression that @mlawren was looking for a diversion here, though.

But through all that, a real omission in Bencode.pm got discovered and pointed out to me, so personally I’m happy.