Re: Bytea PL/Perl transform

út 30. 1. 2024 v 17:18 odesílatel Dagfinn Ilmari Mannsåker <
ilmari(at)ilmari(dot)org> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>
> > út 30. 1. 2024 v 16:43 odesílatel Dagfinn Ilmari Mannsåker <
> > ilmari(at)ilmari(dot)org> napsal:
> >
> >> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> >>
> >> > I inserted perl reference support - hstore_plperl and json_plperl does
> >> it.
> >> >
> >> > +<->/* Dereference references recursively. */
> >> > +<->while (SvROK(in))
> >> > +<-><-->in = SvRV(in);
> >>
> >> That code in hstore_plperl and json_plperl is only relevant because they
> >> deal with non-scalar values (hashes for hstore, and also arrays for
> >> json) which must be passed as references. The recursive nature of the
> >> dereferencing is questionable, and masked the bug fixed by commit
> >> 1731e3741cbbf8e0b4481665d7d523bc55117f63.
> >>
> >> bytea_plperl only deals with scalars (specifically strings), so should
> >> not concern itself with references. In fact, this code breaks returning
> >> objects with overloaded stringification, for example:
> >>
> >> CREATE FUNCTION plperlu_overload() RETURNS bytea LANGUAGE plperlu
> >> TRANSFORM FOR TYPE bytea
> >> AS $$
> >> package StringOverload { use overload '""' => sub { "stuff" }; }
> >> return bless {}, "StringOverload";
> >> $$;
> >>
> >> This makes the server crash with an assertion failure from Perl because
> >> SvPVbyte() was passed a non-scalar value:
> >>
> >> postgres: ilmari regression_bytea_plperl [local] SELECT: sv.c:2865:
> >> Perl_sv_2pv_flags:
> >> Assertion `SvTYPE(sv) != SVt_PVAV && SvTYPE(sv) != SVt_PVHV &&
> SvTYPE(sv)
> >> != SVt_PVFM' failed.
> >>
> >> If I remove the dereferincing loop it succeeds:
> >>
> >> SELECT encode(plperlu_overload(), 'escape') AS string;
> >> string
> >> --------
> >> stuff
> >> (1 row)
> >>
> >> Attached is a v2 patch which removes the dereferencing and includes the
> >> above example as a test.
> >>
> >
> > But without dereference it returns bad value.
>
> Where exactly does it return a bad value? The existing tests pass, and
> the one I included shows that it does the right thing in that case too.
> If you pass it an unblessed reference it returns the stringified version
> of that, as expected.
>

ugly test code

(2024-01-30 13:44:28) postgres=# CREATE or replace FUNCTION
perl_inverse_bytes(bytea) RETURNS bytea
TRANSFORM FOR TYPE bytea
AS $$ my $bytes = pack 'H*', '0123'; my $ref = \$bytes;
return $ref;
$$ LANGUAGE plperlu;
CREATE FUNCTION
(2024-01-30 13:44:33) postgres=# select perl_inverse_bytes(''), ' '::bytea;
┌──────────────────────────────────────┬───────┐
│ perl_inverse_bytes │ bytea │
╞══════════════════════════════════════╪═══════╡
│ \x5343414c41522830783130656134333829 │ \x20 │
└──────────────────────────────────────┴───────┘
(1 row)

expected

(2024-01-30 13:46:58) postgres=# select perl_inverse_bytes(''), ' '::bytea;
┌────────────────────┬───────┐
│ perl_inverse_bytes │ bytea │
╞════════════════════╪═══════╡
│ \x0123 │ \x20 │
└────────────────────┴───────┘
(1 row)

>
> CREATE FUNCTION plperl_reference() RETURNS bytea LANGUAGE plperl
> TRANSFORM FOR TYPE bytea
> AS $$ return []; $$;
>
> SELECT encode(plperl_reference(), 'escape') string;
> string
> -----------------------
> ARRAY(0x559a3109f0a8)
> (1 row)
>
> This would also crash if the dereferencing loop was left in place.
>
> > Maybe there should be a check so references cannot be returned? Probably
> is
> > not safe pass pointers between Perl and Postgres.
>
> There's no reason to ban references, that would break every Perl
> programmer's expectations. And there are no pointers being passed,
> SvPVbyte() returns the stringified form of whatever's passed in, which
> is well-behaved for both blessed and unblessed references.
>
> If we really want to be strict, we should at least allow references to
> objects that overload stringification, as they are explicitly designed
> to be well-behaved as strings. But that would be a lot of extra code
> for very little benefit over just letting Perl stringify everything.
>
> - ilmari
>