Skip to content

Commit 4896813

Browse files
committed
(perl #118551) an empty string from _freeze() now supplies the same to _thaw()/_attach()
The retrieve_hook() code would simply pass the string length to NEWSV(), so if the length was zero, no PV would be allocated, and the SV would not be upgraded. The following code would then set POK on an SV that wasn't SVt_PV (or better), resulting in fun later down the line. Change to always supply at least 1 as the buffer size for NEWSV(), and always set CUR and NUL terminate the buffer.
1 parent a63d7bd commit 4896813

File tree

2 files changed

+39
-4
lines changed

2 files changed

+39
-4
lines changed

dist/Storable/Storable.xs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4903,12 +4903,12 @@ static SV *retrieve_hook_common(pTHX_ stcxt_t *cxt, const char *cname, int large
49034903
else
49044904
GETMARK(len2);
49054905

4906-
frozen = NEWSV(10002, len2);
4906+
frozen = NEWSV(10002, len2 ? len2 : 1);
49074907
if (len2) {
49084908
SAFEREAD(SvPVX(frozen), len2, frozen);
4909-
SvCUR_set(frozen, len2);
4910-
*SvEND(frozen) = '\0';
49114909
}
4910+
SvCUR_set(frozen, len2);
4911+
*SvEND(frozen) = '\0';
49124912
(void) SvPOK_only(frozen); /* Validates string pointer */
49134913
if (cxt->s_tainted) /* Is input source tainted? */
49144914
SvTAINT(frozen);

dist/Storable/t/blessed.t

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use Storable qw(freeze thaw store retrieve);
4444
'long VSTRING' => \(my $lvstring = eval "v" . 0 x 300),
4545
LVALUE => \(my $substr = substr((my $str = "foo"), 0, 3)));
4646

47-
my $test = 12;
47+
my $test = 13;
4848
my $tests = $test + 23 + (2 * 6 * keys %::immortals) + (3 * keys %::weird_refs);
4949
plan(tests => $tests);
5050

@@ -322,3 +322,38 @@ is(ref $t, 'STRESS_THE_STACK');
322322
}
323323
}
324324
}
325+
326+
{
327+
# [perl #118551]
328+
{
329+
package RT118551;
330+
331+
sub new {
332+
my $class = shift;
333+
my $string = shift;
334+
die 'Bad data' unless defined $string;
335+
my $self = { string => $string };
336+
return bless $self, $class;
337+
}
338+
339+
sub STORABLE_freeze {
340+
my $self = shift;
341+
my $cloning = shift;
342+
return if $cloning;
343+
return ($self->{string});
344+
}
345+
346+
sub STORABLE_attach {
347+
my $class = shift;
348+
my $cloning = shift;
349+
my $string = shift;
350+
return $class->new($string);
351+
}
352+
}
353+
354+
my $x = [ RT118551->new('a'), RT118551->new('') ];
355+
356+
$y = freeze($x);
357+
358+
ok(eval {thaw($y)}, "empty serialized") or diag $@; # <-- dies here with "Bad data"
359+
}

0 commit comments

Comments
 (0)