Re: UPDATE syntax problem - Mailing list pgsql-general

Hello,

I would just like to follow up on what you suggested since it went a
little over my head.

 > A couple points:
 >
 > 1) You're wide open to an SQL injection attack.

What's that?

 >    You'll need to
 >    preprocess the parameter values to make sure single quotes are
 >    properly escaped before building the SQL statement.

Do you mean:

string category = \'param["new_prodname"]\'

Does this prevent an sql injection attack?

 > 2) The code structure you're using is awkward--you have to add a new
 >    clause if you add a parameter.  I'd be more likely to make a list
 >    of parameters, and loop through it checking for changed values and
 >    adding clauses to the SQL statement (this would be a good time to
 >    do the quote escaping).  If no values have changed, just don't
 >    execute the SQL at all (your code doesn't handle this case).

I'm not sure how this is done. I would appreciate it if you could
elaborate on this by perhaps providing a quick example.

The following is an excerpt from my script:

if (param["new_catid"] == param["old_catid"] && \
        param["new_prodname"] == param["old_prodname"] && \
        param["new_unitcnt"] == param["old_unitcnt"] && \
        param["new_wprice"] == param["old_wprice"])
    {
        HTMLstream reply("goodbye.html");
        reply.set_field("msg1", "No modification");
        reply.set_field("msg2", "NO modification");
        reply.set_field("msg3", "You didn't modify the select product");
        reply.send();
        return 0;
    }

    string new_catid = param["new_catid"];

    if (param["new_catid"] == "")
    {
        new_catid = param["old_catid"];
    }

    //sql UPDATE statement
    string sql;

    sql = "UPDATE product SET ";
    sql += "prodname = '" + param["new_prodname"] + "',";
    sql += "wprice = " + param["new_wprice"] + ",";
    sql += "unitcnt = '" + param["new_unitcnt"] + "',";
    sql += "catid = " + new_catid;
    sql += " WHERE prodid = '" + param["prodid"] + "'";

    int res = conn.Exec (sql.c_str()); //sql exec

This works, but I'm always interested in finding better ways to do
things. Your way looks better. I realize this is more a programming
question than a postgres question. By the way, should I be using
transactions if I do it this way, or the way you have suggested?

Thanks,

Mark Tessier