Re: Plperl Safe version check fails for Safe 2.09

It seems that the check in src/pl/plperl/plperl.c

eval_pv((safe_version < 2.09 ? safe_bad : safe_ok), FALSE);

is not working quite as expected (CVS HEAD from today):

I have Safe.pm at version 2.09, yet any plperl function I run fails with :

ERROR: error from function: trusted perl functions disabled - please
upgrade perl Safe module to at least 2.09 at (eval 4) line 1.

Just to be sure I amended the test code to :

elog(INFO, "Safe version = %f", safe_version);
eval_pv((safe_version < 2.09 ? safe_bad : safe_ok), FALSE);

and I see :

INFO: Safe version = 2.090000

(Followed by the error)

I confess some puzzlement - as the code *looks* like it should work!
The platform is Linux 2.4.22 glibc 2.3.2, perl 5.8.0 (Patched Redhat 9)

regards

Mark

Could be a rounding issue. What happens if you try this instead:?

eval_pv((safe_version <= 2.08 ? safe_bad : safe_ok), FALSE);

Alternatively, what happens if we make safe_version a double rather than
a float?

(If nothing else works we might have to fall back on a lexical comparison)

cheers

andrew

Mark Kirkwood wrote:

> It seems that the check in src/pl/plperl/plperl.c
>
> eval_pv((safe_version < 2.09 ? safe_bad : safe_ok), FALSE);
>
> is not working quite as expected (CVS HEAD from today):
>
> I have Safe.pm at version 2.09, yet any plperl function I run fails
> with :
>
> ERROR: error from function: trusted perl functions disabled - please
> upgrade perl Safe module to at least 2.09 at (eval 4) line 1.
>
> Just to be sure I amended the test code to :
>
> elog(INFO, "Safe version = %f", safe_version);
> eval_pv((safe_version < 2.09 ? safe_bad : safe_ok), FALSE);
>
> and I see :
>
> INFO: Safe version = 2.090000
>
> (Followed by the error)
>
> I confess some puzzlement - as the code *looks* like it should work!
> The platform is Linux 2.4.22 glibc 2.3.2, perl 5.8.0 (Patched Redhat 9)
>
> regards
>
> Mark
>
>
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

A bit more thinking led me to try:

float safe_version;
...
eval_pv((safe_version < (float)2.09 ? safe_bad : safe_ok), FALSE);

which seems to fix the issue. (after all float *should* be accurate
enough in this case)

cheers

Mark

P.s : trivial patch attached

Andrew Dunstan wrote:

>
> Could be a rounding issue. What happens if you try this instead:?
>
> eval_pv((safe_version <= 2.08 ? safe_bad : safe_ok), FALSE);
>
> Alternatively, what happens if we make safe_version a double rather
> than a float?
>
> (If nothing else works we might have to fall back on a lexical
> comparison)
>
> cheers
>
> andrew
>

Mark Kirkwood <markir(at)coretech(dot)co(dot)nz> writes:
> It seems that the check in src/pl/plperl/plperl.c
> eval_pv((safe_version < 2.09 ? safe_bad : safe_ok), FALSE);
> is not working quite as expected (CVS HEAD from today):

Yah know, I looked at that on Monday and said to myself "Self, that
looks like a rounding problem waiting to happen" ... but in the absence
of a trouble report didn't want to mess with it.

Part of the problem is that Perl NV is double, not float, and so the
declaration of safe_version is wrong on its face. But even with it
properly declared, exact comparison of double values is playing with
fire. I'd be inclined to change it to something like

double safe_version;
...
if (safe_version < 2.0899999) ...

regards, tom lane

Tom Lane said:
> Mark Kirkwood <markir(at)coretech(dot)co(dot)nz> writes:
>> It seems that the check in src/pl/plperl/plperl.c
>> eval_pv((safe_version < 2.09 ? safe_bad : safe_ok), FALSE);
>> is not working quite as expected (CVS HEAD from today):
>
> Yah know, I looked at that on Monday and said to myself "Self, that
> looks like a rounding problem waiting to happen" ... but in the absence
> of a trouble report didn't want to mess with it.
>
> Part of the problem is that Perl NV is double, not float, and so the
> declaration of safe_version is wrong on its face. But even with it
> properly declared, exact comparison of double values is playing with
> fire. I'd be inclined to change it to something like
>
> double safe_version;
> ...
> if (safe_version < 2.0899999) ...
>

Since there was no released version between 2.08 and 2.09, comparing to
2.085 should do the trick (or any of 2.08[1-9]).

cheers

andrew