Re: dunction issue - Mailing list pgsql-general

Alain Roger wrote:
> I do not agree with you Sam.
>
> Stored procedure are safe from hacking (from external access).

In that a stored procedure encapsulates a series of data operations,
meaning that the client doesn't have to know the details or even have
privileges to run the individual operations ? Yes, that can be really
useful, but it's hardly the full story.

Proper use of things like foreign keys, unique constraints, CHECK
constraints, etc adds another level of protection. I'd use those tools
before I restored to using a stored procedure. Like stored procedures,
users with appropriately limited priveleges are unable to bypass, drop,
or modify constraints.

That's true in any database with any sort of user privileges model.

For example, if you want to enforce the rule that a certain field must
have unique values in a table, do you think it's better to do it with a
stored procedure, or by adding a UNIQUE constraint to the field?

I'd say the UNIQUE constraint is better in every way. It's faster. It's
simple and unlike the stored procedure isn't at risk of being bypassed
by coding errors in the stored procedure. It's also self documenting, in
that the schema clearly shows the unque constraint rather than hiding it
in code. It's if anything more secure than a stored procedure because
it's simpler and can be easily protected against user modification.

The same goes for NOT NULL, CHECK constraints, foreign keys, etc.


You're also doing a lot of things in your stored procedure code the long
way. For example, instead of selecting a count() aggregate into a
variable and testing it, why not just use `EXISTS', or select the
information you really wanted and then use `IF FOUND' ?

For example:

-- It seems like not having an email address on record is an error.
-- Ensure that the problem is detected at INSERT/UPDATE time.
ALTER TABLE tmp_newsletterreg ALTER COLUMN email SET NOT NULL;

-- Really basic valiation of email addresses. It's not worth doing much
-- more than this sort of thing IMO because of performance issues and
-- transcient errors (MX lookup fail etc) when doing proper email
-- validation. At least now you don't have to revalidate in every
-- procedure.
ALTER TABLE tmp_newsletterreg ADD CONSTRAINT simplistic_email_check
CHECK lower(trim(both ' ' from email)) LIKE '%_@_%';

-- Add the same check on the user table. I imagine a NOT NULL
-- constraint there would also make sense.
ALTER TABLE users ADD CONSTRAINT simplistic_email_check
CHECK lower(trim(both ' ' from email)) LIKE '%_@_%';

-- Now, using those rules, redefine the stored procedure

CREATE OR REPLACE FUNCTION cust_portal.sp_u_002
  (id_session character varying)
  RETURNS character varying AS $BODY$
DECLARE
     ret_email CHARACTER VARYING(512) :='';
 BEGIN
     set search_path = cust_portal;

     -- Find the customer's email address, or NULL (and set NOT FOUND)
     -- if no such customer exists.
     SELECT email INTO ret_email FROM tmp_newsletterreg
       WHERE tmp_usr_id = id_session;
     IF FOUND THEN
         IF NOT EXISTS (SELECT 1 FROM users
           WHERE users.email= ret_email)
         THEN
             RETURN (ret_email);
         ELSE
            RETURN ('-2');
         END IF;
     ELSE
         RETURN('-1');
     END IF;
 END;
$BODY$ LANGUAGE 'plpgsql';


Personally I think the use of text string error codes gets ugly fast.
I'd either rewrite the function to at least return an integer error code
as an OUT parameter:


CREATE OR REPLACE FUNCTION cust_portal.sp_u_002
  (IN id_session character varying,
   OUT ret_email character varying,
   OUT err_code integer)
  RETURNS record AS $BODY$
DECLARE
 BEGIN
     set search_path = cust_portal;

     -- If we don't find the session, return -1 .
     ret_email := NULL;
     err_code := -1;

     -- Find the customer's email address, or NULL (and set NOT FOUND)
     -- if no such customer exists.
     SELECT email INTO ret_email FROM tmp_newsletterreg
       WHERE tmp_usr_id = id_session;
     IF FOUND THEN
         IF EXISTS (SELECT 1 FROM users
           WHERE users.email= ret_email)
         THEN
            -- User already registered
            ret_email := NULL;
            err_code := '-2';
         END IF;
     END IF;
 RETURN;
 END;
$BODY$ LANGUAGE 'plpgsql';


[note: the above code hasn't actually been tested]

... or preferably throw informative exceptions. However, I do find it
frustrating that I can't attach a value or list of values to a
PostgreSQL exception in a way that is easy for the client app to extract
- I have to resort to text parsing (mega-ugly and unsafe) if I need to
do it.

Especially in an internationalised environment that's not nice.

Being able to obtain the exact exception name (as opposed to the full
error message), the full error string from an exception (without its
context) and also obtain the individual parameters substituted into an
exception string would be AMAZINGLY handy for use with JDBC etc.

> From my point of view transitions should be used only as internal purpose or
> via intrAnet and not thru intErnet.
>
> at list this is how under MS SQL they use to teach.

Do you mean transactions?

Transactions really aren't a security feature/issue. They're a tool
useful for preserving data integrity and consistency by bundling up
related operations into a single change that the database guarantees
will all be applied at once, or not applied at all.

They're critically important in non-trivial database use.

Because you're wrapping most of your operations in stored procedures
they're happening transactionally anyway, but it's still an important
thing to think about.

--
Craig Ringer