Обсуждение: How to ensure that a stored function always returns TRUE or FALSE?

On 3/2/16, Alexander Farber <alexander.farber@gmail.com> wrote:
> Good morning,
>
> with the following stored function I would like to validate user data:
>
>         CREATE OR REPLACE FUNCTION check_user(
>                 in_social integer,
>                 in_sid varchar(255),
>                 in_auth varchar(32))
>                 RETURNS boolean AS
>         $func$
>                 SELECT MD5('secret word' || in_social || in_sid) = in_auth;
>         $func$ LANGUAGE sql IMMUTABLE;
>
> I am going to call it while looping through a JSON array of objects in
> another stored functions - and will RAISE EXCEPTION if it returns FALSE for
> any of the JSON objects (and thus rollback the whole transaction).
>
> I have prepared 3 simple test functions below -
>
> <overquoting>
>
>         CREATE OR REPLACE FUNCTION test3() RETURNS void AS
>         $func$
>         BEGIN
>                 IF NOT check_user(42, 'user1', NULL) THEN
>                         RAISE NOTICE 'invalid user';
>                 ELSE
>                         RAISE NOTICE 'valid user';
>                 END IF;
>         END
>         $func$ LANGUAGE plpgsql;
>
> The 3rd function does NOT work as expected and prints "valid user".
>
> This happens because check_user() returns NULL instead of a boolean value.

I guess it is enough to swap blocks inside of IF statement and reverse
its condition:

        CREATE OR REPLACE FUNCTION test3() RETURNS void AS
        $func$
        BEGIN
                IF check_user(42, 'user1', NULL) THEN
                        RAISE NOTICE 'valid user';
                ELSE
                        RAISE NOTICE 'invalid user';
                END IF;
        END
        $func$ LANGUAGE plpgsql;

would give "invalid user". NULL works as FALSE at the top of IF expressions.
For more information see[1].

> COALESCE could be wrapped around the check_user() call in the
> IF-statement... but is there maybe a nicer way to solve this problem?
>
> Thank you
> Alex
>


[1]https://en.wikipedia.org/wiki/Null_(SQL)#Comparisons_with_NULL_and_the_three-valued_logic_.283VL.29
--
Best regards,
Vitaly Burovoy

On Wed, Mar 2, 2016 at 3:45 AM, Alexander Farber
<alexander.farber@gmail.com> wrote:
> Good morning,
>
> with the following stored function I would like to validate user data:
>
>         CREATE OR REPLACE FUNCTION check_user(
>                 in_social integer,
>                 in_sid varchar(255),
>                 in_auth varchar(32))
>                 RETURNS boolean AS
>         $func$
>                 SELECT MD5('secret word' || in_social || in_sid) = in_auth;
>         $func$ LANGUAGE sql IMMUTABLE;
>
> I am going to call it while looping through a JSON array of objects in
> another stored functions - and will RAISE EXCEPTION if it returns FALSE for
> any of the JSON objects (and thus rollback the whole transaction).

Personally I would write the check like this:
SELECT MD5('secret word' || in_social || in_sid) IS NOT DISTINCT FROM in_auth;

...for better handling of NULLS within the input arguments.  It is
definitely write for this function to be sql, not plpgsql, because it
is a good candidate for inlining.

Also, I tend to wrap RAISE NOTICE with a function:

CREATE OR REPLACE FUNCTION Notice(TEXT) RETURNS VOID AS
$$
BEGIN
  RAISE NOTICE '%', $1;
END;
$$ LANGUAGE PLPGSQL;

CREATE OR REPLACE FUNCTION Exception(TEXT) RETURNS VOID AS
$$
BEGIN
  RAISE NOTICE '%', $1;
END;
$$ LANGUAGE PLPGSQL;

CREATE OR REPLACE FUNCTION NoticeValue(anyelement) RETURNS anyelement AS
$$
  SELECT $1 FROM (SELECT NOTICE($1::TEXT)) q;
$$ LANGUAGE SQL IMMUTABLE;

Then you can write a checker function like this:

        CREATE OR REPLACE FUNCTION test4() RETURNS void AS
        $func$
        BEGIN
                PERFORM Exception('invalid user') WHERE NOT
check_user(42, 'user1', NULL);
        END
        $func$ LANGUAGE plpgsql;

"NoticeValue()" Is a wonderful debugging tool for pl/pgsql.  It allows
you to quickly virtually anything in a query without rewriting the
entire query.

SELECT NoticeValue(foo) FROM bar;

merlin

On 3/2/16, Alexander Farber <alexander.farber@gmail.com> wrote:
> On Wed, Mar 2, 2016 at 11:09 AM, Vitaly Burovoy <vitaly.burovoy@gmail.com>
> wrote:
>
>> On 3/2/16, Alexander Farber <alexander.farber@gmail.com> wrote:
>> >
>> >         CREATE OR REPLACE FUNCTION check_user(
>> >                 in_social integer,
>> >                 in_sid varchar(255),
>> >                 in_auth varchar(32))
>> >                 RETURNS boolean AS
>> >         $func$
>> >                 SELECT MD5('secret word' || in_social || in_sid) =
>> in_auth;
>> >         $func$ LANGUAGE sql IMMUTABLE;
>> >
>> >
>> >         CREATE OR REPLACE FUNCTION test3() RETURNS void AS
>> >         $func$
>> >         BEGIN
>> >                 IF NOT check_user(42, 'user1', NULL) THEN
>> >                         RAISE NOTICE 'invalid user';
>> >                 ELSE
>> >                         RAISE NOTICE 'valid user';
>> >                 END IF;
>> >         END
>> >         $func$ LANGUAGE plpgsql;
>> >
>> > The 3rd function does NOT work as expected and prints "valid user".
>> >
>> > This happens because check_user() returns NULL instead of a boolean
>> value.
>>
>> I guess it is enough to swap blocks inside of IF statement and reverse
>> its condition:
>>
>>         CREATE OR REPLACE FUNCTION test3() RETURNS void AS
>>         $func$
>>         BEGIN
>>                 IF check_user(42, 'user1', NULL) THEN
>>                         RAISE NOTICE 'valid user';
>>                 ELSE
>>                         RAISE NOTICE 'invalid user';
>>                 END IF;
>>         END
>>         $func$ LANGUAGE plpgsql;
>>
>> would give "invalid user". NULL works as FALSE at the top of IF
>> expressions.
>>
>>
>> [1]https://en.wikipedia.org/wiki/Null_(SQL)#Comparisons_with_NULL_and_the_three-valued_logic_.283VL.29
>
> Thanks Vitaly, but instead of inverting the IF-condition I would prefer to
> make my function more robust, since it is kind of security-related and I
> might forget about the special IF-condition later when using it
> elsewhere...

As Merlin Moncure mentioned[2] the best way is to replace "=" by "IS
NOT DISTINCT FROM" in the "check_user" function.
But if you want to change only IF statement in "testX" functions it is
enough to replace the condition "IF NOT check_user(42, 'user1', NULL)
THEN" by "IF check_user(42, 'user1', NULL) IS NOT TRUE THEN". See the
example below:

postgres=# SELECT var, var IS NOT TRUE AS result
postgres-# FROM unnest(ARRAY[TRUE, FALSE, NULL]::bool[])as var;
 var | result
-----+--------
 t   | f
 f   | t
     | t
(3 rows)

P.S.: please, don't top post.

[2]http://www.postgresql.org/message-id/CAHyXU0xdFQ--0aTm3Md7d1x5ZnfBJDe0eMJHnVtCZdBJuFDqew@mail.gmail.com
--
Best regards,
Vitaly Burovoy