Thread: Prepared Statements

Hello everyone,

I have a question regarding the efficiency of Prepared Statements. I'm
working on a project, and my task now is to decide whether it's worth
it to use PS. This problem came up when, beginning to implement jdbc
classes, we noticed that we would need a lot of PS - something like 40
per class. Each PS would be a class variable, and it sounds weird to
have 40 class variables... We could have a more elegant system using
normal statements, but would it be much less efficient?

I started doing some very simple tests: inserting 1000 elements to a
table, doing 1.000.000 simple queries, then 1.000.000 queries with a
join... But suprisingly, Prepared Statements didn't give better results
than normal statements. Before warning the world that prepared
statements are a big lie, I wanted to have your opinion. Has anyone
done a reliable test showing the difference between PS and normal
statements? Does anyone know "how" better PS are supposed to be?

Then, concerning my test, what the hell could be wrong in what I did?
The query is the following:

String theJoinQueryPrepared =
"SELECT tr.text FROM truc tr, test te " +
"WHERE tr.id = te.id AND te.id = ?";

for a Prepared Statement, and

String theJoinQuery = "SELECT tr.text FROM truc tr, test te  " +
 WHERE tr.id = te.id AND te.id = ";

for a Statement.

Then I just do:

    for(int j = 0; j < 1000; j++)
    {
     for(int i = 0; i < 1000; i++)
     {
        thePS.setInt(1, i);
        ResultSet theResultSet = thePS.executeQuery();

     }
    }

and

    for(int j = 0; j < 1000; j++)
    {
        for(int i = 0; i < 1000; i++)
        {
            ResultSet theResultSet =
                theStatement.executeQuery(
                        theJoinQueryPrepared + i);
        }
    }

I realize that this test is ridiculously simple, but shouldn't the first
loop be more efficient? On my server both are equally fast...

Ok, I hope this message wasn't too long / too stupid. Thanks in advance,

Julien

Julien Le Goff wrote:
> Hello everyone,
>
> I have a question regarding the efficiency of Prepared Statements. I'm
> working on a project, and my task now is to decide whether it's worth
> it to use PS. This problem came up when, beginning to implement jdbc
> classes, we noticed that we would need a lot of PS - something like 40
> per class. Each PS would be a class variable, and it sounds weird to
> have 40 class variables... We could have a more elegant system using
> normal statements, but would it be much less efficient?

My understanding is that the efficiency of a PreparedStatement really
depends on the database.  I have no information about this subject in
reference to PostgreSQL (or any other database), however.

Usually you will want to use a PreparedStatement so that in the case
where a database -does- make use of it and caches the query, it will be
done.  However, you've mentioned that you have an equally performant and
more elegant solution using regular queries.  I'd go with elegance until
you really need the performance.



Erik

Prepared statements are more than just for performance.  You also use it for
when you have parametrized data so you don't have to construct query strings for
every value you may want to use the query with.

Some performance gain should come from using server prepared statements from the
time you save from parsing and optimizing the query.  Of course, for a query
that takes a long time to execute this is not very significant.

In any case, try using server prepared statements by setting your statement to
use that option (you must be using a backend at least of version 7.3):

setUseServerPrepare(true);

Let us know if you had any improvement in the time measurements.

Regards,
Fernando



Julien Le Goff wrote:> Hello everyone,
>
> I have a question regarding the efficiency of Prepared Statements. I'm
> working on a project, and my task now is to decide whether it's worth
> it to use PS. This problem came up when, beginning to implement jdbc
> classes, we noticed that we would need a lot of PS - something like 40
> per class. Each PS would be a class variable, and it sounds weird to
> have 40 class variables... We could have a more elegant system using
> normal statements, but would it be much less efficient?
>
> I started doing some very simple tests: inserting 1000 elements to a
> table, doing 1.000.000 simple queries, then 1.000.000 queries with a
> join... But suprisingly, Prepared Statements didn't give better results
> than normal statements. Before warning the world that prepared
> statements are a big lie, I wanted to have your opinion. Has anyone
> done a reliable test showing the difference between PS and normal
> statements? Does anyone know "how" better PS are supposed to be?
>
> Then, concerning my test, what the hell could be wrong in what I did?
> The query is the following:
>
> String theJoinQueryPrepared =
> "SELECT tr.text FROM truc tr, test te " +
> "WHERE tr.id = te.id AND te.id = ?";
>
> for a Prepared Statement, and
>
> String theJoinQuery = "SELECT tr.text FROM truc tr, test te  " +
>  WHERE tr.id = te.id AND te.id = ";
>
> for a Statement.
>
> Then I just do:
>
>     for(int j = 0; j < 1000; j++)
>     {
>      for(int i = 0; i < 1000; i++)
>      {
>         thePS.setInt(1, i);
>         ResultSet theResultSet = thePS.executeQuery();
>
>      }
>     }
>
> and
>
>     for(int j = 0; j < 1000; j++)
>     {
>         for(int i = 0; i < 1000; i++)
>         {
>             ResultSet theResultSet =
>                 theStatement.executeQuery(
>                         theJoinQueryPrepared + i);
>         }
>     }
>
> I realize that this test is ridiculously simple, but shouldn't the first
> loop be more efficient? On my server both are equally fast...
>
> Ok, I hope this message wasn't too long / too stupid. Thanks in advance,
>
> Julien
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>



--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

First of all what postgres version are you using?

It is important, because in 7.2 (and before) PreparedStatements *are*
indeed a "big lie". They are actually *slower* then the simple
Statement, because they parse your sql for questionmarks when you create
it, and replace the question marks with the actual parameters when you
run it - thus the overhead compared to a plain Statement, that just
sends your sql to the backend as is. There is no performance benefit in
using them before 7.3 - every statement is re-parsed, and re-planned by
the server every time you execute it anyway.

In 7.3, I believe, there is a way to cache the query plans in the
backend, and PreparedStatements can use that feature, but, that is *off*
by default AFAIK - you have to turn it on explictly (and I am not sure
how) otherwise the behaviour is the same as in 7.2.

Theoretically, the benefit of PS caching the query plans can be really
*huge*.
I am running an app, that is reading about 80 million entries from a
text file, and for each entry it looks up the corresponding row in the
database to check if it needs to be updated...
It was slow like hell, and, after loading the backend into the debugger,
I found out that about 40% (!) of the total time was spent on parsing
and planning the query I was using to lookup the row in the table (it
was literally taking longer to parse and plan it, then to actually
execute and return the result). I am running on 7.2, so,
PreparedStatements are not an option for me - I had to write a stored
procedure in "C", that caches tne query plan in the backend, runs it,
concatenates the results into a pipe separated text string (you can't
have functions return rows in 7.2 either), and to modify my java app to
call that udr, and parse the returned text back into columns instead of
sending the plain sql query and getting the ResultSet...

That alone gave me about 40% performance improvement...

Dima

Julien Le Goff wrote:

>Hello everyone,
>
>I have a question regarding the efficiency of Prepared Statements. I'm
>working on a project, and my task now is to decide whether it's worth
>it to use PS. This problem came up when, beginning to implement jdbc
>classes, we noticed that we would need a lot of PS - something like 40
>per class. Each PS would be a class variable, and it sounds weird to
>have 40 class variables... We could have a more elegant system using
>normal statements, but would it be much less efficient?
>
>I started doing some very simple tests: inserting 1000 elements to a
>table, doing 1.000.000 simple queries, then 1.000.000 queries with a
>join... But suprisingly, Prepared Statements didn't give better results
>than normal statements. Before warning the world that prepared
>statements are a big lie, I wanted to have your opinion. Has anyone
>done a reliable test showing the difference between PS and normal
>statements? Does anyone know "how" better PS are supposed to be?
>
>Then, concerning my test, what the hell could be wrong in what I did?
>The query is the following:
>
>String theJoinQueryPrepared =
>"SELECT tr.text FROM truc tr, test te " +
>"WHERE tr.id = te.id AND te.id = ?";
>
>for a Prepared Statement, and
>
>String theJoinQuery = "SELECT tr.text FROM truc tr, test te  " +
> WHERE tr.id = te.id AND te.id = ";
>
>for a Statement.
>
>Then I just do:
>
>    for(int j = 0; j < 1000; j++)
>    {
>     for(int i = 0; i < 1000; i++)
>     {
>        thePS.setInt(1, i);
>        ResultSet theResultSet = thePS.executeQuery();
>
>     }
>    }
>
>and
>
>    for(int j = 0; j < 1000; j++)
>    {
>        for(int i = 0; i < 1000; i++)
>        {
>            ResultSet theResultSet =
>                theStatement.executeQuery(
>                        theJoinQueryPrepared + i);
>        }
>    }
>
>I realize that this test is ridiculously simple, but shouldn't the first
>loop be more efficient? On my server both are equally fast...
>
>Ok, I hope this message wasn't too long / too stupid. Thanks in advance,
>
>Julien
>
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 4: Don't 'kill -9' the postmaster
>
>

As for "40 member variables" - what we do is have *one* member variable,
which is a Map, containing prepared statements, keyed by the names, that
are meaningful to the app, so that the code, that uses those PS looks
something like:

PreparedStatement ps = (PreparedStatement) m_MyResources.get
(LOAD_USER_NAMES);
ps.setObject (1, foo);
ps.setObject (2, bar);
return ps.execute ();

Dima

On 16/07/2003 21:24 Julien Le Goff wrote:
> Hello everyone,
>
> I have a question regarding the efficiency of Prepared Statements. I'm
> working on a project, and my task now is to decide whether it's worth
> it to use PS. This problem came up when, beginning to implement jdbc
> classes, we noticed that we would need a lot of PS - something like 40
> per class. Each PS would be a class variable, and it sounds weird to
> have 40 class variables... We could have a more elegant system using
> normal statements, but would it be much less efficient?

I use PreparedStatements all the time. They don't have to be class
variables so whoever is telling you really ought to learn to program in
Java.

>
> I started doing some very simple tests: inserting 1000 elements to a
> table, doing 1.000.000 simple queries, then 1.000.000 queries with a
> join... But suprisingly, Prepared Statements didn't give better results
> than normal statements. Before warning the world that prepared
> statements are a big lie, I wanted to have your opinion. Has anyone
> done a reliable test showing the difference between PS and normal
> statements? Does anyone know "how" better PS are supposed to be?

I think you're correct that there's currently no performance benefit with
PS although this may change in some future release.
>
> Then, concerning my test, what the hell could be wrong in what I did?
> The query is the following:
>
> String theJoinQueryPrepared =
> "SELECT tr.text FROM truc tr, test te " +
> "WHERE tr.id = te.id AND te.id = ?";
>
> for a Prepared Statement, and
>
> String theJoinQuery = "SELECT tr.text FROM truc tr, test te  " +
>  WHERE tr.id = te.id AND te.id = ";
>
> for a Statement.
>
> Then I just do:
>
>     for(int j = 0; j < 1000; j++)
>     {
>      for(int i = 0; i < 1000; i++)
>      {
>         thePS.setInt(1, i);
>         ResultSet theResultSet = thePS.executeQuery();
>
>      }
>     }
>
> and
>
>     for(int j = 0; j < 1000; j++)
>     {
>         for(int i = 0; i < 1000; i++)
>         {
>             ResultSet theResultSet =
>                 theStatement.executeQuery(
>                         theJoinQueryPrepared + i);
>         }
>     }
>
> I realize that this test is ridiculously simple, but shouldn't the first
> loop be more efficient? On my server both are equally fast...
>
> Ok, I hope this message wasn't too long / too stupid. Thanks in advance,

Forget performance for a moment and consider database security. Lets
imagine that you have an address book table called address_book

CREATE TABLE address_booK
(
     name        varchar(30),
     address     text
);

and you want to select a row by name. You might write

String query = "SELECT * from address_book WHERE name = "+strName

where strName was typed in by the user. What would happen if the user
typed:

joe;delete from address_book

This is a security hole known as SQL injection. If you are using a normal
Statement then your users can probably delete whole tables from the
database but with a PreparedStatement you would write

String query = "SELECT * from address_book WHERE name = ?"

and the command actually passed over to the database would be

SELECT * from address_book WHERE name = 'joe;delete from address_book'

I'm sure you can see the difference. Maybe PreparedStatements will have a
performance gain in some future release but at the moment they have a
vital role to play in database security.

HTH

--
Paul Thomas
+------------------------------+---------------------------------------------+
| Thomas Micro Systems Limited | Software Solutions for the Smaller
Business |
| Computer Consultants         |
http://www.thomas-micro-systems-ltd.co.uk   |
+------------------------------+---------------------------------------------+

Anyone know where I can find documentation or an sample implementation of a good preparedstatement caching mechanism?
Iwas thinking on implement one and would like to have some type of reference to look 

What do you mean by caching preparedstatements?


Felipe Schnack wrote:

>   Anyone know where I can find documentation or an sample implementation of a good preparedstatement caching
mechanism?I was thinking on implement one and would like to have some type of reference to look 
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match

You are running jdk 1.3.1?

Try upgrading to 1.4.1 if possible.

Cheers,

Kim

On Thu, 2003-07-17 at 09:16, Arun Desai wrote:
> Hi,
>      I am using Postgresql 7.3.3 version. I have a Java application that talks to the database using JDBC. I wanted
touse CallableStatement for functions that return refcursor. Since 7.3.3 jdbc driver does not support this feature
(ofcourseI know it supports CallableStatement but not for invoking functions that return Refcursor), I had to get the
latestJDBC sources from the CVS web and compile the driver. I am getting the following compilation errors. Can anyone
pleaselet me know how to get rid off them? 
>
>
> [javac] symbol  : method getOffset  (long)
>     [javac] location: class java.util.TimeZone
>     [javac]                     localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
>     [javac]                                                                     ^
>     [javac] /root/postgresql-7.3.3/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java:2104: cannot
resolve
> symbol
>     [javac] symbol  : method getOffset  (long)
>     [javac] location: class java.util.TimeZone
>     [javac]                     localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
>     [javac]                                                                     ^
>     [javac] /root/postgresql-7.3.3/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java:2149: cannot
resolve
> symbol
>     [javac] symbol  : method getOffset  (long)
>     [javac] location: class java.util.TimeZone
>     [javac]                     localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
>     [javac]                                                                     ^
>     [javac] Note: Some input files use or override a deprecated API.
>     [javac] Note: Recompile with -deprecation for details.
>     [javac] 3 errors
>
>
>
>
> Thanks and Regards,
> Arun Desai.
>

Yes I am using JDK 1.3.1. Can I comment out something in the jdbc driver
source to make it compile on JDK 1.3.1. Migrating to JDK1.4 could have some
repercussions on our existing application.
Please let me know how to compile the same sucessfully using jdk1.3.1.


Thanks,
Arun Desai.

----- Original Message -----
From: "Kim Ho" <kho@redhat.com>
To: "Arun Desai" <Arundesai@kinera.com>
Cc: "pgsql-jdbc-list" <pgsql-jdbc@postgresql.org>; "Barry Lind"
<blind@xythos.com>
Sent: Thursday, July 17, 2003 6:35 PM
Subject: Re: [JDBC] JDBC driver compilation error


> You are running jdk 1.3.1?
>
> Try upgrading to 1.4.1 if possible.
>
> Cheers,
>
> Kim
>
> On Thu, 2003-07-17 at 09:16, Arun Desai wrote:
> > Hi,
> >      I am using Postgresql 7.3.3 version. I have a Java application that
talks to the database using JDBC. I wanted to use CallableStatement for
functions that return refcursor. Since 7.3.3 jdbc driver does not support
this feature (ofcourse I know it supports CallableStatement but not for
invoking functions that return Refcursor), I had to get the latest JDBC
sources from the CVS web and compile the driver. I am getting the following
compilation errors. Can anyone please let me know how to get rid off them?
> >
> >
> > [javac] symbol  : method getOffset  (long)
> >     [javac] location: class java.util.TimeZone
> >     [javac]                     localoffset =
java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> >     [javac]
^
> >     [javac]
/root/postgresql-7.3.3/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc
1Statement.java:2104: cannot resolve
> > symbol
> >     [javac] symbol  : method getOffset  (long)
> >     [javac] location: class java.util.TimeZone
> >     [javac]                     localoffset =
java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> >     [javac]
^
> >     [javac]
/root/postgresql-7.3.3/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc
1Statement.java:2149: cannot resolve
> > symbol
> >     [javac] symbol  : method getOffset  (long)
> >     [javac] location: class java.util.TimeZone
> >     [javac]                     localoffset =
java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> >     [javac]
^
> >     [javac] Note: Some input files use or override a deprecated API.
> >     [javac] Note: Recompile with -deprecation for details.
> >     [javac] 3 errors
> >
> >
> >
> >
> > Thanks and Regards,
> > Arun Desai.
> >
>

>
> Forget performance for a moment and consider database security. Lets
> imagine that you have an address book table called address_book
>
> CREATE TABLE address_booK
> (
>     name        varchar(30),
>     address     text
> );
>
> and you want to select a row by name. You might write
>
> String query = "SELECT * from address_book WHERE name = "+strName
>
> where strName was typed in by the user. What would happen if the user
> typed:
>
> joe;delete from address_book



Either the exact same thing as what you describe below with the
PreparedStatement, or a syntax eror if you forget to put the user's
input into quotes when constracting your sql :-)

>
> This is a security hole known as SQL injection.

No, it isn't :-)
The "hole" you are referring to is letting the users type in entire
queries, not just input parameters.
As long as you have control over how your sql is constructed, you not
any less (nor any more) safe with plain Statements than you would be
with PreparedStatements. The do the same exact thing.

Dima

> If you are using a normal Statement then your users can probably
> delete whole tables from the database but with a PreparedStatement you
> would write
>
> String query = "SELECT * from address_book WHERE name = ?"
>
> and the command actually passed over to the database would be
>
> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>
> I'm sure you can see the difference. Maybe PreparedStatements will
> have a performance gain in some future release but at the moment they
> have a vital role to play in database security.
>

On Thu, 2003-07-17 at 16:47, Dmitry Tkach wrote:
[snip]
> >
> > This is a security hole known as SQL injection.
>
> No, it isn't :-)
> The "hole" you are referring to is letting the users type in entire
> queries, not just input parameters.
> As long as you have control over how your sql is constructed, you not
> any less (nor any more) safe with plain Statements than you would be
> with PreparedStatements. The do the same exact thing.

In my understanding the prepared statement will properly escape any
parameter so it can be trusted that the resulting query will not contain
something you wouldn't expect. Example (< and > are delimiters, ignore
them):

query: <SELECT * from address_book WHERE name = ?>

input: <joe';delete from address_book where 'true>

result if you just replace the <?> with <'$input'>:
SELECT * from address_book WHERE name = 'joe';delete from address_book
where 'true'
-> results in 2 statements executed

result if you use prepared statement:
SELECT * from address_book WHERE name = 'joe\';delete from address_book
where \'true'

As you can see, the "clever" injection attack is still rejected through
escaping the <'> characters. You can't possibly cause SQL injection
while using prepared statements.

Cheers,
Csaba.

>
> Dima
>
> > If you are using a normal Statement then your users can probably
> > delete whole tables from the database but with a PreparedStatement you
> > would write
> >
> > String query = "SELECT * from address_book WHERE name = ?"
> >
> > and the command actually passed over to the database would be
> >
> > SELECT * from address_book WHERE name = 'joe;delete from address_book'
> >
> > I'm sure you can see the difference. Maybe PreparedStatements will
> > have a performance gain in some future release but at the moment they
> > have a vital role to play in database security.
> >
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

>
>
>In my understanding the prepared statement will properly escape any
>parameter so it can be trusted that the resulting query will not contain
>something you wouldn't expect. Example (< and > are delimiters, ignore
>them):
>
>query: <SELECT * from address_book WHERE name = ?>
>
>input: <joe';delete from address_book where 'true>
>
>result if you just replace the <?> with <'$input'>:
>SELECT * from address_book WHERE name = 'joe';delete from address_book
>where 'true'
>-> results in 2 statements executed
>
Nope. You missed a quote :-)

The resulting query would be:
SELECT * from address_book WHERE name = 'joe'';delete from address_book 
where 'true';

This will be a syntax error - not "2 statements executed"... not even 
one statement :-)

But that's not the point anyway.
The app that accepts user input the way you describe and just puts 
quotes around it is of little use anyway ...
To be useful, it would have to take care about escaping the special 
characters on its own - not even to prevent "injection attacs", but just 
to be functional in the way that doesn't generate unexpected syntax 
errors (or just totally wrong data being entered)  just because the 
user's input happens to contain a character that has a special meaning 
to the parser.

Dima

On Thu, 2003-07-17 at 17:27, Dmitry Tkach wrote:
> >
> >
> >In my understanding the prepared statement will properly escape any
> >parameter so it can be trusted that the resulting query will not contain
> >something you wouldn't expect. Example (< and > are delimiters, ignore
> >them):
> >
> >query: <SELECT * from address_book WHERE name = ?>
> >
> >input: <joe';delete from address_book where 'true>
> >
> >result if you just replace the <?> with <'$input'>:
> >SELECT * from address_book WHERE name = 'joe';delete from address_book
> >where 'true'
> >-> results in 2 statements executed
> >
> Nope. You missed a quote :-)

No, I didn't. Take a closer look. The user inputs a string which
contains single quotes matching the quotes placed by the program around
the user supplied input. I never said that this method is a good one, on
the contrary, if the program would use the prepared statement instead,
it could safely use the user supplied text as parameter, because the
JDBC driver will safely escape SQL injection attempts. It will also make
sure there will be no syntax error.
The conclusion is that if you must include in your queries strings
coming from the user (and most of the applications do, just think about
a form where you supply your name - that's a string which will go in to
the DB), then use prepared statements, and never build the query
yourself.

So plain queries are not the same as prepared, because you have to
construct them, and believe me that it's easy to make mistakes here...
you can rely on prepared queries being safe, but implementing your own
escaping mechanisms will just give you a false sense of security, and
possibly cause you lots of trouble.

I hope it's clear enough now what I meant.

Cheers,
Csaba.

>
> The resulting query would be:
> SELECT * from address_book WHERE name = 'joe'';delete from address_book
> where 'true';
>
> This will be a syntax error - not "2 statements executed"... not even
> one statement :-)
>
> But that's not the point anyway.
> The app that accepts user input the way you describe and just puts
> quotes around it is of little use anyway ...
> To be useful, it would have to take care about escaping the special
> characters on its own - not even to prevent "injection attacs", but just
> to be functional in the way that doesn't generate unexpected syntax
> errors (or just totally wrong data being entered)  just because the
> user's input happens to contain a character that has a special meaning
> to the parser.
>
> Dima
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly
>

To solve this problem we have stored procedures, know in PostgreSQL land as
'functions'.


Dmitry Tkach wrote:>>
>> Forget performance for a moment and consider database security. Lets
>> imagine that you have an address book table called address_book
>>
>> CREATE TABLE address_booK
>> (
>>     name        varchar(30),
>>     address     text
>> );
>>
>> and you want to select a row by name. You might write
>>
>> String query = "SELECT * from address_book WHERE name = "+strName
>>
>> where strName was typed in by the user. What would happen if the user
>> typed:
>>
>> joe;delete from address_book
>
>
>
>
> Either the exact same thing as what you describe below with the
> PreparedStatement, or a syntax eror if you forget to put the user's
> input into quotes when constracting your sql :-)
>
>>
>> This is a security hole known as SQL injection.
>
>
> No, it isn't :-)
> The "hole" you are referring to is letting the users type in entire
> queries, not just input parameters.
> As long as you have control over how your sql is constructed, you not
> any less (nor any more) safe with plain Statements than you would be
> with PreparedStatements. The do the same exact thing.
>
> Dima
>
>> If you are using a normal Statement then your users can probably
>> delete whole tables from the database but with a PreparedStatement you
>> would write
>>
>> String query = "SELECT * from address_book WHERE name = ?"
>>
>> and the command actually passed over to the database would be
>>
>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>
>> I'm sure you can see the difference. Maybe PreparedStatements will
>> have a performance gain in some future release but at the moment they
>> have a vital role to play in database security.
>>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>



--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

We have found that using PostgreSQL (>7.3) prepared statements is, in
general, more efficient.  We have a lot of queries that are executed
multiple times (sometimes every 30 seconds) and we generally see better
efficiency.  You must, of course, set the useServerPrepare to true
otherwise the query will not be "prepared" by the backend (by default it
is false):

PreparedStatement actionSt = conn.prepareStatement( myquery );
( (PGStatement) actionSt ).setUseServerPrepare( true );

However, we have also seen the reverse effect, depending on the query.
Some queries are not prepared well by the backend and actually execute
*slower* than without setting the useServerPrepare variable.  This is
rare, but when it happens it is can multiply the execution time by 10 or
more.

So from my experience, the efficiency of prepared statements is based on
how well the backend is able to prepare the query.

Nick


On Wed, 2003-07-16 at 22:24, Julien Le Goff wrote:
> Hello everyone,
>
> I have a question regarding the efficiency of Prepared Statements. I'm
> working on a project, and my task now is to decide whether it's worth
> it to use PS. This problem came up when, beginning to implement jdbc
> classes, we noticed that we would need a lot of PS - something like 40
> per class. Each PS would be a class variable, and it sounds weird to
> have 40 class variables... We could have a more elegant system using
> normal statements, but would it be much less efficient?
>
> I started doing some very simple tests: inserting 1000 elements to a
> table, doing 1.000.000 simple queries, then 1.000.000 queries with a
> join... But suprisingly, Prepared Statements didn't give better results
> than normal statements. Before warning the world that prepared
> statements are a big lie, I wanted to have your opinion. Has anyone
> done a reliable test showing the difference between PS and normal
> statements? Does anyone know "how" better PS are supposed to be?
>
> Then, concerning my test, what the hell could be wrong in what I did?
> The query is the following:
>
> String theJoinQueryPrepared =
> "SELECT tr.text FROM truc tr, test te " +
> "WHERE tr.id = te.id AND te.id = ?";
>
> for a Prepared Statement, and
>
> String theJoinQuery = "SELECT tr.text FROM truc tr, test te  " +
>  WHERE tr.id = te.id AND te.id = ";
>
> for a Statement.
>
> Then I just do:
>
>     for(int j = 0; j < 1000; j++)
>     {
>      for(int i = 0; i < 1000; i++)
>      {
>         thePS.setInt(1, i);
>         ResultSet theResultSet = thePS.executeQuery();
>
>      }
>     }
>
> and
>
>     for(int j = 0; j < 1000; j++)
>     {
>         for(int i = 0; i < 1000; i++)
>         {
>             ResultSet theResultSet =
>                 theStatement.executeQuery(
>                         theJoinQueryPrepared + i);
>         }
>     }
>
> I realize that this test is ridiculously simple, but shouldn't the first
> loop be more efficient? On my server both are equally fast...
>
> Ok, I hope this message wasn't too long / too stupid. Thanks in advance,
>
> Julien
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

On 17/07/2003 15:47 Dmitry Tkach wrote:
>>
>> Forget performance for a moment and consider database security. Lets
>> imagine that you have an address book table called address_book
>>
>> CREATE TABLE address_booK
>> (
>>     name        varchar(30),
>>     address     text
>> );
>>
>> and you want to select a row by name. You might write
>>
>> String query = "SELECT * from address_book WHERE name = "+strName
>>
>> where strName was typed in by the user. What would happen if the user
>> typed:
>>
>> joe;delete from address_book
>
>
>
> Either the exact same thing as what you describe below with the
> PreparedStatement, or a syntax eror if you forget to put the user's
> input into quotes when constracting your sql :-)


Guilty as charged. still it was gone midnight... Still, I hope the general
meaning is still clear.

>>
>> This is a security hole known as SQL injection.
>
> No, it isn't :-)
> The "hole" you are referring to is letting the users type in entire
> queries, not just input parameters.

I've certainly seen this referred to as SQL injection. We'll just have to
agree to differ on this one :-)

> As long as you have control over how your sql is constructed, you not
> any less (nor any more) safe with plain Statements than you would be
> with PreparedStatements. The do the same exact thing.

You just need to be aware that there is some extra validation/parsing work
to do when using Statement.

regards

--
Paul Thomas
+------------------------------+---------------------------------------------+
| Thomas Micro Systems Limited | Software Solutions for the Smaller
Business |
| Computer Consultants         |
http://www.thomas-micro-systems-ltd.co.uk   |
+------------------------------+---------------------------------------------+

On 17 Jul 2003, Kim Ho wrote:

> You are running jdk 1.3.1?
>
> Try upgrading to 1.4.1 if possible.

Upgrading is not a reasonable answer.  Anything in the jdbc1 directory
must be compilable by the 1.1 jdk.  Anything in the jdbc2 directory must
be compilable by the 1.2 and 1.3 jdk.  Only the jdbc3 directory can
require the 1.4 jdk.

Kris Jurka

>
> On Thu, 2003-07-17 at 09:16, Arun Desai wrote:
> > Hi,
> >      I am using Postgresql 7.3.3 version. I have a Java application that talks to the database using JDBC. I wanted
touse CallableStatement for functions that return refcursor. Since 7.3.3 jdbc driver does not support this feature
(ofcourseI know it supports CallableStatement but not for invoking functions that return Refcursor), I had to get the
latestJDBC sources from the CVS web and compile the driver. I am getting the following compilation errors. Can anyone
pleaselet me know how to get rid off them? 
> >
> >
> > [javac] symbol  : method getOffset  (long)
> >     [javac] location: class java.util.TimeZone
> >     [javac]                     localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> >     [javac]                                                                     ^
> >     [javac] /root/postgresql-7.3.3/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java:2104:
cannotresolve 
> > symbol
> >     [javac] symbol  : method getOffset  (long)
> >     [javac] location: class java.util.TimeZone
> >     [javac]                     localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> >     [javac]                                                                     ^
> >     [javac] /root/postgresql-7.3.3/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java:2149:
cannotresolve 
> > symbol
> >     [javac] symbol  : method getOffset  (long)
> >     [javac] location: class java.util.TimeZone
> >     [javac]                     localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> >     [javac]                                                                     ^
> >     [javac] Note: Some input files use or override a deprecated API.
> >     [javac] Note: Recompile with -deprecation for details.
> >     [javac] 3 errors
> >
> >
> >
> >
> > Thanks and Regards,
> > Arun Desai.
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

I have to disagree; SQL injection can happen just from input parameters, as
described. The only thing left out was the quotes. If you construct the
query as:
String query = "SELECT * from address_book WHERE name = '" + userInput +
"'";

Then the user needs to change his input to: "joe'; delete from
address_book; '"

I don't know about the JDBC driver, but perl's DBI driver would handle the
above IF it were a parameterized query by escaping all quotes in the user's
input. So if instead of constructing it by hand, you had the "WHERE name
= ?" form and the user passed in the above, postgresql would see:

SELECT * from address_book WHERE name = 'joe''; delete from address_book;
''

(I'm assuming postgresql escapes quotes by doubling them, I don't recall
for sure.)
Hopefully the JDBC driver will do this as well. If not, then all user input
needs to be scanned for quotes, semicolons, etc., so they can be properly
escaped to avoid SQL injection attacks. Incidentally, such attacks might be
a second select query instead of deleting records, so as to get info on all
users in the database instead of just themselves for instance. In that case
it would be much less obvious that an attack had occurred.

Wes Sheldahl



Dmitry Tkach <dmitry@openratings.com>@postgresql.org on 07/17/2003 10:47:49
AM

Sent by:    pgsql-jdbc-owner@postgresql.org


To:    Paul Thomas <paul@tmsl.demon.co.uk>
cc:    "pgsql-jdbc @ postgresql . org" <pgsql-jdbc@postgresql.org>
Subject:    Re: [JDBC] Prepared Statements



>
> This is a security hole known as SQL injection.

No, it isn't :-)
The "hole" you are referring to is letting the users type in entire
queries, not just input parameters.
As long as you have control over how your sql is constructed, you not
any less (nor any more) safe with plain Statements than you would be
with PreparedStatements. The do the same exact thing.

Dima

> If you are using a normal Statement then your users can probably
> delete whole tables from the database but with a PreparedStatement you
> would write
>
> String query = "SELECT * from address_book WHERE name = ?"
>
> and the command actually passed over to the database would be
>
> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>
> I'm sure you can see the difference. Maybe PreparedStatements will
> have a performance gain in some future release but at the moment they
> have a vital role to play in database security.
>




---------------------------(end of broadcast)---------------------------
 TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

If using a PreparedStatement the driver correctly escapes all values to
avoid SQL injection attacks.  While this can also be done when using a
regular Statement object, it is then the resposibility of the programmer
to a) remember they need to escape, b) know specificially how postgresql
needs things escaped, and c) to actually escape all user input.
Invariably this will be forgotten some of the time and therefore I would
always recommend using PreparedStatements when you don't have control
over the values that are being used in the SQL statements.

thanks,
--Barry


wsheldah@lexmark.com wrote:
> I have to disagree; SQL injection can happen just from input parameters, as
> described. The only thing left out was the quotes. If you construct the
> query as:
> String query = "SELECT * from address_book WHERE name = '" + userInput +
> "'";
>
> Then the user needs to change his input to: "joe'; delete from
> address_book; '"
>
> I don't know about the JDBC driver, but perl's DBI driver would handle the
> above IF it were a parameterized query by escaping all quotes in the user's
> input. So if instead of constructing it by hand, you had the "WHERE name
> = ?" form and the user passed in the above, postgresql would see:
>
> SELECT * from address_book WHERE name = 'joe''; delete from address_book;
> ''
>
> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
> for sure.)
> Hopefully the JDBC driver will do this as well. If not, then all user input
> needs to be scanned for quotes, semicolons, etc., so they can be properly
> escaped to avoid SQL injection attacks. Incidentally, such attacks might be
> a second select query instead of deleting records, so as to get info on all
> users in the database instead of just themselves for instance. In that case
> it would be much less obvious that an attack had occurred.
>
> Wes Sheldahl
>
>
>
> Dmitry Tkach <dmitry@openratings.com>@postgresql.org on 07/17/2003 10:47:49
> AM
>
> Sent by:    pgsql-jdbc-owner@postgresql.org
>
>
> To:    Paul Thomas <paul@tmsl.demon.co.uk>
> cc:    "pgsql-jdbc @ postgresql . org" <pgsql-jdbc@postgresql.org>
> Subject:    Re: [JDBC] Prepared Statements
>
>
>
>
>>This is a security hole known as SQL injection.
>
>
> No, it isn't :-)
> The "hole" you are referring to is letting the users type in entire
> queries, not just input parameters.
> As long as you have control over how your sql is constructed, you not
> any less (nor any more) safe with plain Statements than you would be
> with PreparedStatements. The do the same exact thing.
>
> Dima
>
>
>>If you are using a normal Statement then your users can probably
>>delete whole tables from the database but with a PreparedStatement you
>>would write
>>
>>String query = "SELECT * from address_book WHERE name = ?"
>>
>>and the command actually passed over to the database would be
>>
>>SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>
>>I'm sure you can see the difference. Maybe PreparedStatements will
>>have a performance gain in some future release but at the moment they
>>have a vital role to play in database security.
>>
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
>  TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

> If using a PreparedStatement the driver correctly escapes all values to
> avoid SQL injection attacks.  While this can also be done when using a
> regular Statement object, it is then the resposibility of the programmer
> to a) remember they need to escape, b) know specificially how postgresql
> needs things escaped, and c) to actually escape all user input.
> Invariably this will be forgotten some of the time and therefore I would
> always recommend using PreparedStatements when you don't have control over
> the values that are being used in the SQL statements.

This is a corrolary to your point b, but one of the most convincing
argument I've ever heard for using prepared statements is when you change
databases you don't have to check and possibly modify all of your escaping
code. You also don't have to check any code you use for formatting
numbers, dates, booleans, etc.

Michael

--
Web Applications Developer
Open World Ltd, 11 Riverside Court, Riverside Road, Bath, BA2 3DZ.
Tel: +44 1225 444950  Fax: +44 1225 336738  http://www.openworld.co.uk/

CONFIDENTIALITY NOTICE
The information contained in this message is confidential, intended only
for the use of the individual or the entity named as recipient. If the
reader of this message is not that recipient, you are notified that any
dissemination, distribution or copy of this message is strictly
prohibited. If you have received this message in error, please immediately
notify us by telephone on the number above. Your co-operation is
appreciated.

Barry Lind wrote:

> If using a PreparedStatement the driver correctly escapes all values
> to avoid SQL injection attacks.

No, it doesn't :-)
For example:

PreparedStatement s = c.prepareStatement ("select * from user where id =
?");
s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
System.out.println (s.toString ());

select * from user where id=null;drop database mydb

:-)

Dima


> While this can also be done when using a regular Statement object, it
> is then the resposibility of the programmer to a) remember they need
> to escape, b) know specificially how postgresql needs things escaped,
> and c) to actually escape all user input. Invariably this will be
> forgotten some of the time and therefore I would always recommend
> using PreparedStatements when you don't have control over the values
> that are being used in the SQL statements.
>
> thanks,
> --Barry
>
>
> wsheldah@lexmark.com wrote:
>
>> I have to disagree; SQL injection can happen just from input
>> parameters, as
>> described. The only thing left out was the quotes. If you construct the
>> query as:
>> String query = "SELECT * from address_book WHERE name = '" + userInput +
>> "'";
>>
>> Then the user needs to change his input to: "joe'; delete from
>> address_book; '"
>>
>> I don't know about the JDBC driver, but perl's DBI driver would
>> handle the
>> above IF it were a parameterized query by escaping all quotes in the
>> user's
>> input. So if instead of constructing it by hand, you had the "WHERE name
>> = ?" form and the user passed in the above, postgresql would see:
>>
>> SELECT * from address_book WHERE name = 'joe''; delete from
>> address_book;
>> ''
>>
>> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
>> for sure.)
>> Hopefully the JDBC driver will do this as well. If not, then all user
>> input
>> needs to be scanned for quotes, semicolons, etc., so they can be
>> properly
>> escaped to avoid SQL injection attacks. Incidentally, such attacks
>> might be
>> a second select query instead of deleting records, so as to get info
>> on all
>> users in the database instead of just themselves for instance. In
>> that case
>> it would be much less obvious that an attack had occurred.
>>
>> Wes Sheldahl
>>
>>
>>
>> Dmitry Tkach <dmitry@openratings.com>@postgresql.org on 07/17/2003
>> 10:47:49
>> AM
>>
>> Sent by:    pgsql-jdbc-owner@postgresql.org
>>
>>
>> To:    Paul Thomas <paul@tmsl.demon.co.uk>
>> cc:    "pgsql-jdbc @ postgresql . org" <pgsql-jdbc@postgresql.org>
>> Subject:    Re: [JDBC] Prepared Statements
>>
>>
>>
>>
>>> This is a security hole known as SQL injection.
>>
>>
>>
>> No, it isn't :-)
>> The "hole" you are referring to is letting the users type in entire
>> queries, not just input parameters.
>> As long as you have control over how your sql is constructed, you not
>> any less (nor any more) safe with plain Statements than you would be
>> with PreparedStatements. The do the same exact thing.
>>
>> Dima
>>
>>
>>> If you are using a normal Statement then your users can probably
>>> delete whole tables from the database but with a PreparedStatement you
>>> would write
>>>
>>> String query = "SELECT * from address_book WHERE name = ?"
>>>
>>> and the command actually passed over to the database would be
>>>
>>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>>
>>> I'm sure you can see the difference. Maybe PreparedStatements will
>>> have a performance gain in some future release but at the moment they
>>> have a vital role to play in database security.
>>>
>>
>>
>>
>>
>>
>> ---------------------------(end of broadcast)---------------------------
>>  TIP 1: subscribe and unsubscribe commands go to
>> majordomo@postgresql.org
>>
>>
>>
>>
>>
>> ---------------------------(end of broadcast)---------------------------
>> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>>
>
>

This looks like a bug to me.
Still, once these kind of bugs are identified and fixed, it's much safer
to rely on the prepared statement's security than on your own escaping.
Besides, if you would set the parameter as string (as you would normally
do in an application) and not as an integer, the escaping would be
correct. And numeric parameters normally get checked for valid
formatting in the application, so this would not slip through.

Cheers,
Csasba.


On Fri, 2003-07-18 at 16:18, Dmitry Tkach wrote:
> Barry Lind wrote:
>
> > If using a PreparedStatement the driver correctly escapes all values
> > to avoid SQL injection attacks.
>
> No, it doesn't :-)
> For example:
>
> PreparedStatement s = c.prepareStatement ("select * from user where id =
> ?");
> s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> System.out.println (s.toString ());
>
> select * from user where id=null;drop database mydb
>
> :-)
>
> Dima
>
>
> > While this can also be done when using a regular Statement object, it
> > is then the resposibility of the programmer to a) remember they need
> > to escape, b) know specificially how postgresql needs things escaped,
> > and c) to actually escape all user input. Invariably this will be
> > forgotten some of the time and therefore I would always recommend
> > using PreparedStatements when you don't have control over the values
> > that are being used in the SQL statements.
> >
> > thanks,
> > --Barry
> >
> >
> > wsheldah@lexmark.com wrote:
> >
> >> I have to disagree; SQL injection can happen just from input
> >> parameters, as
> >> described. The only thing left out was the quotes. If you construct the
> >> query as:
> >> String query = "SELECT * from address_book WHERE name = '" + userInput +
> >> "'";
> >>
> >> Then the user needs to change his input to: "joe'; delete from
> >> address_book; '"
> >>
> >> I don't know about the JDBC driver, but perl's DBI driver would
> >> handle the
> >> above IF it were a parameterized query by escaping all quotes in the
> >> user's
> >> input. So if instead of constructing it by hand, you had the "WHERE name
> >> = ?" form and the user passed in the above, postgresql would see:
> >>
> >> SELECT * from address_book WHERE name = 'joe''; delete from
> >> address_book;
> >> ''
> >>
> >> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
> >> for sure.)
> >> Hopefully the JDBC driver will do this as well. If not, then all user
> >> input
> >> needs to be scanned for quotes, semicolons, etc., so they can be
> >> properly
> >> escaped to avoid SQL injection attacks. Incidentally, such attacks
> >> might be
> >> a second select query instead of deleting records, so as to get info
> >> on all
> >> users in the database instead of just themselves for instance. In
> >> that case
> >> it would be much less obvious that an attack had occurred.
> >>
> >> Wes Sheldahl
> >>
> >>
> >>
> >> Dmitry Tkach <dmitry@openratings.com>@postgresql.org on 07/17/2003
> >> 10:47:49
> >> AM
> >>
> >> Sent by:    pgsql-jdbc-owner@postgresql.org
> >>
> >>
> >> To:    Paul Thomas <paul@tmsl.demon.co.uk>
> >> cc:    "pgsql-jdbc @ postgresql . org" <pgsql-jdbc@postgresql.org>
> >> Subject:    Re: [JDBC] Prepared Statements
> >>
> >>
> >>
> >>
> >>> This is a security hole known as SQL injection.
> >>
> >>
> >>
> >> No, it isn't :-)
> >> The "hole" you are referring to is letting the users type in entire
> >> queries, not just input parameters.
> >> As long as you have control over how your sql is constructed, you not
> >> any less (nor any more) safe with plain Statements than you would be
> >> with PreparedStatements. The do the same exact thing.
> >>
> >> Dima
> >>
> >>
> >>> If you are using a normal Statement then your users can probably
> >>> delete whole tables from the database but with a PreparedStatement you
> >>> would write
> >>>
> >>> String query = "SELECT * from address_book WHERE name = ?"
> >>>
> >>> and the command actually passed over to the database would be
> >>>
> >>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
> >>>
> >>> I'm sure you can see the difference. Maybe PreparedStatements will
> >>> have a performance gain in some future release but at the moment they
> >>> have a vital role to play in database security.
> >>>
> >>
> >>
> >>
> >>
> >>
> >> ---------------------------(end of broadcast)---------------------------
> >>  TIP 1: subscribe and unsubscribe commands go to
> >> majordomo@postgresql.org
> >>
> >>
> >>
> >>
> >>
> >> ---------------------------(end of broadcast)---------------------------
> >> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> >>
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

Dmitry Tkach wrote:
> Barry Lind wrote:
>
>> If using a PreparedStatement the driver correctly escapes all values
>> to avoid SQL injection attacks.
>
>
> No, it doesn't :-)
> For example:
>
> PreparedStatement s = c.prepareStatement ("select * from user where id =
> ?");
> s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> System.out.println (s.toString ());
>
> select * from user where id=null;drop database mydb
>
> :-)
>

I don't believe this is actually being sent to the backend, maybe it is
just a toString() bug.

The backend should get:

select * from user where id='null;drop database mydb'

(If it does not it is a bug.)


P.S.: The example case would only succeed if the DBA is an idiot.
You program should not be accessing the database (for this queries at
least) as an user who can drop databases unless it is a privileged
program for privileged users (who could do the damage using plain psql
anyway).  Perhaps the injection of a 'DELETE FROM mytable' would be a
more realistic example.


--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

I have checked, the query is indeed sent like that to the backend, I've
just checked.
It is a bug.
Presumably for number types the parameter set is passed as it is,
without any escaping.

Cheers,
Csaba.


On Fri, 2003-07-18 at 16:38, Fernando Nasser wrote:
> Dmitry Tkach wrote:
> > Barry Lind wrote:
> >
> >> If using a PreparedStatement the driver correctly escapes all values
> >> to avoid SQL injection attacks.
> >
> >
> > No, it doesn't :-)
> > For example:
> >
> > PreparedStatement s = c.prepareStatement ("select * from user where id =
> > ?");
> > s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> > System.out.println (s.toString ());
> >
> > select * from user where id=null;drop database mydb
> >
> > :-)
> >
>
> I don't believe this is actually being sent to the backend, maybe it is
> just a toString() bug.
>
> The backend should get:
>
> select * from user where id='null;drop database mydb'
>
> (If it does not it is a bug.)
>
>
> P.S.: The example case would only succeed if the DBA is an idiot.
> You program should not be accessing the database (for this queries at
> least) as an user who can drop databases unless it is a privileged
> program for privileged users (who could do the damage using plain psql
> anyway).  Perhaps the injection of a 'DELETE FROM mytable' would be a
> more realistic example.
>
>
> --
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
>

Csaba Nagy wrote:
> This looks like a bug to me.
> Still, once these kind of bugs are identified and fixed,

That is why I like open source software, right there.


Erik

> In any case, try using server prepared statements by setting your
statement to
> use that option (you must be using a backend at least of version 7.3):
>
> setUseServerPrepare(true);

Hi folks-

Sorry to pull things backward here, but having set my web user up with sane
grants, I'm a bit more concerned about the performance issues- I can't find
this method in the Sun JDBC spec, so I'm assuming it is PostgreSQL-specific?

Is this the correct way to use it:

 PreparedStatement ps = db.prepareStatement("select foo from bar where foo
like ?");
 ps.setUseServerPrepare(true);
 .
 .
 .
 ps.setString(1, "fubar");
 ResultSet rs = ps.executeQuery();

(Where the first two lines get executed once when I init my app, but the
last two lines may get executed any number of times.)

Also- is the javadoc for postgresql JDBC online anywhere for folks like
myself who just download the current stable jarfile?

Thanks
     -NF

Fernando Nasser wrote:

>>
>
> I don't believe this is actually being sent to the backend, maybe it
> is just a toString() bug.

You better do believe it. I tried it, and it works. :-)

>
> The backend should get:
>
> select * from user where id='null;drop database mydb'
>
> (If it does not it is a bug.)

Nah... That's what it would get if you did setString()... setObject ()
doesn't work that way.
I tend to agree, it's a bug - if the type is INTEGER, it should be
checking if the object, passed in is really numeric.

The thing is that, at least, in the current state of the driver, this is
a *really nice* bug, that gives you the only way to use certain
functionality....
For example:

PreparedStatement stmt = c.prepareStatement ("select * from mytable
where data in ?");
stmt.setObject (1, "(1,2,3,4,5)", Types.INTEGER);

... if the "bug" was fixed, there would be no other way to do this kind
of thing :-(

>
>
>
> P.S.: The example case would only succeed if the DBA is an idiot.

No objection here :-)
But, in my opinion, the same comment applies to all the earlier examples
(without PreparedStatements) just as well - the point is, if you are an
idiot, you will trash your database one way or another, with or without
using PS, and if you are not, then you won't :-)

>
> You program should not be accessing the database (for this queries at
> least) as an user who can drop databases unless it is a privileged
> program for privileged users (who could do the damage using plain psql
> anyway).  Perhaps the injection of a 'DELETE FROM mytable' would be a
> more realistic example.

Come on!... Replace 'drop databse' with just 'do whatever you want' :-)
I just put it in to make it look scarier :-) That was a joke ...
It's just an illustration of the nice 'injection attac' using
PreparedStatements, that everybody else around seems to believe is
impossible.

It isn't. If the person writing the code is an idiot, PreparedStatements
won't help him (nothing will), and if he isn't they won't help him
either (because he wouldn't need that kind of help).
I would like the performance benefit of PS (if there was any)... But
security? No way...
If you accept any kind of user input and send it to the database without
bothering to check what the hell is there, you will be doomed, and no
PreparedStatement in the world will save you :-)

Dima

Dmitry Tkach wrote:
> Fernando Nasser wrote:
>
>>>
>>
>> I don't believe this is actually being sent to the backend, maybe it
>> is just a toString() bug.
>
>
> You better do believe it. I tried it, and it works. :-)
>
>>
>> The backend should get:
>>
>> select * from user where id='null;drop database mydb'
>>
>> (If it does not it is a bug.)
>
>
> Nah... That's what it would get if you did setString()... setObject ()
> doesn't work that way.
> I tend to agree, it's a bug - if the type is INTEGER, it should be
> checking if the object, passed in is really numeric.
>
> The thing is that, at least, in the current state of the driver, this is
> a *really nice* bug, that gives you the only way to use certain
> functionality....
> For example:
>
> PreparedStatement stmt = c.prepareStatement ("select * from mytable
> where data in ?");
> stmt.setObject (1, "(1,2,3,4,5)", Types.INTEGER);
>
> ... if the "bug" was fixed, there would be no other way to do this kind
> of thing :-(
>

Well, I guess the bug will have be fixed asap as it is a security risk.

What is the proper JDBC way for filling IN lists in prepared statements?

--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

>
> Well, I guess the bug will have be fixed asap as it is a security risk.
>
> What is the proper JDBC way for filling IN lists in prepared statements?
>

I'm no JDBC expert, but the way we do it: create a prepared statement
with 100 (or whatever the max nr. of accepted params is) parameter
placeholders, and set the ones which are actually needed to their
parameter values, and set the rest to null.
The nulls will be finally ignored by the database.
Not the best solution, but it works just fine for us.

Cheers,
Csaba.

>
> Well, I guess the bug will have be fixed asap as it is a security risk.


I am afraid, it will :-(
That's exactly why, as I told you yesterday, I tried to avoid upgrading
my driver versions too frequently - because of the 'bug fixes' like
this, that break stuff....

>
> What is the proper JDBC way for filling IN lists in prepared statements?

I am afraid, there is no standard about it :-(
Depends on the vendor...
Most of them (not postgres) support SQLData - to let you define and pass
in arbitrary types...

Some (like infomirx for example... don't know about Oracle) have sets
and arrays interchangeable - so that
setObject (1, sqlArrayContainingIdsToMatch, Types.ARRAY)
can be used...

Some people are used to hacks, like one described in an earlier post -
where yuo create a statement with an awful lot of questionmarks, and
then set each member of the set separately and cross your fingers,
hoping that you have enough placeholders for your whole set...

Dima

That's a hack indeed, but there's no finger crossing :-)
We do chunk our queries if needed, i.e. execute them multiple times if
the parameter set is too big. We mostly use this technique to filter a
set according to some criteria. Now if the set is too big, we do the
filtering chunk-wise, and then cumulate the result. I admit that it
won't work for all situations, but then if you have a big set, you might
be looking for a different solution like dumping them in to a temporary
table.

Cheers,
Csaba.

> Some people are used to hacks, like one described in an earlier post -
> where yuo create a statement with an awful lot of questionmarks, and
> then set each member of the set separately and cross your fingers,
> hoping that you have enough placeholders for your whole set...
>
> Dima
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>

If it only skips the escaping for numeric types, the obvious workaround
would be first put the user's entry into an int variable:

int userId = getUserId();
PreparedStatement s = c.prepareStatement ("select * from user where id
= ?");
s.setObject(1, userId, Types.INTEGER);

That way you use java's built-in type checking to avoid sending non-numeric
data to the backend any time you're specifying a numeric type that will
skip the escaping.

Can someone confirm that it at least does do the escaping for
string/varchar inputs?

Wes Sheldahl



Csaba Nagy <nagy@ecircle-ag.com>@postgresql.org on 07/18/2003 10:46:33 AM

Sent by:    pgsql-jdbc-owner@postgresql.org


To:    Fernando Nasser <fnasser@redhat.com>
cc:    Dmitry Tkach <dmitry@openratings.com>, Barry Lind
       <blind@xythos.com>, wsheldah@lexmark.com, "pgsql-jdbc @ postgresql
       " ". org" <pgsql-jdbc@postgresql.org>
Subject:    Re: [JDBC] Prepared Statements


I have checked, the query is indeed sent like that to the backend, I've
just checked.
It is a bug.
Presumably for number types the parameter set is passed as it is,
without any escaping.

Cheers,
Csaba.


On Fri, 2003-07-18 at 16:38, Fernando Nasser wrote:
> Dmitry Tkach wrote:
> > Barry Lind wrote:
> >
> >> If using a PreparedStatement the driver correctly escapes all values
> >> to avoid SQL injection attacks.
> >
> >
> > No, it doesn't :-)
> > For example:
> >
> > PreparedStatement s = c.prepareStatement ("select * from user where id
=
> > ?");
> > s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> > System.out.println (s.toString ());
> >
> > select * from user where id=null;drop database mydb
> >
> > :-)
> >
>
> I don't believe this is actually being sent to the backend, maybe it is
> just a toString() bug.
>
> The backend should get:
>
> select * from user where id='null;drop database mydb'
>
> (If it does not it is a bug.)
>
>
> P.S.: The example case would only succeed if the DBA is an idiot.
> You program should not be accessing the database (for this queries at
> least) as an user who can drop databases unless it is a privileged
> program for privileged users (who could do the damage using plain psql
> anyway).  Perhaps the injection of a 'DELETE FROM mytable' would be a
> more realistic example.
>
>
> --
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
>



---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
      subscribe-nomail command to majordomo@postgresql.org so that your
       message can get through to the mailing list cleanly

Hi,

String fields are escaped (I believe this would cover CHAR, VARCHAR,
etc)

I added the single quotes when binding numbers in my patch for
registerOutParameter and that one is going to be reviewed by Dave.

Cheers,

Kim

On Fri, 2003-07-18 at 11:10, wsheldah@lexmark.com wrote:
>
> If it only skips the escaping for numeric types, the obvious workaround
> would be first put the user's entry into an int variable:
>
> int userId = getUserId();
> PreparedStatement s = c.prepareStatement ("select * from user where id
> = ?");
> s.setObject(1, userId, Types.INTEGER);
>
> That way you use java's built-in type checking to avoid sending non-numeric
> data to the backend any time you're specifying a numeric type that will
> skip the escaping.
>
> Can someone confirm that it at least does do the escaping for
> string/varchar inputs?
>
> Wes Sheldahl
>

In these cases, I just set a single question mark in the query... then I use setObject(index, parameters,
Types.NUMERIC)
  In the "parameters" variable I pass the values concatenated, like:

  PreparedStatement prep = conn.preparePreparedStatement("SELECT * FROM foo WHERE bar IN (?)");
  prep.setObject(1, "1, 2, 3", Types.NUMERIC);

  The problem about this technique is that I can't use driver's scaping of Strings... I just hope this keeps working in
futureversions of the driver :-) 
  There is a way that I can cann driver's scaping methods? Would be nice if they were public.

On 18 Jul 2003 17:32:34 +0200
Csaba Nagy <nagy@ecircle-ag.com> wrote:

> >
> > Well, I guess the bug will have be fixed asap as it is a security risk.
> >
> > What is the proper JDBC way for filling IN lists in prepared statements?
> >
>
> I'm no JDBC expert, but the way we do it: create a prepared statement
> with 100 (or whatever the max nr. of accepted params is) parameter
> placeholders, and set the ones which are actually needed to their
> parameter values, and set the rest to null.
> The nulls will be finally ignored by the database.
> Not the best solution, but it works just fine for us.
>
> Cheers,
> Csaba.
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

Dmitry,

That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?

--Barry

Dmitry Tkach wrote:
> Barry Lind wrote:
>
>> If using a PreparedStatement the driver correctly escapes all values
>> to avoid SQL injection attacks.
>
>
> No, it doesn't :-)
> For example:
>
> PreparedStatement s = c.prepareStatement ("select * from user where id =
> ?");
> s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> System.out.println (s.toString ());
>
> select * from user where id=null;drop database mydb
>
> :-)
>
> Dima
>
>
>> While this can also be done when using a regular Statement object, it
>> is then the resposibility of the programmer to a) remember they need
>> to escape, b) know specificially how postgresql needs things escaped,
>> and c) to actually escape all user input. Invariably this will be
>> forgotten some of the time and therefore I would always recommend
>> using PreparedStatements when you don't have control over the values
>> that are being used in the SQL statements.
>>
>> thanks,
>> --Barry
>>
>>
>> wsheldah@lexmark.com wrote:
>>
>>> I have to disagree; SQL injection can happen just from input
>>> parameters, as
>>> described. The only thing left out was the quotes. If you construct the
>>> query as:
>>> String query = "SELECT * from address_book WHERE name = '" + userInput +
>>> "'";
>>>
>>> Then the user needs to change his input to: "joe'; delete from
>>> address_book; '"
>>>
>>> I don't know about the JDBC driver, but perl's DBI driver would
>>> handle the
>>> above IF it were a parameterized query by escaping all quotes in the
>>> user's
>>> input. So if instead of constructing it by hand, you had the "WHERE name
>>> = ?" form and the user passed in the above, postgresql would see:
>>>
>>> SELECT * from address_book WHERE name = 'joe''; delete from
>>> address_book;
>>> ''
>>>
>>> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
>>> for sure.)
>>> Hopefully the JDBC driver will do this as well. If not, then all user
>>> input
>>> needs to be scanned for quotes, semicolons, etc., so they can be
>>> properly
>>> escaped to avoid SQL injection attacks. Incidentally, such attacks
>>> might be
>>> a second select query instead of deleting records, so as to get info
>>> on all
>>> users in the database instead of just themselves for instance. In
>>> that case
>>> it would be much less obvious that an attack had occurred.
>>>
>>> Wes Sheldahl
>>>
>>>
>>>
>>> Dmitry Tkach <dmitry@openratings.com>@postgresql.org on 07/17/2003
>>> 10:47:49
>>> AM
>>>
>>> Sent by:    pgsql-jdbc-owner@postgresql.org
>>>
>>>
>>> To:    Paul Thomas <paul@tmsl.demon.co.uk>
>>> cc:    "pgsql-jdbc @ postgresql . org" <pgsql-jdbc@postgresql.org>
>>> Subject:    Re: [JDBC] Prepared Statements
>>>
>>>
>>>
>>>
>>>> This is a security hole known as SQL injection.
>>>
>>>
>>>
>>>
>>> No, it isn't :-)
>>> The "hole" you are referring to is letting the users type in entire
>>> queries, not just input parameters.
>>> As long as you have control over how your sql is constructed, you not
>>> any less (nor any more) safe with plain Statements than you would be
>>> with PreparedStatements. The do the same exact thing.
>>>
>>> Dima
>>>
>>>
>>>> If you are using a normal Statement then your users can probably
>>>> delete whole tables from the database but with a PreparedStatement you
>>>> would write
>>>>
>>>> String query = "SELECT * from address_book WHERE name = ?"
>>>>
>>>> and the command actually passed over to the database would be
>>>>
>>>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>>>
>>>> I'm sure you can see the difference. Maybe PreparedStatements will
>>>> have a performance gain in some future release but at the moment they
>>>> have a vital role to play in database security.
>>>>
>>>
>>>
>>>
>>>
>>>
>>> ---------------------------(end of broadcast)---------------------------
>>>  TIP 1: subscribe and unsubscribe commands go to
>>> majordomo@postgresql.org
>>>
>>>
>>>
>>>
>>>
>>> ---------------------------(end of broadcast)---------------------------
>>> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>>>
>>
>>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

Nick,

Nick Fankhauser wrote:
>>In any case, try using server prepared statements by setting your
>
> statement to
>
>>use that option (you must be using a backend at least of version 7.3):
>>
>>setUseServerPrepare(true);
>
>
> Hi folks-
>
> Sorry to pull things backward here, but having set my web user up with sane
> grants, I'm a bit more concerned about the performance issues- I can't find
> this method in the Sun JDBC spec, so I'm assuming it is PostgreSQL-specific?
>
> Is this the correct way to use it:
>
>  PreparedStatement ps = db.prepareStatement("select foo from bar where foo
> like ?");
>  ps.setUseServerPrepare(true);
>  .

Close, but you need to cast your PreparedStatement to
org.postgresql.PGStatement to get access to this method.

>  .
>  .
>  ps.setString(1, "fubar");
>  ResultSet rs = ps.executeQuery();
>
> (Where the first two lines get executed once when I init my app, but the
> last two lines may get executed any number of times.)
>
> Also- is the javadoc for postgresql JDBC online anywhere for folks like
> myself who just download the current stable jarfile?
>

This should be documented in the JDBC chapter of the Postgresql
Programers Guide in the section titled Postgresql Extensions to the JDBC
API.  But this section of the doc is out of date and therefore you
wouldn't find this information there.

thanks,
--Barry

Barry Lind wrote:
> Dmitry,
>
> That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
>

Kim's patch fixes this.  It is pending approval.



--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

To speed things up a bit, since the regoutParam patch is not likely to
be approved anytime soon.

This patch
- adds single quotes for numbers in setObject and also setInt/Byte/etc.
- Improves getInt/Long when you may have parser errors if you're too
close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
- Improves radix point handling when using setObject to an integer
parameter while passing in a float. This is especially important in
callable statements.

Cheers,

Kim

On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
> Barry Lind wrote:
> > Dmitry,
> >
> > That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
> >
>
> Kim's patch fixes this.  It is pending approval.
>
>
>
> --
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

Kim Ho wrote:

>To speed things up a bit, since the regoutParam patch is not likely to
>be approved anytime soon.
>
>This patch
>- adds single quotes for numbers in setObject and also setInt/Byte/etc.
>- Improves getInt/Long when you may have parser errors if you're too
>close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
>- Improves radix point handling when using setObject to an integer
>parameter while passing in a float. This is especially important in
>callable statements.
>
I see :-)
Aside from taking away that ability to be able to pass sets using
setObject(), which is unfortunate, about the only improvement this makes
seems to be that the malicious "injector" would have to pass in a string
like (just making sure it doesn't contain any dots :-)

1';delete from precious_table where 'true

to make a statement like

select * from somewhere where id=?

to get sent as "select * from somewhere where id='1';delete from
precious_table where 'true'" and wipe out your precious table :-)


You really believe you can win this race, by plugging this particular
hole, I am afraid, you are going to have to always parse the input
that,s supposed to be numerical into a number...


Dima

P.S. On a different note, something like
"select ?"
setString (1, "\047");

returns "\047" when executed. Now *this*, is a bug - because it is
supposed to return a string, containing a quote as a single character...






>
>Cheers,
>
>Kim
>
>On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>
>>Barry Lind wrote:
>>
>>
>>>Dmitry,
>>>
>>>That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
>>>
>>>
>>>
>>Kim's patch fixes this.  It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario   M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>>
>>
>
>
>
>
>------------------------------------------------------------------------
>
>? temp.diff
>Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
>retrieving revision 1.13
>diff -c -p -r1.13 AbstractJdbc1ResultSet.java
>*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    30 Jun 2003 21:10:55 -0000    1.13
>--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    18 Jul 2003 17:02:20 -0000
>*************** public abstract class AbstractJdbc1Resul
>*** 805,811 ****
>              try
>              {
>                  s = s.trim();
>!                 return Integer.parseInt(s);
>              }
>              catch (NumberFormatException e)
>              {
>--- 805,811 ----
>              try
>              {
>                  s = s.trim();
>!                 return Float.valueOf(s).intValue();
>              }
>              catch (NumberFormatException e)
>              {
>*************** public abstract class AbstractJdbc1Resul
>*** 822,828 ****
>              try
>              {
>                  s = s.trim();
>!                 return Long.parseLong(s);
>              }
>              catch (NumberFormatException e)
>              {
>--- 822,828 ----
>              try
>              {
>                  s = s.trim();
>!                 return Double.valueOf(s).longValue();
>              }
>              catch (NumberFormatException e)
>              {
>Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
>retrieving revision 1.27
>diff -c -p -r1.27 AbstractJdbc1Statement.java
>*** org/postgresql/jdbc1/AbstractJdbc1Statement.java    9 Jul 2003 05:12:04 -0000    1.27
>--- org/postgresql/jdbc1/AbstractJdbc1Statement.java    18 Jul 2003 17:02:22 -0000
>*************** public abstract class AbstractJdbc1State
>*** 920,926 ****
>       */
>      public void setByte(int parameterIndex, byte x) throws SQLException
>      {
>!         bind(parameterIndex, Integer.toString(x), PG_TEXT);
>      }
>
>      /*
>--- 920,926 ----
>       */
>      public void setByte(int parameterIndex, byte x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 933,939 ****
>       */
>      public void setShort(int parameterIndex, short x) throws SQLException
>      {
>!         bind(parameterIndex, Integer.toString(x), PG_INT2);
>      }
>
>      /*
>--- 933,939 ----
>       */
>      public void setShort(int parameterIndex, short x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 946,952 ****
>       */
>      public void setInt(int parameterIndex, int x) throws SQLException
>      {
>!         bind(parameterIndex, Integer.toString(x), PG_INTEGER);
>      }
>
>      /*
>--- 946,952 ----
>       */
>      public void setInt(int parameterIndex, int x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 959,965 ****
>       */
>      public void setLong(int parameterIndex, long x) throws SQLException
>      {
>!         bind(parameterIndex, Long.toString(x), PG_INT8);
>      }
>
>      /*
>--- 959,965 ----
>       */
>      public void setLong(int parameterIndex, long x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 972,978 ****
>       */
>      public void setFloat(int parameterIndex, float x) throws SQLException
>      {
>!         bind(parameterIndex, Float.toString(x), PG_FLOAT);
>      }
>
>      /*
>--- 972,978 ----
>       */
>      public void setFloat(int parameterIndex, float x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 985,991 ****
>       */
>      public void setDouble(int parameterIndex, double x) throws SQLException
>      {
>!         bind(parameterIndex, Double.toString(x), PG_DOUBLE);
>      }
>
>      /*
>--- 985,991 ----
>       */
>      public void setDouble(int parameterIndex, double x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 1003,1009 ****
>              setNull(parameterIndex, Types.DECIMAL);
>          else
>          {
>!             bind(parameterIndex, x.toString(), PG_NUMERIC);
>          }
>      }
>
>--- 1003,1009 ----
>              setNull(parameterIndex, Types.DECIMAL);
>          else
>          {
>!             bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
>          }
>      }
>
>*************** public abstract class AbstractJdbc1State
>*** 1464,1486 ****
>          switch (targetSqlType)
>          {
>              case Types.INTEGER:
>-                 if (x instanceof Boolean)
>-                     bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
>-                 else
>-                     bind(parameterIndex, x.toString(), PG_INTEGER);
>-                 break;
>              case Types.TINYINT:
>              case Types.SMALLINT:
>              case Types.BIGINT:
>              case Types.REAL:
>              case Types.FLOAT:
>              case Types.DOUBLE:
>              case Types.DECIMAL:
>              case Types.NUMERIC:
>!                 if (x instanceof Boolean)
>!                     bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
>!                 else
>!                     bind(parameterIndex, x.toString(), PG_NUMERIC);
>                  break;
>              case Types.CHAR:
>              case Types.VARCHAR:
>--- 1464,1484 ----
>          switch (targetSqlType)
>          {
>              case Types.INTEGER:
>              case Types.TINYINT:
>              case Types.SMALLINT:
>+                 x = removeRadix(x,Types.INTEGER);
>+                 bindNumber(parameterIndex,x,PG_INTEGER);
>+                 break;
>              case Types.BIGINT:
>+                 x = removeRadix(x,Types.BIGINT);
>+                 bindNumber(parameterIndex,x,PG_INT8);
>+                 break;
>              case Types.REAL:
>              case Types.FLOAT:
>              case Types.DOUBLE:
>              case Types.DECIMAL:
>              case Types.NUMERIC:
>!                 bindNumber(parameterIndex,x,PG_NUMERIC);
>                  break;
>              case Types.CHAR:
>              case Types.VARCHAR:
>*************** public abstract class AbstractJdbc1State
>*** 2026,2031 ****
>--- 2024,2056 ----
>          if (parameterIndex != 1)
>              throw new PSQLException("postgresql.call.noinout");
>      }
>+
>+     private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
>+     {
>+         if (x instanceof Boolean)
>+             bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
>+         else
>+             bind(parameterIndex, "'"+x.toString()+"'", pgtype);
>+     }
>+
>+
>+     private Object removeRadix(Object x, int sqlType)
>+     {
>+         if (x.toString().indexOf(".")>0)
>+         {
>+             switch (sqlType)
>+             {
>+                 case Types.BIGINT:
>+                     x = String.valueOf(Double.valueOf(x.toString()).longValue());
>+                     break;
>+                 default:
>+                     x = String.valueOf(Float.valueOf(x.toString()).intValue());
>+                     break;
>+             }
>+         }
>+         return x;
>+     }
>+
>
>
>
>
>
>------------------------------------------------------------------------
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
>      subscribe-nomail command to majordomo@postgresql.org so that your
>      message can get through to the mailing list cleanly
>
>

  Hey, this means I will not be able anymore to use setObject() to set my IN values like I did??

On Fri, 18 Jul 2003 13:27:13 -0400
Dmitry Tkach <dmitry@openratings.com> wrote:

> Kim Ho wrote:
>
> >To speed things up a bit, since the regoutParam patch is not likely to
> >be approved anytime soon.
> >
> >This patch
> >- adds single quotes for numbers in setObject and also setInt/Byte/etc.
> >- Improves getInt/Long when you may have parser errors if you're too
> >close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
> >- Improves radix point handling when using setObject to an integer
> >parameter while passing in a float. This is especially important in
> >callable statements.
> >
> I see :-)
> Aside from taking away that ability to be able to pass sets using
> setObject(), which is unfortunate, about the only improvement this makes
> seems to be that the malicious "injector" would have to pass in a string
> like (just making sure it doesn't contain any dots :-)
>
> 1';delete from precious_table where 'true
>
> to make a statement like
>
> select * from somewhere where id=?
>
> to get sent as "select * from somewhere where id='1';delete from
> precious_table where 'true'" and wipe out your precious table :-)
>
>
> You really believe you can win this race, by plugging this particular
> hole, I am afraid, you are going to have to always parse the input
> that,s supposed to be numerical into a number...
>
>
> Dima
>
> P.S. On a different note, something like
> "select ?"
> setString (1, "\047");
>
> returns "\047" when executed. Now *this*, is a bug - because it is
> supposed to return a string, containing a quote as a single character...
>
>
>
>
>
>
> >
> >Cheers,
> >
> >Kim
> >
> >On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
> >
> >
> >>Barry Lind wrote:
> >>
> >>
> >>>Dmitry,
> >>>
> >>>That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
> >>>
> >>>
> >>>
> >>Kim's patch fixes this.  It is pending approval.
> >>
> >>
> >>
> >>--
> >>Fernando Nasser
> >>Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> >>2323 Yonge Street, Suite #300
> >>Toronto, Ontario   M4P 2C9
> >>
> >>
> >>---------------------------(end of broadcast)---------------------------
> >>TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> >>
> >>
> >
> >
> >
> >
> >------------------------------------------------------------------------
> >
> >? temp.diff
> >Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
> >===================================================================
> >RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
> >retrieving revision 1.13
> >diff -c -p -r1.13 AbstractJdbc1ResultSet.java
> >*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    30 Jun 2003 21:10:55 -0000    1.13
> >--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    18 Jul 2003 17:02:20 -0000
> >*************** public abstract class AbstractJdbc1Resul
> >*** 805,811 ****
> >              try
> >              {
> >                  s = s.trim();
> >!                 return Integer.parseInt(s);
> >              }
> >              catch (NumberFormatException e)
> >              {
> >--- 805,811 ----
> >              try
> >              {
> >                  s = s.trim();
> >!                 return Float.valueOf(s).intValue();
> >              }
> >              catch (NumberFormatException e)
> >              {
> >*************** public abstract class AbstractJdbc1Resul
> >*** 822,828 ****
> >              try
> >              {
> >                  s = s.trim();
> >!                 return Long.parseLong(s);
> >              }
> >              catch (NumberFormatException e)
> >              {
> >--- 822,828 ----
> >              try
> >              {
> >                  s = s.trim();
> >!                 return Double.valueOf(s).longValue();
> >              }
> >              catch (NumberFormatException e)
> >              {
> >Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> >===================================================================
> >RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> >retrieving revision 1.27
> >diff -c -p -r1.27 AbstractJdbc1Statement.java
> >*** org/postgresql/jdbc1/AbstractJdbc1Statement.java    9 Jul 2003 05:12:04 -0000    1.27
> >--- org/postgresql/jdbc1/AbstractJdbc1Statement.java    18 Jul 2003 17:02:22 -0000
> >*************** public abstract class AbstractJdbc1State
> >*** 920,926 ****
> >       */
> >      public void setByte(int parameterIndex, byte x) throws SQLException
> >      {
> >!         bind(parameterIndex, Integer.toString(x), PG_TEXT);
> >      }
> >
> >      /*
> >--- 920,926 ----
> >       */
> >      public void setByte(int parameterIndex, byte x) throws SQLException
> >      {
> >!         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
> >      }
> >
> >      /*
> >*************** public abstract class AbstractJdbc1State
> >*** 933,939 ****
> >       */
> >      public void setShort(int parameterIndex, short x) throws SQLException
> >      {
> >!         bind(parameterIndex, Integer.toString(x), PG_INT2);
> >      }
> >
> >      /*
> >--- 933,939 ----
> >       */
> >      public void setShort(int parameterIndex, short x) throws SQLException
> >      {
> >!         bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
> >      }
> >
> >      /*
> >*************** public abstract class AbstractJdbc1State
> >*** 946,952 ****
> >       */
> >      public void setInt(int parameterIndex, int x) throws SQLException
> >      {
> >!         bind(parameterIndex, Integer.toString(x), PG_INTEGER);
> >      }
> >
> >      /*
> >--- 946,952 ----
> >       */
> >      public void setInt(int parameterIndex, int x) throws SQLException
> >      {
> >!         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
> >      }
> >
> >      /*
> >*************** public abstract class AbstractJdbc1State
> >*** 959,965 ****
> >       */
> >      public void setLong(int parameterIndex, long x) throws SQLException
> >      {
> >!         bind(parameterIndex, Long.toString(x), PG_INT8);
> >      }
> >
> >      /*
> >--- 959,965 ----
> >       */
> >      public void setLong(int parameterIndex, long x) throws SQLException
> >      {
> >!         bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
> >      }
> >
> >      /*
> >*************** public abstract class AbstractJdbc1State
> >*** 972,978 ****
> >       */
> >      public void setFloat(int parameterIndex, float x) throws SQLException
> >      {
> >!         bind(parameterIndex, Float.toString(x), PG_FLOAT);
> >      }
> >
> >      /*
> >--- 972,978 ----
> >       */
> >      public void setFloat(int parameterIndex, float x) throws SQLException
> >      {
> >!         bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
> >      }
> >
> >      /*
> >*************** public abstract class AbstractJdbc1State
> >*** 985,991 ****
> >       */
> >      public void setDouble(int parameterIndex, double x) throws SQLException
> >      {
> >!         bind(parameterIndex, Double.toString(x), PG_DOUBLE);
> >      }
> >
> >      /*
> >--- 985,991 ----
> >       */
> >      public void setDouble(int parameterIndex, double x) throws SQLException
> >      {
> >!         bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
> >      }
> >
> >      /*
> >*************** public abstract class AbstractJdbc1State
> >*** 1003,1009 ****
> >              setNull(parameterIndex, Types.DECIMAL);
> >          else
> >          {
> >!             bind(parameterIndex, x.toString(), PG_NUMERIC);
> >          }
> >      }
> >
> >--- 1003,1009 ----
> >              setNull(parameterIndex, Types.DECIMAL);
> >          else
> >          {
> >!             bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
> >          }
> >      }
> >
> >*************** public abstract class AbstractJdbc1State
> >*** 1464,1486 ****
> >          switch (targetSqlType)
> >          {
> >              case Types.INTEGER:
> >-                 if (x instanceof Boolean)
> >-                     bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
> >-                 else
> >-                     bind(parameterIndex, x.toString(), PG_INTEGER);
> >-                 break;
> >              case Types.TINYINT:
> >              case Types.SMALLINT:
> >              case Types.BIGINT:
> >              case Types.REAL:
> >              case Types.FLOAT:
> >              case Types.DOUBLE:
> >              case Types.DECIMAL:
> >              case Types.NUMERIC:
> >!                 if (x instanceof Boolean)
> >!                     bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
> >!                 else
> >!                     bind(parameterIndex, x.toString(), PG_NUMERIC);
> >                  break;
> >              case Types.CHAR:
> >              case Types.VARCHAR:
> >--- 1464,1484 ----
> >          switch (targetSqlType)
> >          {
> >              case Types.INTEGER:
> >              case Types.TINYINT:
> >              case Types.SMALLINT:
> >+                 x = removeRadix(x,Types.INTEGER);
> >+                 bindNumber(parameterIndex,x,PG_INTEGER);
> >+                 break;
> >              case Types.BIGINT:
> >+                 x = removeRadix(x,Types.BIGINT);
> >+                 bindNumber(parameterIndex,x,PG_INT8);
> >+                 break;
> >              case Types.REAL:
> >              case Types.FLOAT:
> >              case Types.DOUBLE:
> >              case Types.DECIMAL:
> >              case Types.NUMERIC:
> >!                 bindNumber(parameterIndex,x,PG_NUMERIC);
> >                  break;
> >              case Types.CHAR:
> >              case Types.VARCHAR:
> >*************** public abstract class AbstractJdbc1State
> >*** 2026,2031 ****
> >--- 2024,2056 ----
> >          if (parameterIndex != 1)
> >              throw new PSQLException("postgresql.call.noinout");
> >      }
> >+
> >+     private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
> >+     {
> >+         if (x instanceof Boolean)
> >+             bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
> >+         else
> >+             bind(parameterIndex, "'"+x.toString()+"'", pgtype);
> >+     }
> >+
> >+
> >+     private Object removeRadix(Object x, int sqlType)
> >+     {
> >+         if (x.toString().indexOf(".")>0)
> >+         {
> >+             switch (sqlType)
> >+             {
> >+                 case Types.BIGINT:
> >+                     x = String.valueOf(Double.valueOf(x.toString()).longValue());
> >+                     break;
> >+                 default:
> >+                     x = String.valueOf(Float.valueOf(x.toString()).intValue());
> >+                     break;
> >+             }
> >+         }
> >+         return x;
> >+     }
> >+
> >
> >
> >
> >
> >
> >------------------------------------------------------------------------
> >
> >
> >---------------------------(end of broadcast)---------------------------
> >TIP 3: if posting/reading through Usenet, please send an appropriate
> >      subscribe-nomail command to majordomo@postgresql.org so that your
> >      message can get through to the mailing list cleanly
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

And also, this breaks the hexademical notation:
PreparedStatement s = c.prepareStatement ("select * from mytable where
id=?");
s.setObject (1, "x'a'");

That works in the current driver will stop working after this patch is
applied.

Dima


Kim Ho wrote:

>To speed things up a bit, since the regoutParam patch is not likely to
>be approved anytime soon.
>
>This patch
>- adds single quotes for numbers in setObject and also setInt/Byte/etc.
>- Improves getInt/Long when you may have parser errors if you're too
>close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
>- Improves radix point handling when using setObject to an integer
>parameter while passing in a float. This is especially important in
>callable statements.
>
>Cheers,
>
>Kim
>
>On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>
>>Barry Lind wrote:
>>
>>
>>>Dmitry,
>>>
>>>That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
>>>
>>>
>>>
>>Kim's patch fixes this.  It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario   M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>>
>>
>
>
>
>
>------------------------------------------------------------------------
>
>? temp.diff
>Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
>retrieving revision 1.13
>diff -c -p -r1.13 AbstractJdbc1ResultSet.java
>*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    30 Jun 2003 21:10:55 -0000    1.13
>--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    18 Jul 2003 17:02:20 -0000
>*************** public abstract class AbstractJdbc1Resul
>*** 805,811 ****
>              try
>              {
>                  s = s.trim();
>!                 return Integer.parseInt(s);
>              }
>              catch (NumberFormatException e)
>              {
>--- 805,811 ----
>              try
>              {
>                  s = s.trim();
>!                 return Float.valueOf(s).intValue();
>              }
>              catch (NumberFormatException e)
>              {
>*************** public abstract class AbstractJdbc1Resul
>*** 822,828 ****
>              try
>              {
>                  s = s.trim();
>!                 return Long.parseLong(s);
>              }
>              catch (NumberFormatException e)
>              {
>--- 822,828 ----
>              try
>              {
>                  s = s.trim();
>!                 return Double.valueOf(s).longValue();
>              }
>              catch (NumberFormatException e)
>              {
>Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
>retrieving revision 1.27
>diff -c -p -r1.27 AbstractJdbc1Statement.java
>*** org/postgresql/jdbc1/AbstractJdbc1Statement.java    9 Jul 2003 05:12:04 -0000    1.27
>--- org/postgresql/jdbc1/AbstractJdbc1Statement.java    18 Jul 2003 17:02:22 -0000
>*************** public abstract class AbstractJdbc1State
>*** 920,926 ****
>       */
>      public void setByte(int parameterIndex, byte x) throws SQLException
>      {
>!         bind(parameterIndex, Integer.toString(x), PG_TEXT);
>      }
>
>      /*
>--- 920,926 ----
>       */
>      public void setByte(int parameterIndex, byte x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 933,939 ****
>       */
>      public void setShort(int parameterIndex, short x) throws SQLException
>      {
>!         bind(parameterIndex, Integer.toString(x), PG_INT2);
>      }
>
>      /*
>--- 933,939 ----
>       */
>      public void setShort(int parameterIndex, short x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 946,952 ****
>       */
>      public void setInt(int parameterIndex, int x) throws SQLException
>      {
>!         bind(parameterIndex, Integer.toString(x), PG_INTEGER);
>      }
>
>      /*
>--- 946,952 ----
>       */
>      public void setInt(int parameterIndex, int x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 959,965 ****
>       */
>      public void setLong(int parameterIndex, long x) throws SQLException
>      {
>!         bind(parameterIndex, Long.toString(x), PG_INT8);
>      }
>
>      /*
>--- 959,965 ----
>       */
>      public void setLong(int parameterIndex, long x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 972,978 ****
>       */
>      public void setFloat(int parameterIndex, float x) throws SQLException
>      {
>!         bind(parameterIndex, Float.toString(x), PG_FLOAT);
>      }
>
>      /*
>--- 972,978 ----
>       */
>      public void setFloat(int parameterIndex, float x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 985,991 ****
>       */
>      public void setDouble(int parameterIndex, double x) throws SQLException
>      {
>!         bind(parameterIndex, Double.toString(x), PG_DOUBLE);
>      }
>
>      /*
>--- 985,991 ----
>       */
>      public void setDouble(int parameterIndex, double x) throws SQLException
>      {
>!         bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
>      }
>
>      /*
>*************** public abstract class AbstractJdbc1State
>*** 1003,1009 ****
>              setNull(parameterIndex, Types.DECIMAL);
>          else
>          {
>!             bind(parameterIndex, x.toString(), PG_NUMERIC);
>          }
>      }
>
>--- 1003,1009 ----
>              setNull(parameterIndex, Types.DECIMAL);
>          else
>          {
>!             bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
>          }
>      }
>
>*************** public abstract class AbstractJdbc1State
>*** 1464,1486 ****
>          switch (targetSqlType)
>          {
>              case Types.INTEGER:
>-                 if (x instanceof Boolean)
>-                     bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
>-                 else
>-                     bind(parameterIndex, x.toString(), PG_INTEGER);
>-                 break;
>              case Types.TINYINT:
>              case Types.SMALLINT:
>              case Types.BIGINT:
>              case Types.REAL:
>              case Types.FLOAT:
>              case Types.DOUBLE:
>              case Types.DECIMAL:
>              case Types.NUMERIC:
>!                 if (x instanceof Boolean)
>!                     bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
>!                 else
>!                     bind(parameterIndex, x.toString(), PG_NUMERIC);
>                  break;
>              case Types.CHAR:
>              case Types.VARCHAR:
>--- 1464,1484 ----
>          switch (targetSqlType)
>          {
>              case Types.INTEGER:
>              case Types.TINYINT:
>              case Types.SMALLINT:
>+                 x = removeRadix(x,Types.INTEGER);
>+                 bindNumber(parameterIndex,x,PG_INTEGER);
>+                 break;
>              case Types.BIGINT:
>+                 x = removeRadix(x,Types.BIGINT);
>+                 bindNumber(parameterIndex,x,PG_INT8);
>+                 break;
>              case Types.REAL:
>              case Types.FLOAT:
>              case Types.DOUBLE:
>              case Types.DECIMAL:
>              case Types.NUMERIC:
>!                 bindNumber(parameterIndex,x,PG_NUMERIC);
>                  break;
>              case Types.CHAR:
>              case Types.VARCHAR:
>*************** public abstract class AbstractJdbc1State
>*** 2026,2031 ****
>--- 2024,2056 ----
>          if (parameterIndex != 1)
>              throw new PSQLException("postgresql.call.noinout");
>      }
>+
>+     private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
>+     {
>+         if (x instanceof Boolean)
>+             bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
>+         else
>+             bind(parameterIndex, "'"+x.toString()+"'", pgtype);
>+     }
>+
>+
>+     private Object removeRadix(Object x, int sqlType)
>+     {
>+         if (x.toString().indexOf(".")>0)
>+         {
>+             switch (sqlType)
>+             {
>+                 case Types.BIGINT:
>+                     x = String.valueOf(Double.valueOf(x.toString()).longValue());
>+                     break;
>+                 default:
>+                     x = String.valueOf(Float.valueOf(x.toString()).intValue());
>+                     break;
>+             }
>+         }
>+         return x;
>+     }
>+
>
>
>
>
>
>------------------------------------------------------------------------
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
>      subscribe-nomail command to majordomo@postgresql.org so that your
>      message can get through to the mailing list cleanly
>
>

Dmitry Tkach wrote:

>
> s.setObject (1, "x'a'");
>
I meant s.setObject (1, "x'a'", Types.INTEGER) of course...

Dima

Can't you instead use setString(1, "x'a'")?

If not, this also brings up another thing. Did you want to treat "x'a'"
as a number now?

In any case, here is a revised version of the patch. =) Thanks for the
pointers.

Also, the remove radix thing is not meant for preventing SQL injection.
It is meant for this like:

create function integer_in(integer) ....

and then using things like select integer_in(1.11231E9)

Kim

On Fri, 2003-07-18 at 13:40, Dmitry Tkach wrote:
> Dmitry Tkach wrote:
>
> >
> > s.setObject (1, "x'a'");
> >
> I meant s.setObject (1, "x'a'", Types.INTEGER) of course...
>
> Dima
>
>

Kim Ho wrote:

>Can't you instead use setString(1, "x'a'")?
>
Nope - that will get converted into ... where id='x\'a\'' - that won't
be understood by the backend - it wants it *exactly* that way - x
outside the quotes, followed by a quoted hexademical number...

>
>If not, this also brings up another thing. Did you want to treat "x'a'"
>as a number now?
>
Yes, I did (and still do) :-)

>
>In any case, here is a revised version of the patch. =) Thanks for the
>pointers.
>
I must be missing something, but I don't see any difference with the
previous version ....

>
>Also, the remove radix thing is not meant for preventing SQL injection.
>It is meant for this like:
>
>create function integer_in(integer) ....
>
>and then using things like select integer_in(1.11231E9)
>
I understand that. I was just saying that adding quotes around the input
doesn't help much in preventing injections, but does take away valuable
functionality...

Dima

> >
> >In any case, here is a revised version of the patch. =) Thanks for the
> >pointers.
> >
> I must be missing something, but I don't see any difference with the
> previous version ....
>
There is an attempt to do a:
Double.valueOf(x.toString());

in bindNumbers to ensure that the object's string representation is a
number. This is what you were talking about before right?

About breaking functionality with hex.

I'm not sure about this, maybe we could get other opinions in
(specifically Dave and Barry). I'm not so sure that it should be
allowed. (I am not saying that it is not useful.)

Cheers,

Kim

I just can't understand why a call to setObject(1, someString, Types.NUMERIC) would scape the contents of my
"someString"variable, as I specified that it's a number 

On Fri, 18 Jul 2003 13:53:39 -0400
Dmitry Tkach <dmitry@openratings.com> wrote:

> Kim Ho wrote:
>
> >Can't you instead use setString(1, "x'a'")?
> >
> Nope - that will get converted into ... where id='x\'a\'' - that won't
> be understood by the backend - it wants it *exactly* that way - x
> outside the quotes, followed by a quoted hexademical number...
>
> >
> >If not, this also brings up another thing. Did you want to treat "x'a'"
> >as a number now?
> >
> Yes, I did (and still do) :-)
>
> >
> >In any case, here is a revised version of the patch. =) Thanks for the
> >pointers.
> >
> I must be missing something, but I don't see any difference with the
> previous version ....
>
> >
> >Also, the remove radix thing is not meant for preventing SQL injection.
> >It is meant for this like:
> >
> >create function integer_in(integer) ....
> >
> >and then using things like select integer_in(1.11231E9)
> >
> I understand that. I was just saying that adding quotes around the input
> doesn't help much in preventing injections, but does take away valuable
> functionality...
>
> Dima
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faqs/FAQ.html


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

Kim Ho wrote:

>>>In any case, here is a revised version of the patch. =) Thanks for the
>>>pointers.
>>>
>>>
>>>
>>I must be missing something, but I don't see any difference with the
>>previous version ....
>>
>>
>>
>There is an attempt to do a:
>Double.valueOf(x.toString());
>
>in bindNumbers to ensure that the object's string representation is a
>number. This is what you were talking about before right?
>
>
Oh, I see...
Now you end up parsing it twice in some cases (once in removeRadix(),
and once again in bindNumber()) - why don't you merge those two together
to save one parse?

Not that I care, because I am not going to be able to use this at all
(and will have to build my own driver if it ever gets in), because it
breaks the existing functionality :-(

The hex thing may be questionable - although, I do sympathize with
people who may be using it currently, and will be forced to rewrite
their app now, but, at least, they have a (relatively) simple solution
(which is to take on the  backend's parsing of hex numbers, and do it on
the app side)...
What I am concerned about is the "in" thing -

select * from sometable where x in ?;
setObject (1, "(1,2,3,4,5)");

that works just fine right now, and will be irreperably broken by this
patch...
No way I am going to rewrite all the existing code to do something like

select * from sometable where x in

(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
... and then loop through the set and set each param separately, hoping
I have enough questionmarks to fit them all in :-)


Dima

>About breaking functionality with hex.
>
>I'm not sure about this, maybe we could get other opinions in
>(specifically Dave and Barry). I'm not so sure that it should be
>allowed. (I am not saying that it is not useful.)
>
>Cheers,
>
>Kim
>
>

Yes, If a patch prevents this use of setObject() for IN clauses, and also will HAVE to create my own driver,
somethingI'm trying to avoid at all costs 

On Fri, 18 Jul 2003 14:22:24 -0400
Dmitry Tkach <dmitry@openratings.com> wrote:

> Kim Ho wrote:
>
> >>>In any case, here is a revised version of the patch. =) Thanks for the
> >>>pointers.
> >>>
> >>>
> >>>
> >>I must be missing something, but I don't see any difference with the
> >>previous version ....
> >>
> >>
> >>
> >There is an attempt to do a:
> >Double.valueOf(x.toString());
> >
> >in bindNumbers to ensure that the object's string representation is a
> >number. This is what you were talking about before right?
> >
> >
> Oh, I see...
> Now you end up parsing it twice in some cases (once in removeRadix(),
> and once again in bindNumber()) - why don't you merge those two together
> to save one parse?
>
> Not that I care, because I am not going to be able to use this at all
> (and will have to build my own driver if it ever gets in), because it
> breaks the existing functionality :-(
>
> The hex thing may be questionable - although, I do sympathize with
> people who may be using it currently, and will be forced to rewrite
> their app now, but, at least, they have a (relatively) simple solution
> (which is to take on the  backend's parsing of hex numbers, and do it on
> the app side)...
> What I am concerned about is the "in" thing -
>
> select * from sometable where x in ?;
> setObject (1, "(1,2,3,4,5)");
>
> that works just fine right now, and will be irreperably broken by this
> patch...
> No way I am going to rewrite all the existing code to do something like
>
> select * from sometable where x in
>
(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
> ... and then loop through the set and set each param separately, hoping
> I have enough questionmarks to fit them all in :-)
>
>
> Dima
>
> >About breaking functionality with hex.
> >
> >I'm not sure about this, maybe we could get other opinions in
> >(specifically Dave and Barry). I'm not so sure that it should be
> >allowed. (I am not saying that it is not useful.)
> >
> >Cheers,
> >
> >Kim
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

If I could throw in a question from the sidelines...

Is this something that a PreparedStatement should be able to handle, in general?

>
> select * from sometable where x in ?;
> setObject (1, "(1,2,3,4,5)", Types.INTEGER);
>
>

With a PreparedStatement that gets precompiled, will a substitution like this
work?  Do other JDBC drivers support this kind of substitution?

It looks like you are exploiting a bug (or perhaps an out of spec behaviour) in
the JDBC.  If that is the case, then I don't have much sympathy for you losing
this functionality.

Instead of creating your own driver, why not just subclass Statement into
something that looks like a PreparedStatement but just glues strings together?
That sounds like what you want anyway.  It also seems like a much eaiser task,
especially for long term maintainance.

Darin

Here is the patch that only parses once. =/ Thanks Dmitry.

I have to say that I agree with Darin. If it's just an exploit, it
doesn't really belong in the JDBC driver.

Cheers,

Kim

Well, the fact is that this patch would break existing code... so some people, like me, would not be able to update
ourdrivers anymore. 

On Fri, 18 Jul 2003 14:55:55 -0400
Darin Ohashi <DOhashi@maplesoft.com> wrote:

>
> If I could throw in a question from the sidelines...
>
> Is this something that a PreparedStatement should be able to handle, in general?
>
> >
> > select * from sometable where x in ?;
> > setObject (1, "(1,2,3,4,5)", Types.INTEGER);
> >
> >
>
> With a PreparedStatement that gets precompiled, will a substitution like this
> work?  Do other JDBC drivers support this kind of substitution?
>
> It looks like you are exploiting a bug (or perhaps an out of spec behaviour) in
> the JDBC.  If that is the case, then I don't have much sympathy for you losing
> this functionality.
>
> Instead of creating your own driver, why not just subclass Statement into
> something that looks like a PreparedStatement but just glues strings together?
> That sounds like what you want anyway.  It also seems like a much eaiser task,
> especially for long term maintainance.
>
> Darin
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

Darin Ohashi wrote:

>If I could throw in a question from the sidelines...
>
>Is this something that a PreparedStatement should be able to handle, in general?
>
>
I think yes.
Depends on your definiton of "in general", of course,
Where I come from, at least "in general", the PreparedStatement should
be able to handle *everything* you can do with sql.

>
>
>With a PreparedStatement that gets precompiled, will a substitution like this
>work?
>
Depends on the implementation...
If the backend doesn't support it, the driver should take care of
working around that
(e.g. by switching to 'non-precompiled' version in this case) until the
backend is able to support it (I don't see any reason why it just cannot
be done theoretically).

> Do other JDBC drivers support this kind of substitution?
>
Yes. They do (at least, the ones I worked with do).

>
>It looks like you are exploiting a bug (or perhaps an out of spec behaviour) in
>the JDBC.  If that is the case, then I don't have much sympathy for you losing
>this functionality.
>
I am not going to lose it :-)
Thanks to the driver being opensource I have control over what I am
going to lose :-)

>
>Instead of creating your own driver, why not just subclass Statement into
>something that looks like a PreparedStatement but just glues strings together?
>
Ummm... because it already exists? That's what postgres
PreparedStatement does (by default in 7.3, and always in previous
versions)...

>That sounds like what you want anyway.  It also seems like a much eaiser task,
>especially for long term maintainance.
>
>
No, that's not what I want...
What I want is an abstraction of the implementation-specific details,
that lets me effectively execute sql queries.
If it can precompile my query, I want it to precompile it. If it cannot,
I can live with that (for a while).
I just don't want to *know* what it is doing.

Dima

> I think yes.
> Depends on your definiton of "in general", of course,
> Where I come from, at least "in general", the
> PreparedStatement should
> be able to handle *everything* you can do with sql.

Well, what I meant by "in general", is more along the lines of "is this
behaviour covered by the spec".  Does the JDBC spec explicitly deny this kind of
behaviour.

My understanding, which might be wrong, is that "?" is a placeholder for a data
value and not a placeholder for SQL syntax, so, for example I would never expect
a PS would be able to do something like this:

ps := c.prepareStatement( "??" );
ps.setString( 1, "SELECT * " );
ps.setString( 2, "FROM table" );
ps.execute();

The final SQL statement produced by a PS should be any valid SQL string, but a
PS should not have to deal with being able to put that string together from
arbitrary pieces of data.  I would be surprised if there was not a document
somewhere that specified what is and is not valid to pass via a "?".

Looking at this particular example, it looks to me (of course, I have a limited
amount of SQL experience) that "(1,2,3,4)" contains syntax.  Using (?,?,?,?) the
"?" replace data and that should work.  As "(1,2,3,4)" contains syntax, I don't
think it is a valid thing to substitute in.

> Depends on the implementation...
> If the backend doesn't support it, the driver should take care of
> working around that
> (e.g. by switching to 'non-precompiled' version in this case)
> until the
> backend is able to support it (I don't see any reason why it
> just cannot
> be done theoretically).

Well, if you allow syntax to be substituted in to the PS, then the backend can't
precompile because it does not have a syntatically complete statement.  What you
substitute in could completely change the query optimization.

>
> > Do other JDBC drivers support this kind of substitution?
> >
> Yes. They do (at least, the ones I worked with do).

Do they use actual precompiled statements or just with string concats?

>> I am not going to lose it :-)
> Thanks to the driver being opensource I have control over what I am
> going to lose :-)

Lose it from the official driver.  You can, of course, do what ever you like
with your personal version of the driver.

>
> >
> >Instead of creating your own driver, why not just subclass
> Statement into
> >something that looks like a PreparedStatement but just glues
> strings together?
> >
> Ummm... because it already exists? That's what postgres
> PreparedStatement does (by default in 7.3, and always in previous
> versions)...

That is what it currently does, I don't think that is what it will do in the
future when (if) it actually represents  a precompiled statement.

> No, that's not what I want...
> What I want is an abstraction of the implementation-specific details,
> that lets me effectively execute sql queries.
> If it can precompile my query, I want it to precompile it. If
> it cannot,
> I can live with that (for a while).
> I just don't want to *know* what it is doing.

I accept that, but then you have to be willing to live by the rules of the spec,
and I suspect this behaviour violates the spec.

Darin

I believe the main problem is that we would simply break existing code. This is a serious issue! If this patch do get
applied,I'm a lucky person to read this list, because I can protect myself from problems in my system knowing about
this...but lots of people probably will have serious problems using this new driver versions... 

On Fri, 18 Jul 2003 16:22:39 -0400
Darin Ohashi <DOhashi@maplesoft.com> wrote:

> > I think yes.
> > Depends on your definiton of "in general", of course,
> > Where I come from, at least "in general", the
> > PreparedStatement should
> > be able to handle *everything* you can do with sql.
>
> Well, what I meant by "in general", is more along the lines of "is this
> behaviour covered by the spec".  Does the JDBC spec explicitly deny this kind of
> behaviour.
>
> My understanding, which might be wrong, is that "?" is a placeholder for a data
> value and not a placeholder for SQL syntax, so, for example I would never expect
> a PS would be able to do something like this:
>
> ps := c.prepareStatement( "??" );
> ps.setString( 1, "SELECT * " );
> ps.setString( 2, "FROM table" );
> ps.execute();
>
> The final SQL statement produced by a PS should be any valid SQL string, but a
> PS should not have to deal with being able to put that string together from
> arbitrary pieces of data.  I would be surprised if there was not a document
> somewhere that specified what is and is not valid to pass via a "?".
>
> Looking at this particular example, it looks to me (of course, I have a limited
> amount of SQL experience) that "(1,2,3,4)" contains syntax.  Using (?,?,?,?) the
> "?" replace data and that should work.  As "(1,2,3,4)" contains syntax, I don't
> think it is a valid thing to substitute in.
>
> > Depends on the implementation...
> > If the backend doesn't support it, the driver should take care of
> > working around that
> > (e.g. by switching to 'non-precompiled' version in this case)
> > until the
> > backend is able to support it (I don't see any reason why it
> > just cannot
> > be done theoretically).
>
> Well, if you allow syntax to be substituted in to the PS, then the backend can't
> precompile because it does not have a syntatically complete statement.  What you
> substitute in could completely change the query optimization.
>
> >
> > > Do other JDBC drivers support this kind of substitution?
> > >
> > Yes. They do (at least, the ones I worked with do).
>
> Do they use actual precompiled statements or just with string concats?
>
> >> I am not going to lose it :-)
> > Thanks to the driver being opensource I have control over what I am
> > going to lose :-)
>
> Lose it from the official driver.  You can, of course, do what ever you like
> with your personal version of the driver.
>
> >
> > >
> > >Instead of creating your own driver, why not just subclass
> > Statement into
> > >something that looks like a PreparedStatement but just glues
> > strings together?
> > >
> > Ummm... because it already exists? That's what postgres
> > PreparedStatement does (by default in 7.3, and always in previous
> > versions)...
>
> That is what it currently does, I don't think that is what it will do in the
> future when (if) it actually represents  a precompiled statement.
>
> > No, that's not what I want...
> > What I want is an abstraction of the implementation-specific details,
> > that lets me effectively execute sql queries.
> > If it can precompile my query, I want it to precompile it. If
> > it cannot,
> > I can live with that (for a while).
> > I just don't want to *know* what it is doing.
>
> I accept that, but then you have to be willing to live by the rules of the spec,
> and I suspect this behaviour violates the spec.
>
> Darin
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

Darin Ohashi wrote:

>The final SQL statement produced by a PS should be any valid SQL string, but a
>PS should not have to deal with being able to put that string together from
>arbitrary pieces of data.
>
I agree with you on this one - not *arbitrary* pieces of data, no.

> I would be surprised if there was not a document
>somewhere that specified what is and is not valid to pass via a "?".
>
>
If there is any, I've never heard of it...

>Looking at this particular example, it looks to me (of course, I have a limited
>amount of SQL experience) that "(1,2,3,4)" contains syntax.  Using (?,?,?,?) the
>"?" replace data and that should work.  As "(1,2,3,4)" contains syntax, I don't
>think it is a valid thing to substitute in.
>
Well... "(1,2,3,4)" *does* contain syntax. I agree on this one too.
*But*, (1,2,34)   itself is *not* just an arbitrary set of data, but a
syntactical construct that represents a set.
I never said that using setObject (1, "(1,2,3,4,5)") is the ideal
solution to my problem - it is just a workaround to the drivers
inability to pass in a set any other way.
I would much prefer to use something like setObject (1,
javaSetOfIntegers) if I could.
But we don't have it, and what I have now is better than nothing at all.

>
>Well, if you allow syntax to be substituted in to the PS, then the backend can't
>precompile because it does not have a syntatically complete statement.  What you
>substitute in could completely change the query optimization.
>
>
Sure. Even if you substitute just an int, it can very well change the
optimal query plan
select ... where x=1 or select ... where x = 2000 could result in two
totally different plans.
This is a known complication of precompiled statements.

One just has to know when it is better to precompile your queries and
when it is not.

>
>
>>>Do other JDBC drivers support this kind of substitution?
>>>
>>>
>>>
>>Yes. They do (at least, the ones I worked with do).
>>
>>
>
>Do they use actual precompiled statements or just with string concats?
>
>
Yes, they do precompile the statements.

>
>
>>No, that's not what I want...
>>What I want is an abstraction of the implementation-specific details,
>>that lets me effectively execute sql queries.
>>If it can precompile my query, I want it to precompile it. If
>>it cannot,
>>I can live with that (for a while).
>>I just don't want to *know* what it is doing.
>>
>>
>
>I accept that, but then you have to be willing to live by the rules of the spec,
>and I suspect this behaviour violates the spec.
>
>
>
It doesn't violate it. I'll agree that it probably *extends* the spec...
But I just don't think it is such a bad thing...


Dima

Well, maybe we could discuss another way of passing sets to the preparedstatement? I would be happy to have a better
wayto do it too...  
  Although I still think is a bad idea to break compatibility :-)

On Fri, 18 Jul 2003 16:39:02 -0400
Dmitry Tkach <dmitry@openratings.com> wrote:

> Darin Ohashi wrote:
>
> >The final SQL statement produced by a PS should be any valid SQL string, but a
> >PS should not have to deal with being able to put that string together from
> >arbitrary pieces of data.
> >
> I agree with you on this one - not *arbitrary* pieces of data, no.
>
> > I would be surprised if there was not a document
> >somewhere that specified what is and is not valid to pass via a "?".
> >
> >
> If there is any, I've never heard of it...
>
> >Looking at this particular example, it looks to me (of course, I have a limited
> >amount of SQL experience) that "(1,2,3,4)" contains syntax.  Using (?,?,?,?) the
> >"?" replace data and that should work.  As "(1,2,3,4)" contains syntax, I don't
> >think it is a valid thing to substitute in.
> >
> Well... "(1,2,3,4)" *does* contain syntax. I agree on this one too.
> *But*, (1,2,34)   itself is *not* just an arbitrary set of data, but a
> syntactical construct that represents a set.
> I never said that using setObject (1, "(1,2,3,4,5)") is the ideal
> solution to my problem - it is just a workaround to the drivers
> inability to pass in a set any other way.
> I would much prefer to use something like setObject (1,
> javaSetOfIntegers) if I could.
> But we don't have it, and what I have now is better than nothing at all.
>
> >
> >Well, if you allow syntax to be substituted in to the PS, then the backend can't
> >precompile because it does not have a syntatically complete statement.  What you
> >substitute in could completely change the query optimization.
> >
> >
> Sure. Even if you substitute just an int, it can very well change the
> optimal query plan
> select ... where x=1 or select ... where x = 2000 could result in two
> totally different plans.
> This is a known complication of precompiled statements.
>
> One just has to know when it is better to precompile your queries and
> when it is not.
>
> >
> >
> >>>Do other JDBC drivers support this kind of substitution?
> >>>
> >>>
> >>>
> >>Yes. They do (at least, the ones I worked with do).
> >>
> >>
> >
> >Do they use actual precompiled statements or just with string concats?
> >
> >
> Yes, they do precompile the statements.
>
> >
> >
> >>No, that's not what I want...
> >>What I want is an abstraction of the implementation-specific details,
> >>that lets me effectively execute sql queries.
> >>If it can precompile my query, I want it to precompile it. If
> >>it cannot,
> >>I can live with that (for a while).
> >>I just don't want to *know* what it is doing.
> >>
> >>
> >
> >I accept that, but then you have to be willing to live by the rules of the spec,
> >and I suspect this behaviour violates the spec.
> >
> >
> >
> It doesn't violate it. I'll agree that it probably *extends* the spec...
> But I just don't think it is such a bad thing...
>
>
> Dima
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

> > I would be surprised if there was not a document
> >somewhere that specified what is and is not valid to pass via a "?".
> >
> >
> If there is any, I've never heard of it...
>

Just as a data point, both postgresql's documentation for PREPARE

http://www.postgresql.org/docs/view.php?version=7.3&idoc=0&file=sql-prepare.html

and IBM's

http://www-3.ibm.com/cgi-bin/db2www/data/db2/udb/winos2unix/support/document.d2w
/report?fn=db2v7s0sqls0645.htm#HDRPREPH2

imply that only data can be passed in.  Its by implication because they discuss
the "data" having a "datatype".  I'm not sure this would make sense if you were
allowed to pass in strings containing SQL syntax.

Darin

>
>
>Just as a data point, both postgresql's documentation for PREPARE
>
>http://www.postgresql.org/docs/view.php?version=7.3&idoc=0&file=sql-prepare.html
>
>and IBM's
>
>http://www-3.ibm.com/cgi-bin/db2www/data/db2/udb/winos2unix/support/document.d2w
>/report?fn=db2v7s0sqls0645.htm#HDRPREPH2
>
>imply that only data can be passed in.  Its by implication because they discuss
>the "data" having a "datatype".  I'm not sure this would make sense if you were
>allowed to pass in strings containing SQL syntax.
>
>
>
That's fine.
I just don't know why you refuse to call a set, containing numbers (or
strings or whatever else) 'data'...
What about arrays by the way?
Is {1,2,3,4} not 'data' too?

There is a similar problem with arrays in the current driver (that is
just about to become worth with this patch) too -
the only way I know to send in an array to a statement like

update mytable set array_field = ? where id=?

is, again, to use setObject () with the properly constructed string
representation of the array
(there is a setArray(), but no way to construct the actual
java.sql.Array object to pass there)

It sounds like we are about to arrive to the new definition of 'data' as
"something that is supported by our jdbc driver" :-)

Dima

Dmitry Tkach wrote:
>> I would be surprised if there was not a document
>> somewhere that specified what is and is not valid to pass via a "?".
>>
>>
> If there is any, I've never heard of it...
>

It is in the JDBC spec.  For each function and JDBC type it clearly
states what is the SQL type to be sent.  It clearly indicates that each
'?' corresponds to a single value of a standard SQL type.

Things like sending a series of values (1, 2, 3, 4, 5) or x'b4' are
hacks that explore bugs in a non-JDBC compliant driver.


--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

On Fri, Jul 18, 2003 at 02:22:24PM -0400, Dmitry Tkach wrote:

> What I am concerned about is the "in" thing -
>
> select * from sometable where x in ?;
> setObject (1, "(1,2,3,4,5)");
>
> that works just fine right now, and will be irreperably broken by this
> patch...

How about:

 + create an org.postgresql.InSet class that wraps an Object[] array and java.sql.Types value
 + use setObject(N, new InSet(myArray, Types.INTEGER));

The Javadoc for PreparedStatement.setObject() says:

  Note that this method may be used to pass datatabase- specific abstract data
  types, by using a driver-specific Java type.

so presumably this is the right way to do it. setObject() would handle an
InSet by individually escaping the components of the array it wraps as if
they had been passed to setObject() and turning them into an IN-like clause.

For arrays we just need to fix setArray() so that it does the same sort of
thing as described above, using the java.sql.Array methods to get the
component objects. And maybe provide a simple implementation of Array that
wraps a Java array (users can always provide their own, anyway).

Any thoughts? I can try to sort out patches, but I'm a bit short on time
right now so no promises.

-O

    I have to agree that it would be better to provide a solution
using API calls than a solutions that requires one to send SQL syntax
into a ?.

    But why can't we kill both birds with an implementation of
java.sql.Array?  If you need to use an Array, you can use a PGArray that
wraps an Object[].  If you want to do an in clause, do "where foo in (?)"
or "where foo in ?" and then use ps.setArray() instead of setObject().
Then we can make the docs explicit on this.

    It does look like java.sql.Array is kind of overkill here, but we
can provide simple constructor that makes the common usage patterns easy,
and if you can't get a full ResultSet from your in clause in v1.0, so be
it.

Aaron

On Sat, 19 Jul 2003, Oliver Jowett wrote:
> How about:
>
>  + create an org.postgresql.InSet class that wraps an Object[] array and java.sql.Types value
>  + use setObject(N, new InSet(myArray, Types.INTEGER));
>
> The Javadoc for PreparedStatement.setObject() says:
>
>   Note that this method may be used to pass datatabase- specific abstract data
>   types, by using a driver-specific Java type.
>
> so presumably this is the right way to do it. setObject() would handle an
> InSet by individually escaping the components of the array it wraps as if
> they had been passed to setObject() and turning them into an IN-like clause.
>
> For arrays we just need to fix setArray() so that it does the same sort of
> thing as described above, using the java.sql.Array methods to get the
> component objects. And maybe provide a simple implementation of Array that
> wraps a Java array (users can always provide their own, anyway).
>
> Any thoughts? I can try to sort out patches, but I'm a bit short on time
> right now so no promises.
>
> -O
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>

On Sat, Jul 19, 2003 at 03:03:20AM -0400, Aaron Mulder wrote:
>     I have to agree that it would be better to provide a solution
> using API calls than a solutions that requires one to send SQL syntax
> into a ?.
>
>     But why can't we kill both birds with an implementation of
> java.sql.Array?  If you need to use an Array, you can use a PGArray that
> wraps an Object[].  If you want to do an in clause, do "where foo in (?)"
> or "where foo in ?" and then use ps.setArray() instead of setObject().
> Then we can make the docs explicit on this.

setArray() needs to generate "'{1,2,3}'" for an array field, but you want
"(1,2,3)" for an IN clause. Requiring the JDBC driver to parse the query to
distinguish the two cases seems awkward.

-O

To keep things simple, wouldn't it be reasonable to test the last SQL
fragment and see if it ends in " IN" (trimmed and case insensitively)
and use the parenthesis notation instead of the array one?  I don't
think we can have a PostgreSQL array as the argument of an IN clause anyway.


--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

The problem with this (and other similar suggestions in this thread -
like use PGArray etc.) is that the app will not even compile with
postgres jdbc classes.
The whole point in using jdbc interfaces is to abstract the application
from the particular driver implementation.

The way it works currently, it is entirely possible either (text
representation of arrays vary between vendors, as do ways you can use to
specify sets as parameters), but, at least,  you don't have to maintain
the whole special piece of code, that can only compile when postgres
driver is available.

Dima

Oliver Jowett wrote:

>On Fri, Jul 18, 2003 at 02:22:24PM -0400, Dmitry Tkach wrote:
>
>
>
>>What I am concerned about is the "in" thing -
>>
>>select * from sometable where x in ?;
>>setObject (1, "(1,2,3,4,5)");
>>
>>that works just fine right now, and will be irreperably broken by this
>>patch...
>>
>>
>
>How about:
>
> + create an org.postgresql.InSet class that wraps an Object[] array and java.sql.Types value
> + use setObject(N, new InSet(myArray, Types.INTEGER));
>
>The Javadoc for PreparedStatement.setObject() says:
>
>  Note that this method may be used to pass datatabase- specific abstract data
>  types, by using a driver-specific Java type.
>
>so presumably this is the right way to do it. setObject() would handle an
>InSet by individually escaping the components of the array it wraps as if
>they had been passed to setObject() and turning them into an IN-like clause.
>
>For arrays we just need to fix setArray() so that it does the same sort of
>thing as described above, using the java.sql.Array methods to get the
>component objects. And maybe provide a simple implementation of Array that
>wraps a Java array (users can always provide their own, anyway).
>
>Any thoughts? I can try to sort out patches, but I'm a bit short on time
>right now so no promises.
>
>-O
>
>

On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
> The problem with this (and other similar suggestions in this thread -
> like use PGArray etc.) is that the app will not even compile with
> postgres jdbc classes.
> The whole point in using jdbc interfaces is to abstract the application
> from the particular driver implementation.

My current approach is what Fernando suggested -- use setArray() and look
for a preceeding IN. This can work without needing any postgres specific
classes -- I'll add a simple implementation of java.sql.Array that wraps a
Java array to the driver source, but if you don't want to be dependent on
the driver you can provide your own implementation.

The interface-abstraction argument only really works up to the point where
you want to do something not defined in the interface. Usually when I'm
doing DB-specific extension stuff I have a per-DB subclass that does the
special bits; if you don't have the driver available, you don't compile that
subclass. So I don't really buy the "can't build" argument. The same
thing applies to extensions like setUseServerPrepare(), BTW.

-O

Oliver Jowett wrote:

>On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
>
>
>>The problem with this (and other similar suggestions in this thread -
>>like use PGArray etc.) is that the app will not even compile with
>>postgres jdbc classes.
>>The whole point in using jdbc interfaces is to abstract the application
>>from the particular driver implementation.
>>
>>
>
>My current approach is what Fernando suggested -- use setArray() and look
>for a preceeding IN. This can work without needing any postgres specific
>classes -- I'll add a simple implementation of java.sql.Array that wraps a
>Java array to the driver source, but if you don't want to be dependent on
>the driver you can provide your own implementation.
>
>
I don't want to provide my own iplementation either :-)
I was fairly happy with what it used to be - just call setObject () and
be done with it
Anything else does look like an overkill...

>The interface-abstraction argument only really works up to the point where
>you want to do something not defined in the interface. Usually when I'm
>doing DB-specific extension stuff I have a per-DB subclass that does the
>special bits; if you don't have the driver available, you don't compile that
>subclass. So I don't really buy the "can't build" argument. The same
>thing applies to extensions like setUseServerPrepare(), BTW.
>
Yep. That's exactly why I was arguing against having it to beging with
back in the beginning of 7.3
.. I lost that one too :-(
... but still believe it was the wrong idea.
I mean, as I said before, I do have do go an extra mile because of this
precompiled query plan stuff...
I am  writing functions in C that cache query plans and return strings,
and call them from java, and pase strings back in rows...  I just can't
afford casting my statements to org.jdbc.postgres.Something... sorry - I
can't write my apps assuming that postgres is the only db they'll ever
need :-(

Dima

(Cc: list trimmed)

On Sun, Jul 20, 2003 at 10:36:53PM -0400, Dima Tkach wrote:

> Yep. That's exactly why I was arguing against having it to beging with
> back in the beginning of 7.3
> .. I lost that one too :-(
> ... but still believe it was the wrong idea.
> I mean, as I said before, I do have do go an extra mile because of this
> precompiled query plan stuff...
> I am  writing functions in C that cache query plans and return strings,
> and call them from java, and pase strings back in rows...  I just can't
> afford casting my statements to org.jdbc.postgres.Something... sorry - I
> can't write my apps assuming that postgres is the only db they'll ever
> need :-(

Um, so you don't want to depend on the JDBC postgresql driver at all, but
don't mind having postgresql-specific behaviour elsewhere so long
as you can do it purely through the JDBC interface? That seems a bit
inconsistent.

Note that using setObject() to set IN clauses is no less postgresql-specific
than using setArray() would be .. I know of at least one DB where you really
can only set data values in a prepared statement as the prepared SQL is
compiled down to a form of bytecode for DB-side execution, there *is* no
textual substitution that goes on.

What I was suggesting was a scheme like:

  interface JDBCExtensions {
     void initStatement(PreparedStatement stmt);
     void setInClause(PreparedStatement stmt, int index, int[] values);
  }

  class PostgresqlJDBCExceptions implements JDBCExtensions {
    public void initStatement(PreparedStatement stmt) {
      ((org.postgresql.PGStatement)stmt).setUseServerPrepare(true);
    }

    public void setInClause(PreparedStatement stmt, int index, int[] values) {
      stmt.setArray(index, new org.postgresql.jdbc2.WrappedArray(values, Types.INTEGER));
    }
  }

  class FooJDBCExceptions implements JDBCExtensions {
    // implementation for FooDB
  }

  // ...

  Class datasourceClass = Class.forName(configuredDatasourceClass);
  Class extensionsClass = Class.forName(configuredExtensionsClass);
  JDBCExtensions extensions = extensionsClass.newInstance();

  // ...

  PreparedStatement stmt = connection.prepareStatement(...);
  extensions.initStatement(stmt);
  extensions.setInClause(stmt, 4, new int[] { 1,2,3,4 });

Then conditionalize compilation of implementations of JDBCExceptions on the
presence of the appropriate driver. When built, you get extension support
for all the drivers present on the build system. When running from a built
jar, only the configured extension class is loaded (and presumably the
end-user has the corresponding JDBC driver available, or they can't talk to
the DB anyway!)

What prevents you from taking this approach?

-O

On Mon, Jul 21, 2003 at 03:01:57PM +1200, Oliver Jowett wrote:

>   class PostgresqlJDBCExceptions implements JDBCExtensions {
>   class FooJDBCExceptions implements JDBCExtensions {
> Then conditionalize compilation of implementations of JDBCExceptions on the

s/Exceptions/Extensions/, of course :)

-O

Dima Tkach wrote:

> I was fairly happy with what it used to be - just call setObject () and
> be done with it

Unfortunately that is not an option as it is a security risk.

You cannot leave a driver out there which allows people to insert
potentially harmful SQL statements just to make it easier for someone to
  specify a set.

In any case, I wonder if all PreparedStatements won't be server side
only one day as the client side interface was created to fill in for the
lack of that in older backends.  Once that happens and the V3 protocol
is used (7.4+ backends) I doubt that SQL injection, and the hack to set
IN arguments, will work.

Regards to all,
Fernando

--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

>
>
>Um, so you don't want to depend on the JDBC postgresql driver at all, but
>don't mind having postgresql-specific behaviour elsewhere so long
>as you can do it purely through the JDBC interface? That seems a bit
>inconsistent.
>
>
I don't see anything "inconsistent" about it.
I am just choosing the lesser of two evils.
At least, I don't need to know what database I am going to be working
with when I compile the code.

>Note that using setObject() to set IN clauses is no less postgresql-specific
>than using setArray() would be .. I know of at least one DB where you really
>can only set data values in a prepared statement as the prepared SQL is
>compiled down to a form of bytecode for DB-side execution, there *is* no
>textual substitution that goes on.
>
Right. I do need a scheme like what you suggest below to set up
database-specific logic.
The difference is - I do not need all the possible jdbc drivers to be
installed just to compile my application.
And I do not need to know which driver a particular customer is using to
send him my application.
And I don't need to force my customers to have to request a new app from
me if they decide to switch to another database.

With your suggestion, the only way I can have the above is to pack all
the possible jdbc drivers into every  build of the app...

Dima


>
>What I was suggesting was a scheme like:
>
>  interface JDBCExtensions {
>     void initStatement(PreparedStatement stmt);
>     void setInClause(PreparedStatement stmt, int index, int[] values);
>  }
>
>  class PostgresqlJDBCExceptions implements JDBCExtensions {
>    public void initStatement(PreparedStatement stmt) {
>      ((org.postgresql.PGStatement)stmt).setUseServerPrepare(true);
>    }
>
>    public void setInClause(PreparedStatement stmt, int index, int[] values) {
>      stmt.setArray(index, new org.postgresql.jdbc2.WrappedArray(values, Types.INTEGER));
>    }
>  }
>
>  class FooJDBCExceptions implements JDBCExtensions {
>    // implementation for FooDB
>  }
>
>  // ...
>
>  Class datasourceClass = Class.forName(configuredDatasourceClass);
>  Class extensionsClass = Class.forName(configuredExtensionsClass);
>  JDBCExtensions extensions = extensionsClass.newInstance();
>
>  // ...
>
>  PreparedStatement stmt = connection.prepareStatement(...);
>  extensions.initStatement(stmt);
>  extensions.setInClause(stmt, 4, new int[] { 1,2,3,4 });
>
>Then conditionalize compilation of implementations of JDBCExceptions on the
>presence of the appropriate driver. When built, you get extension support
>for all the drivers present on the build system. When running from a built
>jar, only the configured extension class is loaded (and presumably the
>end-user has the corresponding JDBC driver available, or they can't talk to
>the DB anyway!)
>
>What prevents you from taking this approach?
>
>-O
>
>

Oliver Jowett wrote:

>On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
>
>
>>The problem with this (and other similar suggestions in this thread -
>>like use PGArray etc.) is that the app will not even compile with
>>postgres jdbc classes.
>>The whole point in using jdbc interfaces is to abstract the application
>>from the particular driver implementation.
>>
>>
>
>My current approach is what Fernando suggested -- use setArray() and look
>for a preceeding IN. This can work without needing any postgres specific
>classes -- I'll add a simple implementation of java.sql.Array that wraps a
>Java array to the driver source, but if you don't want to be dependent on
>the driver you can provide your own implementation.
>
>
Why not just allow setObject() to take Collection as an argument?
You would not need any special implementations then... and the
application would not need to waste cycles on wrapping/unwrapping those
Arrays every time...

Dima

On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:

> You can't possibly hope that JDBC driver will take care of alll of the
> security risks for you. If you don't know how to write safe code, you'll
> be doomed. If you do, then you do not need help from jdbc driver. JDBC
> driver's whole purpose is to provide an abstraction layer between a
> database and an application program.
> It has nothing to do with security whatsoever.

This is only true if all DBs use identical SQL syntax, which they don't.
Tried embedding a NUL into a query lately?

Even if it was true, it's still better to have one piece of code that does
the escaping, rather than N different ones. With escaping in the JDBC
driver, you've reduced the scope of the code you need to audit for syntax
from "all query strings and all parameters" to "the JDBC driver's
parameter-escaping code and all query strings".

-O

On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:

> Why not just allow setObject() to take Collection as an argument?

You need information on the SQL type of the contents to be able to turn them
into a DB representation correctly. We can't use the type parameter to
setObject() for this as that should reflect the whole paramater, i.e.
probably Types.OTHER in this case.

java.sql.Array has a getBaseType() that does the job.

> You would not need any special implementations then... and the
> application would not need to waste cycles on wrapping/unwrapping those
> Arrays every time...

Hah, and it's faster to wrap your array of ints in a bunch of
java.lang.Integer objects so you can put in in a Collection? :)

-O

On Mon, Jul 21, 2003 at 10:24:15AM -0400, Dmitry Tkach wrote:

> >Note that using setObject() to set IN clauses is no less
> >postgresql-specific
> >than using setArray() would be .. I know of at least one DB where you
> >really
> >can only set data values in a prepared statement as the prepared SQL is
> >compiled down to a form of bytecode for DB-side execution, there *is* no
> >textual substitution that goes on.
> >
> Right. I do need a scheme like what you suggest below to set up
> database-specific logic.
> The difference is - I do not need all the possible jdbc drivers to be
> installed just to compile my application.
> And I do not need to know which driver a particular customer is using to
> send him my application.
> And I don't need to force my customers to have to request a new app from
> me if they decide to switch to another database.
>
> With your suggestion, the only way I can have the above is to pack all
> the possible jdbc drivers into every  build of the app...

Pack the support code for all drivers in, yes. The drivers themselves you
don't need to provide, as the support classes don't get loaded unless the
particular driver it supports is present.

There's no reason you can't have a "generic" extension implementation that
works on a plain JDBC driver -- you just don't get the benefit of the
extensions. Default to that for drivers you don't recognize at runtime.

If your app doesn't work without a real implementation of the extensions,
then you're not going to be able to just plug in a new JDBC driver and have
it work *regardless of how you get at those extensions* -- you have an
application that requires more JDBC provides, end of story. If you're in
this situation but want to allow your customers to run on arbitary DBs
without your intervention -- document your Extensions interface and release
it to your customers; they can build their own implementation for whatever
DB they like.

-O

Oliver Jowett wrote:

>On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:
>
>
>
>>Why not just allow setObject() to take Collection as an argument?
>>
>>
>
>You need information on the SQL type of the contents to be able to turn them
>into a DB representation correctly. We can't use the type parameter to
>setObject() for this as that should reflect the whole paramater, i.e.
>probably Types.OTHER in this case.
>
It doesn't seem to be required anywhere - it just says "the type to be
sent to the database" in the description of that
argument. You can interpret it to be the type of the contents when
dealing with collections/sets/arrays

>
>Hah, and it's faster to wrap your array of ints in a bunch of
>java.lang.Integer objects so you can put in in a Collection? :)
>
>
>
Maybe, I had  a List of Integers to begin with?
Or, perhaps, you'll be so kind to provide a smart implementation that
will understand arrays too, not just collections :-)

Dima

On Mon, Jul 21, 2003 at 10:49:19AM -0400, Dmitry Tkach wrote:
> >
> >
> >Pack the support code for all drivers in, yes. The drivers themselves you
> >don't need to provide, as the support classes don't get loaded unless the
> >particular driver it supports is present.
> >
> >
> I need to *compile* all of that code.
> And to *compile* it, I'll need to have all of those drivers available...
> and I'll need all the possible  versions of those drivers too, because I
> don't know which version the customer will be using...
>
> This is just simply impossible to do.

So you have to pick a set of drivers to support and go with that. If you have
multiple interface versions that are mutually incompatible, build multiple
versions of your extensions independantly of each other. Yes, it's messy to
build, but not impossible and certainly no worse than any other situation
where you have to support multiple versions of a dependent library.

If you don't want to support a large number of driver versions, those
versions you don't support can be handled by the "generic" implementation I
suggested in my previous email.

I'm not sure this argument is really relevant to "what should setObject()
vs. setArray() do?" any more. Replies off-list, please.

-O

Dmitry Tkach wrote:> Oliver Jowett wrote:
>
>> On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
>>
>>
>>> The problem with this (and other similar suggestions in this thread -
>>> like use PGArray etc.) is that the app will not even compile with
>>> postgres jdbc classes.
>>> The whole point in using jdbc interfaces is to abstract the
>>> application from the particular driver implementation.
>>>
>>
>>
>> My current approach is what Fernando suggested -- use setArray() and look
>> for a preceeding IN. This can work without needing any postgres specific
>> classes -- I'll add a simple implementation of java.sql.Array that
>> wraps a
>> Java array to the driver source, but if you don't want to be dependent on
>> the driver you can provide your own implementation.
>>
>>
> Why not just allow setObject() to take Collection as an argument?
> You would not need any special implementations then... and the
> application would not need to waste cycles on wrapping/unwrapping those
> Arrays every time...
>

That is an interesting idea.  But how do we know the type of the elements that
should go in the list?  We will just get java.Objects as we go through the
Collection.


--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

See my comments below.

On Mon, 2003-07-21 at 16:24, Dmitry Tkach wrote:
> >
> >
> >Um, so you don't want to depend on the JDBC postgresql driver at all, but
> >don't mind having postgresql-specific behaviour elsewhere so long
> >as you can do it purely through the JDBC interface? That seems a bit
> >inconsistent.
> >
> >
> I don't see anything "inconsistent" about it.

But please... did you ever use a different database than Postgres ? The
way you're passing the parameters is VERY much postgres specific,
furthermore, it relies on some non-standard behavior...

> I am just choosing the lesser of two evils.
> At least, I don't need to know what database I am going to be working
> with when I compile the code.

... which problem you wouldn't have if you would use standard JDBC code.
I agree that standards suck sometime, but then if you want to use a DB
specific feature, I can't see why not make it a compile time necessity -
it might be a further benefit to remind you that your code will not work
with other databases.

>
> >Note that using setObject() to set IN clauses is no less postgresql-specific
> >than using setArray() would be .. I know of at least one DB where you really
> >can only set data values in a prepared statement as the prepared SQL is
> >compiled down to a form of bytecode for DB-side execution, there *is* no
> >textual substitution that goes on.
> >
> Right. I do need a scheme like what you suggest below to set up
> database-specific logic.
> The difference is - I do not need all the possible jdbc drivers to be
> installed just to compile my application.
> And I do not need to know which driver a particular customer is using to
> send him my application.
> And I don't need to force my customers to have to request a new app from
> me if they decide to switch to another database.

But yes you force your customers to use Postgres, or maybe to a set of
databases which accept this hack - it is not standard.

>
> With your suggestion, the only way I can have the above is to pack all
> the possible jdbc drivers into every  build of the app...

Yes, all the drivers which you are using special features from. That
makes perfect sense - those are vendor specific dependencies in your
code, the library will remind you about it all the time so you don't
forget to tell the customers about it ;-)

Cheers,
Csaba.

Oliver Jowett wrote:

>On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:
>
>
>
>>You can't possibly hope that JDBC driver will take care of alll of the
>>security risks for you. If you don't know how to write safe code, you'll
>>be doomed. If you do, then you do not need help from jdbc driver. JDBC
>>driver's whole purpose is to provide an abstraction layer between a
>>database and an application program.
>>It has nothing to do with security whatsoever.
>>
>>
>
>This is only true if all DBs use identical SQL syntax, which they don't.
>Tried embedding a NUL into a query lately?
>
If you use standard SQL, and standard  compliant database, you should be ok.
If you use certain db-specific extensions, you'll still benefit from
JDBC, abstracting *most* of your sql for you.

My point was that it has nothing to do with security anyway. :-)
I was not planning to start discussing how much abstraction it provides.
I agree, that it could be better.

>Even if it was true, it's still better to have one piece of code that does
>the escaping, rather than N different ones. With escaping in the JDBC
>driver, you've reduced the scope of the code you need to audit for syntax
>from "all query strings and all parameters" to "the JDBC driver's
>parameter-escaping code and all query strings".
>
>
>

Sure. And that's good.
That's precisely the point - if you guys start taking functionality
away, so that I am not longer able to do things with it that I used to
be able to do, then I will not be able to benefit from it as much as I
used to - I'll have to switch from PreparedStatements to Statements and
do all that escaping/parsing on my own.
That's exactly what I am trying to avoid

Dima

On Tue, 22 Jul 2003 02:30:02 +1200 Oliver Jowett <oliver@opencloud.com> wrote:
> On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:

> > You can't possibly hope that JDBC driver will take care of alll of the
> > security risks for you. If you don't know how to write safe code,
> you'll
> > be doomed. If you do, then you do not need help from jdbc driver. JDBC
> > driver's whole purpose is to provide an abstraction layer between a
> > database and an application program.
> > It has nothing to do with security whatsoever.
...
> Even if it was true, it's still better to have one piece of code that
> does
> the escaping, rather than N different ones. With escaping in the JDBC
> driver, you've reduced the scope of the code you need to audit for syntax
> from "all query strings and all parameters" to "the JDBC driver's
> parameter-escaping code and all query strings".

eewwww.

in a multi-tier architecture where the code that actually talks to
the database is isolated from the GUI, this is a totally unreasonable
expectation -- you really need to audit fields in the GUI, not somewhere
way back in the code.

even if PostgreSQL's jdbc driver somehow had wonderful code to handle
security problems, sensible DB independent code will _still_ need to audit
in the GUI because there is no reasonable expectation that all jdbc drivers
that might be used will have similar code.

i understand your desire for a single point of control, but moving this
into the jdbc driver is simply wrong. there are simply better ways;
java/swing/javabeans are powerful tools.

richard
--
Richard Welty                                         rwelty@averillpark.net
Averill Park Networking                                         518-573-7592
    Java, PHP, PostgreSQL, Unix, Linux, IP Network Engineering, Security

On Mon, Jul 21, 2003 at 10:43:28AM -0400, Dmitry Tkach wrote:
> Oliver Jowett wrote:
>
> >On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:
> >
> >
> >
> >>Why not just allow setObject() to take Collection as an argument?
> >>
> >>
> >
> >You need information on the SQL type of the contents to be able to turn
> >them
> >into a DB representation correctly. We can't use the type parameter to
> >setObject() for this as that should reflect the whole paramater, i.e.
> >probably Types.OTHER in this case.
> >
> It doesn't seem to be required anywhere - it just says "the type to be
> sent to the database" in the description of that
> argument. You can interpret it to be the type of the contents when
> dealing with collections/sets/arrays

java.sql.Types has pretty explicit mappings between type values and SQL
types. A collection of integers is definitely not a SQL INTEGER.

Types.OTHER says:

  The constant in the Java programming language that indicates that the SQL
  type is database-specific and gets mapped to a Java object that can be
  accessed via the methods getObject and setObject.

which fits this use of Collection.

Responding to comments elsewhere .. you do need the component type to behave
correctly. For example, these produce different results:

 setObject(1, new Date(...), Types.STRING)
 setObject(1, new Date(...), Types.TIMESTAMP)

-O

>
>
>Pack the support code for all drivers in, yes. The drivers themselves you
>don't need to provide, as the support classes don't get loaded unless the
>particular driver it supports is present.
>
>
I need to *compile* all of that code.
And to *compile* it, I'll need to have all of those drivers available...
and I'll need all the possible  versions of those drivers too, because I
don't know which version the customer will be using...

This is just simply impossible to do.

Dima

> Sure. And that's good.
> That's precisely the point - if you guys start taking functionality
> away, so that I am not longer able to do things with it that I used to
> be able to do, then I will not be able to benefit from it as much as I
> used to - I'll have to switch from PreparedStatements to Statements and
> do all that escaping/parsing on my own.
> That's exactly what I am trying to avoid

Dima, you are talking about "functionality" which is not documented,
discovered by yourself as working. This is similar with directly using
the com.sun.* classes - might work now, but will be a major PITA when
sun decides to change their internal API. You should have expected this
outcome... and it's certainly not an argument against fixing the driver
to be standards compliant...

Cheers,
Csaba.

>
> Dima
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly
>

On Mon, Jul 21, 2003 at 11:01:56AM -0400, Richard Welty wrote:
> On Tue, 22 Jul 2003 02:30:02 +1200 Oliver Jowett <oliver@opencloud.com> wrote:
> > On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:
>
> > > You can't possibly hope that JDBC driver will take care of alll of the
> > > security risks for you. If you don't know how to write safe code,
> > you'll
> > > be doomed. If you do, then you do not need help from jdbc driver. JDBC
> > > driver's whole purpose is to provide an abstraction layer between a
> > > database and an application program.
> > > It has nothing to do with security whatsoever.
> ...
> > Even if it was true, it's still better to have one piece of code that
> > does
> > the escaping, rather than N different ones. With escaping in the JDBC
> > driver, you've reduced the scope of the code you need to audit for syntax
> > from "all query strings and all parameters" to "the JDBC driver's
> > parameter-escaping code and all query strings".
>
> eewwww.
>
> in a multi-tier architecture where the code that actually talks to
> the database is isolated from the GUI, this is a totally unreasonable
> expectation -- you really need to audit fields in the GUI, not somewhere
> way back in the code.

I was very careful to say "audit for syntax". You certainly want to make
sure you have input validation earlier on, too! -- but you don't need to
worry about, for example, correctly escaping strings that could validly have
a bare "'" in them before you pass them to the DB.

-O

On Mon, Jul 21, 2003 at 10:39:11AM -0400, Dmitry Tkach wrote:
> Oliver Jowett wrote:
>
> >Even if it was true, it's still better to have one piece of code that does
> >the escaping, rather than N different ones. With escaping in the JDBC
> >driver, you've reduced the scope of the code you need to audit for syntax
> >from "all query strings and all parameters" to "the JDBC driver's
> >parameter-escaping code and all query strings".
> >
> >
> >
>
> Sure. And that's good.
> That's precisely the point - if you guys start taking functionality
> away, so that I am not longer able to do things with it that I used to
> be able to do, then I will not be able to benefit from it as much as I
> used to - I'll have to switch from PreparedStatements to Statements and
> do all that escaping/parsing on my own.
> That's exactly what I am trying to avoid

The functionality we are "taking away" allows you to bypass the JDBC
driver's parameter escaping. You can't have it both ways.

-O

> eewwww.
>
> in a multi-tier architecture where the code that actually talks to
> the database is isolated from the GUI, this is a totally unreasonable
> expectation -- you really need to audit fields in the GUI, not somewhere
> way back in the code.

Which it should be done indeed, but you also can't expect that a
middle-ware can correctly escape an input string against injection
attacks, as it can't know all the JDBC drivers it will talk to... this
is the job of the JDBC driver, the app talking to it should not even
attempt this.

>
> even if PostgreSQL's jdbc driver somehow had wonderful code to handle
> security problems, sensible DB independent code will _still_ need to audit
> in the GUI because there is no reasonable expectation that all jdbc drivers
> that might be used will have similar code.
>
This is not just about security problems fixed, it's about deterministic
behavior. If you have a non standard driver, you will not know how it
behaves unless you try out every possible input, and even less how it
will behave tomorrow. That's why is so important to have standards
compliance.

> i understand your desire for a single point of control, but moving this
> into the jdbc driver is simply wrong. there are simply better ways;
> java/swing/javabeans are powerful tools.
>

Yes, the application has to validate it's data, but this has nothing to
do with the JDBC validation. There could be perfectly valid data from
the application point of view which can result in unexpected results if
the driver doesn't do it's validation job correctly.

Cheers,
Csaba.

> richard
> --
> Richard Welty                                         rwelty@averillpark.net
> Averill Park Networking                                         518-573-7592
>     Java, PHP, PostgreSQL, Unix, Linux, IP Network Engineering, Security
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

Oliver Jowett wrote:

>On Mon, Jul 21, 2003 at 10:39:11AM -0400, Dmitry Tkach wrote:
>
>
>>Oliver Jowett wrote:
>>
>>
>>
>>>Even if it was true, it's still better to have one piece of code that does
>>>the escaping, rather than N different ones. With escaping in the JDBC
>>>driver, you've reduced the scope of the code you need to audit for syntax
>>>
>>>
>>>from "all query strings and all parameters" to "the JDBC driver's
>>
>>
>>>parameter-escaping code and all query strings".
>>>
>>>
>>>
>>>
>>>
>>Sure. And that's good.
>>That's precisely the point - if you guys start taking functionality
>>away, so that I am not longer able to do things with it that I used to
>>be able to do, then I will not be able to benefit from it as much as I
>>used to - I'll have to switch from PreparedStatements to Statements and
>>do all that escaping/parsing on my own.
>>That's exactly what I am trying to avoid
>>
>>
>
>The functionality we are "taking away" allows you to bypass the JDBC
>driver's parameter escaping. You can't have it both ways.
>
>
Sure, I can :-)
I *do* "have it both ways" right now :-) - in situations when I need
drivers escaping, I use it, in situations when I don't I don't.
I have both the functionality, and the flexibility not to use it when I
don't need it.

Dima

>
>

Oliver Jowett wrote:> On Mon, Jul 21, 2003 at 10:43:28AM -0400, Dmitry Tkach wrote:
>
>>Oliver Jowett wrote:
>>
>>
>>>On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:
>>>
>>>
>>>
>>>
>>>>Why not just allow setObject() to take Collection as an argument?
>>>>
>>>>
>>>
>>>You need information on the SQL type of the contents to be able to turn
>>>them
>>>into a DB representation correctly. We can't use the type parameter to
>>>setObject() for this as that should reflect the whole paramater, i.e.
>>>probably Types.OTHER in this case.
>>>
>>
>>It doesn't seem to be required anywhere - it just says "the type to be
>>sent to the database" in the description of that
>>argument. You can interpret it to be the type of the contents when
>>dealing with collections/sets/arrays
>
>
> java.sql.Types has pretty explicit mappings between type values and SQL
> types. A collection of integers is definitely not a SQL INTEGER.
>
> Types.OTHER says:
>
>   The constant in the Java programming language that indicates that the SQL
>   type is database-specific and gets mapped to a Java object that can be
>   accessed via the methods getObject and setObject.
>
> which fits this use of Collection.
>
> Responding to comments elsewhere .. you do need the component type to behave
> correctly. For example, these produce different results:
>
>  setObject(1, new Date(...), Types.STRING)
>  setObject(1, new Date(...), Types.TIMESTAMP)
>

Oliver,

I think Dima is arguing that Collection could be treated as an special case
where it indicates a list of something instead of a single something.

So, we would iterate through this Collection using its iterator and, for each
Object obtained, act like a setObject has been used with that Object and the
JAVA TYPE as an argument.

The Types.OTHER is used for database specific SQL types or for Dynamic Data
Access support.  As the Collection class is not a data type there is no chance
of confusion.

It seems that Dima's idea can indeed work.

Regards,
Fernando



--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Well, looks like Dima's problem can be solved after all while still
being standards compliant in the standard cases :-)
Still, the individual data types should be validated and correctly
escaped. If it counts at all, I would vote with this approach.
Just one more question Dima, how will you at runtime that the current
driver supports this functionality ? (for that matter, how do you know
now ?)

Cheers,
Csaba.


> Oliver,
>
> I think Dima is arguing that Collection could be treated as an special case
> where it indicates a list of something instead of a single something.
>
> So, we would iterate through this Collection using its iterator and, for each
> Object obtained, act like a setObject has been used with that Object and the
> JAVA TYPE as an argument.
>
> The Types.OTHER is used for database specific SQL types or for Dynamic Data
> Access support.  As the Collection class is not a data type there is no chance
> of confusion.
>
> It seems that Dima's idea can indeed work.
>
> Regards,
> Fernando

On Mon, Jul 21, 2003 at 11:26:11AM -0400, Fernando Nasser wrote:

> I think Dima is arguing that Collection could be treated as an special case
> where it indicates a list of something instead of a single something.
>
> So, we would iterate through this Collection using its iterator and, for
> each Object obtained, act like a setObject has been used with that Object
> and the JAVA TYPE as an argument.
>
> The Types.OTHER is used for database specific SQL types or for Dynamic Data
> Access support.  As the Collection class is not a data type there is no
> chance of confusion.

Ya, I understand. My main objection is that setObject(n, object,
Types.INTEGER), in all other cases, means "interpret object as an integer,
or fail if it can't be cast in that way".

-O

On Tue, Jul 22, 2003 at 03:47:49AM +1200, Oliver Jowett wrote:
> On Mon, Jul 21, 2003 at 11:26:11AM -0400, Fernando Nasser wrote:
>
> > I think Dima is arguing that Collection could be treated as an special case
> > where it indicates a list of something instead of a single something.
> >
> > So, we would iterate through this Collection using its iterator and, for
> > each Object obtained, act like a setObject has been used with that Object
> > and the JAVA TYPE as an argument.
> >
> > The Types.OTHER is used for database specific SQL types or for Dynamic Data
> > Access support.  As the Collection class is not a data type there is no
> > chance of confusion.
>
> Ya, I understand. My main objection is that setObject(n, object,
> Types.INTEGER), in all other cases, means "interpret object as an integer,
> or fail if it can't be cast in that way".

Also.. what would we do with this object?

public class AnnoyingObject implements java.util.Collection, java.sql.Array {
  // ...
}

then setObject(n, new AnnoyingObject(), Types.ARRAY);

Is that an Array, or an IN clause of Arrays? :)

(Array is the obvious candidate for also being a Collection, but potentially
you could do it with other types too)

-O

>
>
>Also.. what would we do with this object?
>
>public class AnnoyingObject implements java.util.Collection, java.sql.Array {
>  // ...
>}
>
>then setObject(n, new AnnoyingObject(), Types.ARRAY);
>
>Is that an Array, or an IN clause of Arrays? :)
>
>(Array is the obvious candidate for also being a Collection, but potentially
>you could do it with other types too)
>
>
>
java.sql.Array is an ARRAY.
I don't think there is any ambiguity here, as it is the only reason I
can imagine for somebody to implement a sql.Array is to pass an ARRAY
into a PreparedStatement.


If they wanted a set of arrays, they would have to pass in a Collection,
containing java.sql.Arrays as elements...

Dima

Oliver Jowett wrote:> On Tue, Jul 22, 2003 at 03:47:49AM +1200, Oliver Jowett wrote:
>
>>On Mon, Jul 21, 2003 at 11:26:11AM -0400, Fernando Nasser wrote:
>>
>>
>>>I think Dima is arguing that Collection could be treated as an special case
>>>where it indicates a list of something instead of a single something.
>>>
>>>So, we would iterate through this Collection using its iterator and, for
>>>each Object obtained, act like a setObject has been used with that Object
>>>and the JAVA TYPE as an argument.
>>>
>>>The Types.OTHER is used for database specific SQL types or for Dynamic Data
>>>Access support.  As the Collection class is not a data type there is no
>>>chance of confusion.
>>
>>Ya, I understand. My main objection is that setObject(n, object,
>>Types.INTEGER), in all other cases, means "interpret object as an integer,
>>or fail if it can't be cast in that way".
>
>
> Also.. what would we do with this object?
>
> public class AnnoyingObject implements java.util.Collection, java.sql.Array {
>   // ...
> }
>
> then setObject(n, new AnnoyingObject(), Types.ARRAY);
>
> Is that an Array, or an IN clause of Arrays? :)
>
> (Array is the obvious candidate for also being a Collection, but potentially
> you could do it with other types too)
>

I am not sure if this is an useful or usual Java class at all, but if you want
to pass a list of this AnnoyingObject you can always create a Collection of such
objects (like an ArrayList).

With setObject, if the specified type is Array and the passed type is an Array
of some sort we have to honor that.  So, in the obscure case where someone wants
to set a list of Arrays they will have to add the Arrays to a Collection (that
is not an Array itself).

But you are right, to my pseudo code we do have to check the Array to ARRAY case
before assuming that the list is what the programmer wants.

Also, we may limit this behavior with Collections to the IN clause only.  Where
else could we need lists to replace the '?' ?

--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

>
> Also, we may limit this behavior with Collections to the IN clause
> only.  Where else could we need lists to replace the '?' ?
>
Arrays would be nice :-) (so that the user would not have to implement
his own java.sql.Array, and wrap his collections into it to be able to
set array parameters).
Informix supports that, for example (but it has to be ArrayList for some
reason - not just any Collection), oracle does too IIRC...


Dima

On Mon, Jul 21, 2003 at 12:33:40PM -0400, Dmitry Tkach wrote:
> >
> >
> >Also.. what would we do with this object?
> >
> >public class AnnoyingObject implements java.util.Collection,
> >java.sql.Array {
> > // ...
> >}
> >
> >then setObject(n, new AnnoyingObject(), Types.ARRAY);
> >
> >Is that an Array, or an IN clause of Arrays? :)
> >
> >(Array is the obvious candidate for also being a Collection, but
> >potentially
> >you could do it with other types too)
> >
> >
> >
> java.sql.Array is an ARRAY.
> I don't think there is any ambiguity here, as it is the only reason I
> can imagine for somebody to implement a sql.Array is to pass an ARRAY
> into a PreparedStatement.
>
>
> If they wanted a set of arrays, they would have to pass in a Collection,
> containing java.sql.Arrays as elements...

Reread the class declaration, AnnoyingObject is both an Array and a
Collection.

-O

On Mon, Jul 21, 2003 at 01:51:41PM -0400, Fernando Nasser wrote:
> Oliver Jowett wrote:> On Tue, Jul 22, 2003 at 03:47:49AM +1200, Oliver
> Jowett wrote:

> >Also.. what would we do with this object?
> >
> >public class AnnoyingObject implements java.util.Collection,
> >java.sql.Array {
> >  // ...
> >}
> >
> >then setObject(n, new AnnoyingObject(), Types.ARRAY);
> >
> >Is that an Array, or an IN clause of Arrays? :)
> >
> >(Array is the obvious candidate for also being a Collection, but
> >potentially
> >you could do it with other types too)
> >
>
> I am not sure if this is an useful or usual Java class at all, but if you
> want to pass a list of this AnnoyingObject you can always create a
> Collection of such objects (like an ArrayList).

Um, no, that's not my point.

Consider this (more realistic) example:

  public class MutableArray extends ArrayList implements java.sql.Array {
     // implementation of java.sql.Array in terms of ArrayList methods
  }

  MutableArray myarray = new MutableArray();
  myarray.add("abcd");
  myarray.add("efgh");

  stmt.setArray(1, myarray);  // This sets param 1 as an array of strings
  stmt.setObject(1, myarray); // but what does this do?
  stmt.setObject(1, myarray, Types.ARRAY);   // and this?
  stmt.setObject(1, myarray, Types.VARCHAR); // and this?

Yes, you can avoid this by using composition not inheritance. But this is a
very fragile and nonobvious way for setObject to behave. Adding an extra,
commonly used, non-JDBC, non-Postgresql interface to an existing class
shouldn't cause large changes to how the driver treats it!

> With setObject, if the specified type is Array and the passed type is an
> Array of some sort we have to honor that.  So, in the obscure case where
> someone wants to set a list of Arrays they will have to add the Arrays to a
> Collection (that is not an Array itself).

So the extension becomes "If you pass a Collection.. unless the Collection
is also an Array and you specify Types.ARRAY.. or it's also a Blob and you
specify Types.BLOB.. or ...". This is getting messy.

> Also, we may limit this behavior with Collections to the IN clause only.
> Where else could we need lists to replace the '?' ?

Ideally, we want an interface where the API user can provide the JDBC driver
with enough information to say "this is definitely an IN clause parameter"
without having to inspect the query. Otherwise you have the situation where
the same setObject() call expands differently depending on parameter context,
which is pretty nasty.

-O

Peter Kovacs wrote:> That's, then, even simpler that I originally thought. The
only thing to
> be done is to make using "real" prepared statements the default
> behaviour of the PreparedStatement instances, is not it?
>

I agree with you.  I would make it the default to 7.4 backends at least...
But it is up to Barry and Dave to decide.

We will probably have to keep the client side emulation around for a while.
Even if all the supported backend versions already have server side prepared
statements that implementation is recent enogh for us to have an alternative one
around, even if for comparison purposes.  Besides, someone was claiming to have,
in some situations, faster results with the client side emulation.  I believe
the 7.4 backend and the V3 protocol will solve that but we must be sure that is
so before removing this code (7.6 version will only have to support 7.3 and 7.4
backends so it is a possible end of the line for it).

As we have to keep the old code around, even if not the default, we will have to
fix it so nobody gets hurt.

--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

>
>
>Dima, you are talking about "functionality" which is not documented,
>discovered by yourself as working. This is similar with directly using
>the com.sun.* classes - might work now, but will be a major PITA when
>sun decides to change their internal API.
>
I know. I understood the risk when I took that approach. And I am not
complaining.
If you had good reasons to take that functionality away, I would not
mind at all.
In this case though, it just looks like you are going to take it out
"just because", and I think, it is a bad idea. That's all.

>You should have expected this
>outcome... and it's certainly not an argument against fixing the driver
>to be standards compliant...
>
>
>
What standard are you talking about?
Where does it say anything about the driver being *required* to provide
no way for people to specify a set as a parameter?
Or where does it even mention anything about the requirement to quote
every single parameter the user sends in?

If you can show me a standard, that *requires* either of the above (not
just is silent about it), I'll have to give up, and agree that I am wrong.

Dima

Actually, here is what javadoc comments say in the jdk's
PreparedStatement class:


/**
 * An object that represents a precompiled SQL statement.
 * <P>A SQL statement is precompiled and stored in a
 * <code>PreparedStatement</code> object. This object can then be used to
 * efficiently execute this statement multiple times.
 *
 * <P><B>Note:</B> The setter methods (<code>setShort</code>,
<code>setString</code>,
 * and so on) for setting IN parameter values
 * must specify types that are compatible with the defined SQL type of
 * the input parameter. For instance, if the IN parameter has SQL type
 * <code>INTEGER</code>, then the method <code>setInt</code> should be used.
 *
 * <p>If arbitrary parameter type conversions are required, the method
 * <code>setObject</code> should be used with a target SQL type.
 * <P>
*/

Two things that stricke me here:

- no mention of "security" stuff whatsoever. The sole purpose of
 PreparedStatement according to this is to "efficiently execute this
statement multipe times",
not "to prevent slq injection attacks" or anything like that;

- it is *explicitly* stated that setObject () should be used for
"arbitrary type conversions";


Dima







Peter Kovacs wrote:

> I think that the simplest thing would be to have an option in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements. I am not
> sure why this feature (multiple statments in one query) is there
> anyway. "Reduce network roundtrips" is the usual reply, but, then,
> what is the purpose of stored procedures (functions in PostgreSQL)?
>
> In my perception, JDBC is meant to be a client side extension of the
> server - bridge for Java clients to use the server (which in our case
> is the right and honorable PostgreSQL). Prepared statements is a
> mechanism to indicate the server that more query of this kind is
> likely to come so the plan of the query should be kept ready to be
> used again. That is what meant by PreparedStatement in the JDBC
> driver. I find the concept of "client side prepared statements" pretty
> weird.
>
> From this perspective, the whole wrestling with "drop table..." and
> similar risks seem farily vain to me. At least, the driver is not the
> place to solve this kind of security problems which basically exist
> due to the wya the server behaves. Instead, the question should be
> asked: is the behaviour of the server optimal?. Do we need this
> feature (processing multiple, semi-colon separated statements)? Should
> not this feature be eventually optional?
>
> Cheers,
> Peter
>
> Fernando Nasser wrote:
>
>> Dima Tkach wrote:
>>
>>> I was fairly happy with what it used to be - just call setObject ()
>>> and be done with it
>>
>>
>>
>> Unfortunately that is not an option as it is a security risk.
>>
>> You cannot leave a driver out there which allows people to insert
>> potentially harmful SQL statements just to make it easier for someone
>> to  specify a set.
>>
>> In any case, I wonder if all PreparedStatements won't be server side
>> only one day as the client side interface was created to fill in for
>> the lack of that in older backends.  Once that happens and the V3
>> protocol is used (7.4+ backends) I doubt that SQL injection, and the
>> hack to set IN arguments, will work.
>>
>> Regards to all,
>> Fernando
>>

Fernando Nasser wrote:

> Dima Tkach wrote:
>
>> I was fairly happy with what it used to be - just call setObject ()
>> and be done with it
>
>
> Unfortunately that is not an option as it is a security risk.
>
> You cannot leave a driver out there which allows people to insert
> potentially harmful SQL statements just to make it easier for someone
> to  specify a set.

The driver allows people to "insert potentially harmful SQL" *anyway* -
even if every "problem" of this kind with PreparedStatement is fixed,
the *driver* still allows you to send in anything you want by simply
using Statement instead...

You can't possibly hope that JDBC driver will take care of alll of the
security risks for you. If you don't know how to write safe code, you'll
be doomed. If you do, then you do not need help from jdbc driver. JDBC
driver's whole purpose is to provide an abstraction layer between a
database and an application program.
It has nothing to do with security whatsoever.

Dima

Peter Kovacs wrote:> I think that the simplest thing would be to have an option
in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements. I am not sure
> why this feature (multiple statments in one query) is there anyway.
> "Reduce network roundtrips" is the usual reply, but, then, what is the
> purpose of stored procedures (functions in PostgreSQL)?
>

I don't think the backend maintainers will like that.  And btw we don't need
this at all.

1) There is no risk of SQL injection with "real" (or "server side") prepared
statements.

2) With the proper implementation of the client side prepared statements
emulation the injected statements will not go through, because:

    a) all non-string results are properly quoted and generated from primary types.

    b) String type contents are quoted and have its quotes escaped.

> In my perception, JDBC is meant to be a client side extension of the
> server - bridge for Java clients to use the server (which in our case is
> the right and honorable PostgreSQL). Prepared statements is a mechanism
> to indicate the server that more query of this kind is likely to come so
> the plan of the query should be kept ready to be used again. That is
> what meant by PreparedStatement in the JDBC driver. I find the concept
> of "client side prepared statements" pretty weird.
>

It was added before the server had prepared statements so the applications could
still use PreparedStatements and the getXXX and setXXX methods instead of
recreating strings with queries all the time.  Some of the applications run with
other databases as well.

>  From this perspective, the whole wrestling with "drop table..." and
> similar risks seem farily vain to me. At least, the driver is not the
> place to solve this kind of security problems which basically exist due
> to the wya the server behaves. Instead, the question should be asked: is
> the behaviour of the server optimal?. Do we need this feature
> (processing multiple, semi-colon separated statements)? Should not this
> feature be eventually optional?
>

No SQL injection is possible with the backend implementation of prepared
statements (with the V3 protocol, at least).



--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

>
>
>>>
>> Why not just allow setObject() to take Collection as an argument?
>> You would not need any special implementations then... and the
>> application would not need to waste cycles on wrapping/unwrapping
>> those Arrays every time...
>>
>
> That is an interesting idea.  But how do we know the type of the
> elements that should go in the list?  We will just get java.Objects as
> we go through the Collection.
>
>
You can require the 'type' argument to setObject() to specify the target
type.

Besides, knowing the exact type doesn't really matter much, because you
aare going to be quoting everything anyways :-)

Dima

Dmitry Tkach wrote:> Fernando Nasser wrote:
>
>> Dmitry Tkach wrote:
>>
>>>
>>> Two things that stricke me here:
>>>
>>> - no mention of "security" stuff whatsoever. The sole purpose of
>>> PreparedStatement according to this is to "efficiently execute this
>>> statement multipe times",
>>> not "to prevent slq injection attacks" or anything like that;
>>>
>>
>> Because in "real" prepared statements there is no such risk.  The risk
>> is the artifact of a bug in our client side simulation of prepared
>> statements (not real prepared statements as per definition).
>
>
> My point was that  the risk exists, when you do *not* use
> PreparedStatements, right?
> If the purpose of PreparedStatement was to eliminate that risk, it would
> have been mentioned. But it is not. Because PreparedStatement has
> nothing to do with the security. It is all about efficiency.
>

I don't agree with your reading.  It is not mentioned because it is
intrinsically safe.


>
>>> - it is *explicitly* stated that setObject () should be used for
>>> "arbitrary type conversions";
>>>
>>
>> Not that arbitrary.  There is a table specifying for each java type
>> that the passed object is member of the proper JDBC type for the
>> converted result.  Which must be the type of the field you are trying
>> to specify the value for.
>>
>> So it is not that arbitrary.
>
>
> It doesn't say *how* arbitrary. It just says "arbitrary". :-)
> If you could only pass objects of types in that table, you would not
> need setObject () - just setString(), setInt() etc... would suffice.
> The whole idea of setObject () is to be able to pass in an argument for
> each there is no specialized setter function.
>

No, you are misreading the spec.  The catch all is there, java class, which
result in JAVA_OBJECT.

The setObject method is intended to allow conversion between types, which is not
possible with the type specific setXXX that always convert to the default type
for that method.

--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Dmitry Tkach wrote:
>
> Two things that stricke me here:
>
> - no mention of "security" stuff whatsoever. The sole purpose of
> PreparedStatement according to this is to "efficiently execute this
> statement multipe times",
> not "to prevent slq injection attacks" or anything like that;
>

Because in "real" prepared statements there is no such risk.  The risk is the
artifact of a bug in our client side simulation of prepared statements (not real
prepared statements as per definition).


> - it is *explicitly* stated that setObject () should be used for
> "arbitrary type conversions";
>

Not that arbitrary.  There is a table specifying for each java type that the
passed object is member of the proper JDBC type for the converted result.  Which
must be the type of the field you are trying to specify the value for.

So it is not that arbitrary.

--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

>
>
>But please... did you ever use a different database than Postgres ? The
>way you're passing the parameters is VERY much postgres specific,
>furthermore, it relies on some non-standard behavior...
>
>
Yes. That's what I meant below - 'the lesser of two evils'

>
>
>>I am just choosing the lesser of two evils.
>>At least, I don't need to know what database I am going to be working
>>with when I compile the code.
>>
>>
>
>... which problem you wouldn't have if you would use standard JDBC code.
>
True. I would not have this problem then. I would have another problem -
just no way to pass a set or array in whatsoever.... :-)
I know a better way to not have problems - if I just didn't do anything
at all, I would not have any. :-)

>I agree that standards suck sometime, but then if you want to use a DB
>specific feature, I can't see why not make it a compile time necessity -
>it might be a further benefit to remind you that your code will not work
>with other databases.
>
I explained the reason for not making it a 'compile time necessity'
earlier - there is no way whatsoever I can have all possible jdbc
drivers in the world available to me at compile time.

>But yes you force your customers to use Postgres, or maybe to a set of
>databases which accept this hack - it is not standard.
>
>
I don't force them to use Postgres.
I do have the driver-specific logic that uses the correct way to pass in
the parameters depending on the driver.
The difference is that this is *run-time* logic, not *compilation-time*.

>
>
>>With your suggestion, the only way I can have the above is to pack all
>>the possible jdbc drivers into every  build of the app...
>>
>>
>
>Yes, all the drivers which you are using special features from. That
>makes perfect sense - those are vendor specific dependencies in your
>code, the library will remind you about it all the time so you don't
>forget to tell the customers about it ;-)
>
>
It doesn't matter really if it makes sense or not (to me, it doesn't
anyway).
It is just not possible to do. I simply do not have all the drivers in
the world available to me.
Even if I did, I can't begin to imagine what my customers would tell
when they finish laughing after receiving an app from me, with all the
possible drivers built in :-)

As for not forgetting to tell the customers about it - that's the whole
idea - I am *not* telling the customers about it, they don't want to
know that, and don't need to care.

Dima

I think that the simplest thing would be to have an option in the
backend to disable processing of multiple statements in one query --
i.e. disallow the use of ';' as a separator of statements. I am not sure
why this feature (multiple statments in one query) is there anyway.
"Reduce network roundtrips" is the usual reply, but, then, what is the
purpose of stored procedures (functions in PostgreSQL)?

In my perception, JDBC is meant to be a client side extension of the
server - bridge for Java clients to use the server (which in our case is
the right and honorable PostgreSQL). Prepared statements is a mechanism
to indicate the server that more query of this kind is likely to come so
the plan of the query should be kept ready to be used again. That is
what meant by PreparedStatement in the JDBC driver. I find the concept
of "client side prepared statements" pretty weird.

 From this perspective, the whole wrestling with "drop table..." and
similar risks seem farily vain to me. At least, the driver is not the
place to solve this kind of security problems which basically exist due
to the wya the server behaves. Instead, the question should be asked: is
the behaviour of the server optimal?. Do we need this feature
(processing multiple, semi-colon separated statements)? Should not this
feature be eventually optional?

Cheers,
Peter

Fernando Nasser wrote:

> Dima Tkach wrote:
>
>> I was fairly happy with what it used to be - just call setObject ()
>> and be done with it
>
>
> Unfortunately that is not an option as it is a security risk.
>
> You cannot leave a driver out there which allows people to insert
> potentially harmful SQL statements just to make it easier for someone
> to  specify a set.
>
> In any case, I wonder if all PreparedStatements won't be server side
> only one day as the client side interface was created to fill in for
> the lack of that in older backends.  Once that happens and the V3
> protocol is used (7.4+ backends) I doubt that SQL injection, and the
> hack to set IN arguments, will work.
>
> Regards to all,
> Fernando
>

That's, then, even simpler that I originally thought. The only thing to
be done is to make using "real" prepared statements the default
behaviour of the PreparedStatement instances, is not it?

Fernando Nasser wrote:

> Peter Kovacs wrote:> I think that the simplest thing would be to have
> an option in the
>
>> backend to disable processing of multiple statements in one query --
>> i.e. disallow the use of ';' as a separator of statements. I am not
>> sure why this feature (multiple statments in one query) is there
>> anyway. "Reduce network roundtrips" is the usual reply, but, then,
>> what is the purpose of stored procedures (functions in PostgreSQL)?
>>
>
> I don't think the backend maintainers will like that.  And btw we
> don't need this at all.
>
> 1) There is no risk of SQL injection with "real" (or "server side")
> prepared statements.
>
> 2) With the proper implementation of the client side prepared
> statements emulation the injected statements will not go through,
> because:
>
>    a) all non-string results are properly quoted and generated from
> primary types.
>
>    b) String type contents are quoted and have its quotes escaped.
>
>> In my perception, JDBC is meant to be a client side extension of the
>> server - bridge for Java clients to use the server (which in our case
>> is the right and honorable PostgreSQL). Prepared statements is a
>> mechanism to indicate the server that more query of this kind is
>> likely to come so the plan of the query should be kept ready to be
>> used again. That is what meant by PreparedStatement in the JDBC
>> driver. I find the concept of "client side prepared statements"
>> pretty weird.
>>
>
> It was added before the server had prepared statements so the
> applications could still use PreparedStatements and the getXXX and
> setXXX methods instead of recreating strings with queries all the
> time.  Some of the applications run with other databases as well.
>
>>  From this perspective, the whole wrestling with "drop table..." and
>> similar risks seem farily vain to me. At least, the driver is not the
>> place to solve this kind of security problems which basically exist
>> due to the wya the server behaves. Instead, the question should be
>> asked: is the behaviour of the server optimal?. Do we need this
>> feature (processing multiple, semi-colon separated statements)?
>> Should not this feature be eventually optional?
>>
>
> No SQL injection is possible with the backend implementation of
> prepared statements (with the V3 protocol, at least).
>
>
>

Fernando Nasser wrote:

> Dmitry Tkach wrote:
>
>>
>> Two things that stricke me here:
>>
>> - no mention of "security" stuff whatsoever. The sole purpose of
>> PreparedStatement according to this is to "efficiently execute this
>> statement multipe times",
>> not "to prevent slq injection attacks" or anything like that;
>>
>
> Because in "real" prepared statements there is no such risk.  The risk
> is the artifact of a bug in our client side simulation of prepared
> statements (not real prepared statements as per definition).

My point was that  the risk exists, when you do *not* use
PreparedStatements, right?
If the purpose of PreparedStatement was to eliminate that risk, it would
have been mentioned. But it is not. Because PreparedStatement has
nothing to do with the security. It is all about efficiency.


>> - it is *explicitly* stated that setObject () should be used for
>> "arbitrary type conversions";
>>
>
> Not that arbitrary.  There is a table specifying for each java type
> that the passed object is member of the proper JDBC type for the
> converted result.  Which must be the type of the field you are trying
> to specify the value for.
>
> So it is not that arbitrary.

It doesn't say *how* arbitrary. It just says "arbitrary". :-)
If you could only pass objects of types in that table, you would not
need setObject () - just setString(), setInt() etc... would suffice.
The whole idea of setObject () is to be able to pass in an argument for
each there is no specialized setter function.

Dima

>
>
>I mean how does your code decide if this or that functionality is
>available or not ? You can only make sure about that by trying out each
>driver version you support for some functionality.
>
Yup. That's what I do.

>
>For example if you have written code which tests for postgres driver and
>does not check the version of that driver, then after this change your
>runtime code will not work anymore with new postgres drivers, because it
>falsely presumes they support setting a list of integers as a parameter
>using the setObject(x, String, Types.INTEGER) workaround.
>
I know :-(
That's exactly why I was trying so hard to argue against it.
I guess, I am going to have to tell my customers that postgres driver
later than 7.3 is not supported...
or I'll have to modify the new driver to put the original functionality
back, and let them download my version :-(

>
>On the other hand if your code does check version numbers, then indeed
>you have to have some kind of list for each such feature of the drivers
>supporting it, with the versions which support it, and you have to
>follow when they stop doing it, etc. And you have to hard-code this
>information in the application code so it has it at runtime, to be able
>to make those decisions...
>
It is *really really* (I can't repeat it enough times) uncommon for
database vendors (other that postgres obviously - sigh) to make
backwards incompatible changes to their software. In other words, I
don't really need any 'list of versions' - just the earliest supported
version should be enough.

>
>I guess this is a lot of maintenance work...
>
>
It is not really that bad... not until I run into this
backwards-incompatible api changes...
It happened in the past once or twice (before I got here, so I don't
know the details), and they even had to drop support of one of the
databases because of that :-(

Dima

Folks, it is getting hard to follow this thread which has actually forked into 3
or more subjects.

Lets please use the newsgroups convention of changing the subject line (making
the Re: into a Was: between square brackets).

Thanks a lot.

--
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Csaba Nagy wrote:

>Well, looks like Dima's problem can be solved after all while still
>being standards compliant in the standard cases :-)
>Still, the individual data types should be validated and correctly
>escaped. If it counts at all, I would vote with this approach.
>Just one more question Dima, how will you at runtime that the current
>driver supports this functionality ? (for that matter, how do you know
>now ?)
>
I am not sure what you mean by 'runtime' in this context.

*At runtime* i just need to know *which* driver I am using. There is, of
course, still work to be done at the implementation stage to figure out
and implement the proper way of dealing with that driver. So, the way I
know is same as usual - by looking at the source and/or documentation of
the driver.

What I am avoiding is direct references to the vendor-specific
classes/packages (like org.postgresql.*), because, if I had those, the
code would not compile unless I have all the drivers available at
compile time. That's what I am trying to avoid.

I understand that I still have to know specifics of the drivers I
support, and have driver-dependent logic in the code, that uses the
right way to deal with the driver at runtime. But I am trying to keep
that logic independent from the actual driver implementation, so that it
does not require the driver to compile.

Dima


>
>Cheers,
>Csaba.
>
>
>
>
>>Oliver,
>>
>>I think Dima is arguing that Collection could be treated as an special case
>>where it indicates a list of something instead of a single something.
>>
>>So, we would iterate through this Collection using its iterator and, for each
>>Object obtained, act like a setObject has been used with that Object and the
>>JAVA TYPE as an argument.
>>
>>The Types.OTHER is used for database specific SQL types or for Dynamic Data
>>Access support.  As the Collection class is not a data type there is no chance
>>of confusion.
>>
>>It seems that Dima's idea can indeed work.
>>
>>Regards,
>>Fernando
>>
>>
>
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 8: explain analyze is your friend
>
>

Peter Kovacs wrote:
> That's, then, even simpler that I originally thought. The only thing to
> be done is to make using "real" prepared statements the default
> behaviour of the PreparedStatement instances, is not it?
>

Yes and that is the plan.  But it is a lot of work.

--Barry


> Fernando Nasser wrote:
>
>> Peter Kovacs wrote:> I think that the simplest thing would be to have
>> an option in the
>>
>>> backend to disable processing of multiple statements in one query --
>>> i.e. disallow the use of ';' as a separator of statements. I am not
>>> sure why this feature (multiple statments in one query) is there
>>> anyway. "Reduce network roundtrips" is the usual reply, but, then,
>>> what is the purpose of stored procedures (functions in PostgreSQL)?
>>>
>>
>> I don't think the backend maintainers will like that.  And btw we
>> don't need this at all.
>>
>> 1) There is no risk of SQL injection with "real" (or "server side")
>> prepared statements.
>>
>> 2) With the proper implementation of the client side prepared
>> statements emulation the injected statements will not go through,
>> because:
>>
>>    a) all non-string results are properly quoted and generated from
>> primary types.
>>
>>    b) String type contents are quoted and have its quotes escaped.
>>
>>> In my perception, JDBC is meant to be a client side extension of the
>>> server - bridge for Java clients to use the server (which in our case
>>> is the right and honorable PostgreSQL). Prepared statements is a
>>> mechanism to indicate the server that more query of this kind is
>>> likely to come so the plan of the query should be kept ready to be
>>> used again. That is what meant by PreparedStatement in the JDBC
>>> driver. I find the concept of "client side prepared statements"
>>> pretty weird.
>>>
>>
>> It was added before the server had prepared statements so the
>> applications could still use PreparedStatements and the getXXX and
>> setXXX methods instead of recreating strings with queries all the
>> time.  Some of the applications run with other databases as well.
>>
>>>  From this perspective, the whole wrestling with "drop table..." and
>>> similar risks seem farily vain to me. At least, the driver is not the
>>> place to solve this kind of security problems which basically exist
>>> due to the wya the server behaves. Instead, the question should be
>>> asked: is the behaviour of the server optimal?. Do we need this
>>> feature (processing multiple, semi-colon separated statements)?
>>> Should not this feature be eventually optional?
>>>
>>
>> No SQL injection is possible with the backend implementation of
>> prepared statements (with the V3 protocol, at least).
>>
>>
>>
>
>

> >Just one more question Dima, how will you at runtime that the current
> >driver supports this functionality ? (for that matter, how do you know
> >now ?)
> >
> I am not sure what you mean by 'runtime' in this context.

I mean how does your code decide if this or that functionality is
available or not ? You can only make sure about that by trying out each
driver version you support for some functionality.

For example if you have written code which tests for postgres driver and
does not check the version of that driver, then after this change your
runtime code will not work anymore with new postgres drivers, because it
falsely presumes they support setting a list of integers as a parameter
using the setObject(x, String, Types.INTEGER) workaround.

On the other hand if your code does check version numbers, then indeed
you have to have some kind of list for each such feature of the drivers
supporting it, with the versions which support it, and you have to
follow when they stop doing it, etc. And you have to hard-code this
information in the application code so it has it at runtime, to be able
to make those decisions...

I guess this is a lot of maintenance work...

I was just wondering how you manage it.

Cheers,
Csaba.

On Mon, 21 Jul 2003, Peter Kovacs wrote:

> I think that the simplest thing would be to have an option in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements. I am not sure
> why this feature (multiple statments in one query) is there anyway.
> "Reduce network roundtrips" is the usual reply, but, then, what is the
> purpose of stored procedures (functions in PostgreSQL)?
>

>  From this perspective, the whole wrestling with "drop table..." and
> similar risks seem farily vain to me. At least, the driver is not the
> place to solve this kind of security problems which basically exist due
> to the wya the server behaves. Instead, the question should be asked: is
> the behaviour of the server optimal?. Do we need this feature
> (processing multiple, semi-colon separated statements)? Should not this
> feature be eventually optional?

The second statement type of attack is just one variant.  Consider a query
that displayed a list of your orders.  SELECT * FROM orders WHERE
userid='username'.  Suppose I substituted a username of username' OR
true OR userid='.  This is another injection attack that does not
require the backend to support multiple statements per query.

Kris Jurka

I have applied a fix for the reported SQL injection vulnerability in
setObject for both 7.3 and 7.4.  I have also places new builds with this
bugfix on the jdbc.postgresql.org website.

For the record, the vulnerability was limited to the
setObject(int,Object,int) method where Object is a String and the type
is a numeric type (i.e. SqlTypes.INTEGER, SqlTypes.LONG, etc)

Given the ongoing discussion that this SQL injection vulnerability has
caused, I decided not to apply the below patch from Kim and instead
fixed the problem in a different way.  The fix essentially applies the
regular escaping done for setString to appropriate values passed to
setObject.  It does not however add quotes to the value.  Thus existing
uses of setObject for in clause and array type values will still
continue to work.

I didn't want to break backward compatibility in a patch to 7.3 and
didn't want to change the functionality in 7.4 until the current
discussions come to a conclusion.

thanks,
--Barry

PS.  I have not yet uploaded new jdbc1 builds to the website.  As I have
recently upgraded to RH9, my jdk1.1 environment no longer works and I
will need to do this build on a different machine tomorrow.


Kim Ho wrote:
> To speed things up a bit, since the regoutParam patch is not likely to
> be approved anytime soon.
>
> This patch
> - adds single quotes for numbers in setObject and also setInt/Byte/etc.
> - Improves getInt/Long when you may have parser errors if you're too
> close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
> - Improves radix point handling when using setObject to an integer
> parameter while passing in a float. This is especially important in
> callable statements.
>
> Cheers,
>
> Kim
>
> On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>>Barry Lind wrote:
>>
>>>Dmitry,
>>>
>>>That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
>>>
>>
>>Kim's patch fixes this.  It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario   M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
>
>
> ------------------------------------------------------------------------
>
> ? temp.diff
> Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
> retrieving revision 1.13
> diff -c -p -r1.13 AbstractJdbc1ResultSet.java
> *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    30 Jun 2003 21:10:55 -0000    1.13
> --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    18 Jul 2003 17:02:20 -0000
> *************** public abstract class AbstractJdbc1Resul
> *** 805,811 ****
>               try
>               {
>                   s = s.trim();
> !                 return Integer.parseInt(s);
>               }
>               catch (NumberFormatException e)
>               {
> --- 805,811 ----
>               try
>               {
>                   s = s.trim();
> !                 return Float.valueOf(s).intValue();
>               }
>               catch (NumberFormatException e)
>               {
> *************** public abstract class AbstractJdbc1Resul
> *** 822,828 ****
>               try
>               {
>                   s = s.trim();
> !                 return Long.parseLong(s);
>               }
>               catch (NumberFormatException e)
>               {
> --- 822,828 ----
>               try
>               {
>                   s = s.trim();
> !                 return Double.valueOf(s).longValue();
>               }
>               catch (NumberFormatException e)
>               {
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.27
> diff -c -p -r1.27 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java    9 Jul 2003 05:12:04 -0000    1.27
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java    18 Jul 2003 17:02:22 -0000
> *************** public abstract class AbstractJdbc1State
> *** 920,926 ****
>        */
>       public void setByte(int parameterIndex, byte x) throws SQLException
>       {
> !         bind(parameterIndex, Integer.toString(x), PG_TEXT);
>       }
>
>       /*
> --- 920,926 ----
>        */
>       public void setByte(int parameterIndex, byte x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 933,939 ****
>        */
>       public void setShort(int parameterIndex, short x) throws SQLException
>       {
> !         bind(parameterIndex, Integer.toString(x), PG_INT2);
>       }
>
>       /*
> --- 933,939 ----
>        */
>       public void setShort(int parameterIndex, short x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 946,952 ****
>        */
>       public void setInt(int parameterIndex, int x) throws SQLException
>       {
> !         bind(parameterIndex, Integer.toString(x), PG_INTEGER);
>       }
>
>       /*
> --- 946,952 ----
>        */
>       public void setInt(int parameterIndex, int x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 959,965 ****
>        */
>       public void setLong(int parameterIndex, long x) throws SQLException
>       {
> !         bind(parameterIndex, Long.toString(x), PG_INT8);
>       }
>
>       /*
> --- 959,965 ----
>        */
>       public void setLong(int parameterIndex, long x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 972,978 ****
>        */
>       public void setFloat(int parameterIndex, float x) throws SQLException
>       {
> !         bind(parameterIndex, Float.toString(x), PG_FLOAT);
>       }
>
>       /*
> --- 972,978 ----
>        */
>       public void setFloat(int parameterIndex, float x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 985,991 ****
>        */
>       public void setDouble(int parameterIndex, double x) throws SQLException
>       {
> !         bind(parameterIndex, Double.toString(x), PG_DOUBLE);
>       }
>
>       /*
> --- 985,991 ----
>        */
>       public void setDouble(int parameterIndex, double x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 1003,1009 ****
>               setNull(parameterIndex, Types.DECIMAL);
>           else
>           {
> !             bind(parameterIndex, x.toString(), PG_NUMERIC);
>           }
>       }
>
> --- 1003,1009 ----
>               setNull(parameterIndex, Types.DECIMAL);
>           else
>           {
> !             bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
>           }
>       }
>
> *************** public abstract class AbstractJdbc1State
> *** 1464,1486 ****
>           switch (targetSqlType)
>           {
>               case Types.INTEGER:
> -                 if (x instanceof Boolean)
> -                     bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
> -                 else
> -                     bind(parameterIndex, x.toString(), PG_INTEGER);
> -                 break;
>               case Types.TINYINT:
>               case Types.SMALLINT:
>               case Types.BIGINT:
>               case Types.REAL:
>               case Types.FLOAT:
>               case Types.DOUBLE:
>               case Types.DECIMAL:
>               case Types.NUMERIC:
> !                 if (x instanceof Boolean)
> !                     bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
> !                 else
> !                     bind(parameterIndex, x.toString(), PG_NUMERIC);
>                   break;
>               case Types.CHAR:
>               case Types.VARCHAR:
> --- 1464,1484 ----
>           switch (targetSqlType)
>           {
>               case Types.INTEGER:
>               case Types.TINYINT:
>               case Types.SMALLINT:
> +                 x = removeRadix(x,Types.INTEGER);
> +                 bindNumber(parameterIndex,x,PG_INTEGER);
> +                 break;
>               case Types.BIGINT:
> +                 x = removeRadix(x,Types.BIGINT);
> +                 bindNumber(parameterIndex,x,PG_INT8);
> +                 break;
>               case Types.REAL:
>               case Types.FLOAT:
>               case Types.DOUBLE:
>               case Types.DECIMAL:
>               case Types.NUMERIC:
> !                 bindNumber(parameterIndex,x,PG_NUMERIC);
>                   break;
>               case Types.CHAR:
>               case Types.VARCHAR:
> *************** public abstract class AbstractJdbc1State
> *** 2026,2031 ****
> --- 2024,2056 ----
>           if (parameterIndex != 1)
>               throw new PSQLException("postgresql.call.noinout");
>       }
> +
> +     private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
> +     {
> +         if (x instanceof Boolean)
> +             bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
> +         else
> +             bind(parameterIndex, "'"+x.toString()+"'", pgtype);
> +     }
> +
> +
> +     private Object removeRadix(Object x, int sqlType)
> +     {
> +         if (x.toString().indexOf(".")>0)
> +         {
> +             switch (sqlType)
> +             {
> +                 case Types.BIGINT:
> +                     x = String.valueOf(Double.valueOf(x.toString()).longValue());
> +                     break;
> +                 default:
> +                     x = String.valueOf(Float.valueOf(x.toString()).intValue());
> +                     break;
> +             }
> +         }
> +         return x;
> +     }
> +
>
>
>
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly

On Mon, Jul 21, 2003 at 10:49:14PM -0700, Barry Lind wrote:

> Given the ongoing discussion that this SQL injection vulnerability has
> caused, I decided not to apply the below patch from Kim and instead
> fixed the problem in a different way.  The fix essentially applies the
> regular escaping done for setString to appropriate values passed to
> setObject.  It does not however add quotes to the value.  Thus existing
> uses of setObject for in clause and array type values will still
> continue to work.

I haven't looked at the updated tree yet, but from your description won't
this break code that does something like this? :

  stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
  stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);

-O

On 21/07/2003 18:51 Fernando Nasser wrote:
> Also, we may limit this behavior with Collections to the IN clause
> only.  Where else could we need lists to replace the '?' ?

Nowhere. Not even with an IN clause. If the programmer needs IN(1,2,3,4,5)
then he must write IN(?,?,?,?,?) in his prepare string. That's the way
JDBC works. Period. Acceptance of any other behaviour is un-professional
and against the standards. As you said yourself, neither Oracle nor DB2
support this behavior. Neither should PostgreSQL.


--
Paul Thomas
+------------------------------+---------------------------------------------+
| Thomas Micro Systems Limited | Software Solutions for the Smaller
Business |
| Computer Consultants         |
http://www.thomas-micro-systems-ltd.co.uk   |
+------------------------------+---------------------------------------------+

On Tue, Jul 22, 2003 at 09:33:53AM -0400, Tom Lane wrote:
> Oliver Jowett <oliver@opencloud.com> writes:
> > ... won't this break code that does something like this? :
>
> >   stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
> >   stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);
>
> Code that does that is just going to have to break.  We should try to
> provide equivalent functionality in a less unsafe fashion; but
> backwards compatibility with code that is exploiting a security hole
> is not an option.

I agree; since we can't remain backwards-compatible in all cases, is it
worth doing the odd halfway-escaped thing for the sake of the remaining
cases?

-O

Oliver Jowett <oliver@opencloud.com> writes:
> ... won't this break code that does something like this? :

>   stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
>   stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);

Code that does that is just going to have to break.  We should try to
provide equivalent functionality in a less unsafe fashion; but
backwards compatibility with code that is exploiting a security hole
is not an option.

            regards, tom lane

Paul Thomas wrote:
>
> On 21/07/2003 18:51 Fernando Nasser wrote:
>
>> Also, we may limit this behavior with Collections to the IN clause
>> only.  Where else could we need lists to replace the '?' ?
>
>
> Nowhere. Not even with an IN clause. If the programmer needs
> IN(1,2,3,4,5) then he must write IN(?,?,?,?,?) in his prepare string.
> That's the way JDBC works. Period. Acceptance of any other behaviour is
> un-professional and against the standards. As you said yourself, neither
> Oracle nor DB2 support this behavior. Neither should PostgreSQL.
>

Well, I was just informed that the 7.4 backend supports an IN list which
is filled with a PostgreSQL array.  As the syntax requires the
parenthesis to be in place (it only fills the list itself) there is no
ambiguity.

Its support is limited on 7.4 (the optimized is not aware of it and
generates a crappy plan) but one may consider improving it for 7.5.


--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Well, that's your opinion, calm down :-)
  Anyway, I really think the solution of add various parameter marks ("?") and fill the ones you don't use with nulls
israther awful. There is a database that provides another solution for that? 

On Tue, 22 Jul 2003 09:34:10 +0100
Paul Thomas <paul@tmsl.demon.co.uk> wrote:

>
> On 21/07/2003 18:51 Fernando Nasser wrote:
> > Also, we may limit this behavior with Collections to the IN clause
> > only.  Where else could we need lists to replace the '?' ?
>
> Nowhere. Not even with an IN clause. If the programmer needs IN(1,2,3,4,5)
> then he must write IN(?,?,?,?,?) in his prepare string. That's the way
> JDBC works. Period. Acceptance of any other behaviour is un-professional
> and against the standards. As you said yourself, neither Oracle nor DB2
> support this behavior. Neither should PostgreSQL.
>
>
> --
> Paul Thomas
> +------------------------------+---------------------------------------------+
> | Thomas Micro Systems Limited | Software Solutions for the Smaller
> Business |
> | Computer Consultants         |
> http://www.thomas-micro-systems-ltd.co.uk   |
> +------------------------------+---------------------------------------------+
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match


--

 /~\ The ASCII        Felipe Schnack (felipes@ritterdosreis.br)
 \ / Ribbon Campaign  Analista de Sistemas
  X  Against HTML     Cel.: 51-91287530
 / \ Email!           Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter@ritterdosreis.br
Fone: 51-32303341

Felipe Schnack wrote:
>   Well, that's your opinion, calm down :-)
>   Anyway, I really think the solution of add various parameter marks ("?") and fill the ones you don't use with nulls
israther awful. There is a database that provides another solution for that? 
>

Not DB2 nor Oracle.  Only PostgreSQL 7.4 and with the planner
restriction I've mentioned.

Fernando

P.S.: Although I agree that any extension to the standard must be very
carefully thought, the PostgreSQL community has been very successful at
being less restrictive and has actually improved the behavior compared
to the standard.  So, if we can come up with a sensible way of filling
the <in values list> of the IN <predicate> I believe we should.

But _never_ leaving a security hole or in a way that clearly will break
possible future expansions of the JDBC.



> On Tue, 22 Jul 2003 09:34:10 +0100
> Paul Thomas <paul@tmsl.demon.co.uk> wrote:
>
>
>>On 21/07/2003 18:51 Fernando Nasser wrote:
>>
>>>Also, we may limit this behavior with Collections to the IN clause
>>>only.  Where else could we need lists to replace the '?' ?
>>
>>Nowhere. Not even with an IN clause. If the programmer needs IN(1,2,3,4,5)
>>then he must write IN(?,?,?,?,?) in his prepare string. That's the way
>>JDBC works. Period. Acceptance of any other behaviour is un-professional
>>and against the standards. As you said yourself, neither Oracle nor DB2
>>support this behavior. Neither should PostgreSQL.
>>
>>
>>--
>>Paul Thomas
>>+------------------------------+---------------------------------------------+
>>| Thomas Micro Systems Limited | Software Solutions for the Smaller
>>Business |
>>| Computer Consultants         |
>>http://www.thomas-micro-systems-ltd.co.uk   |
>>+------------------------------+---------------------------------------------+
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 9: the planner will ignore your desire to choose an index scan if your
>>      joining column's datatypes do not match
>
>
>


--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Oliver,

Yes that will no longer work.  But syntactically it shouldn't anyway.
You are passing a set of strings and saying the type is NUMERIC.  What
will still work is passing a set of numeric values:

    stmt.setObject(1, "(1, 2, 3)", Types.NUMERIC);

thanks,
--Barry


Oliver Jowett wrote:
> On Mon, Jul 21, 2003 at 10:49:14PM -0700, Barry Lind wrote:
>
>
>>Given the ongoing discussion that this SQL injection vulnerability has
>>caused, I decided not to apply the below patch from Kim and instead
>>fixed the problem in a different way.  The fix essentially applies the
>>regular escaping done for setString to appropriate values passed to
>>setObject.  It does not however add quotes to the value.  Thus existing
>>uses of setObject for in clause and array type values will still
>>continue to work.
>
>
> I haven't looked at the updated tree yet, but from your description won't
> this break code that does something like this? :
>
>   stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
>   stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);
>
> -O
>

On Tue, Jul 22, 2003 at 08:53:36AM -0700, Barry Lind wrote:
> Oliver,
>
> Yes that will no longer work.  But syntactically it shouldn't anyway.
> You are passing a set of strings and saying the type is NUMERIC.  What
> will still work is passing a set of numeric values:
>
>    stmt.setObject(1, "(1, 2, 3)", Types.NUMERIC);

I agree that it makes no sense syntantically, but it *is* a loophole we're
talking about here! Interpreting "(1,2,3)" as a NUMERIC type doesn't make
sense either.

Anyway, if the half-escaping doesn't break anything standard, fine. I'd just
rather not make the driver ugly for the sake of backwards compatibility with
a *subset* of the cases where setObject was used in a non-standard way :)

-O

Peter Kovacs <peter.kovacs@siemens.com> writes:
> I think that the simplest thing would be to have an option in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements.

FWIW, the new "extended query" protocol has exactly such a restriction.
However that hardly excuses any sloppiness in allowing
non-syntax-checked parameter values through.  Consider changing
"WHERE x < ?" to
"WHERE x < 42 AND my_function_with_interesting_side_effects()"

No semicolons in sight, but I can still clean out your bank balance ;-)

            regards, tom lane

Tom Lane wrote:

>Peter Kovacs <peter.kovacs@siemens.com> writes:
>
>
>>I think that the simplest thing would be to have an option in the
>>backend to disable processing of multiple statements in one query --
>>i.e. disallow the use of ';' as a separator of statements.
>>
>>
>
>FWIW, the new "extended query" protocol has exactly such a restriction.
>However that hardly excuses any sloppiness in allowing
>non-syntax-checked parameter values through.  Consider changing
>"WHERE x < ?" to
>"WHERE x < 42 AND my_function_with_interesting_side_effects()"
>
>No semicolons in sight, but I can still clean out your bank balance ;-)
>
...and it would serve me right :(.

BTW, I presume that one can deny a user the right to create stored
procedures in PostgreSQL. Anyway, I now recognize that the issue is more
complicated than allowing';'.

Regards,
Peter

>
>            regards, tom lane
>
>

Barry Lind wrote:
> Oliver,
>
> Yes that will no longer work.  But syntactically it shouldn't anyway.
> You are passing a set of strings and saying the type is NUMERIC.  What
> will still work is passing a set of numeric values:
>
>    stmt.setObject(1, "(1, 2, 3)", Types.NUMERIC);
>

Can we pass a set of strings?  Otherwise it is a half-way solution.

stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);

Will it be the string '('a1', 'b2', 'c3')' or the list of strings 'a1'
'b2' and 'c3'?


--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Fernando Nasser wrote:
> Barry Lind wrote:
>
>> Oliver,
>>
>> Yes that will no longer work.  But syntactically it shouldn't anyway.
>> You are passing a set of strings and saying the type is NUMERIC.  What
>> will still work is passing a set of numeric values:
>>
>>    stmt.setObject(1, "(1, 2, 3)", Types.NUMERIC);
>>
>
> Can we pass a set of strings?  Otherwise it is a half-way solution.
>
> stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);

I am not sure what you are asking, but if you make the above call you
will send the following to the server:

where ... in (\'a1\', \'b2\', \'c3\') ...

Which is as it has always been since Types.VARCHAR caused proper
escaping.  The commited change causes the above to happen even when you
say the type is Types.NUMERIC.

I don't know what you mean by a half-way solution, the fix closes the
security vulnerability and makes the behavior for Types.NUMERIC
consistent with the behavior of Types.VARCHAR.

thanks,
--Barry

Barry Lind wrote:
>
>
> Fernando Nasser wrote:
>
>> Barry Lind wrote:
>>
>>> Oliver,
>>>
>>> Yes that will no longer work.  But syntactically it shouldn't anyway.
>>> You are passing a set of strings and saying the type is NUMERIC.
>>> What will still work is passing a set of numeric values:
>>>
>>>    stmt.setObject(1, "(1, 2, 3)", Types.NUMERIC);
>>>
>>
>> Can we pass a set of strings?  Otherwise it is a half-way solution.
>>
>> stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);
>
>
> I am not sure what you are asking, but if you make the above call you
> will send the following to the server:
>
> where ... in (\'a1\', \'b2\', \'c3\') ...
>
> Which is as it has always been since Types.VARCHAR caused proper
> escaping.  The commited change causes the above to happen even when you
> say the type is Types.NUMERIC.
>

OK, let me rephrase it:

What if my string (which is a string, not a list) contains the
characters "('a1', 'b2', 'c3')"?   How do I set my parameter to such a
string with setObject?





--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

I haven't seen the code... but, I I understand correctly what you are
describing, "does the regular escaping like setString(), but doesn't
include th einput in quotes), I don't understand how it helps to fix
that "security problem" that started all this...
Won't something like "select * from users where id in ?"   get
translated into 'select * from users where id in (1);drop table users'
just like before after a
setObject (1, "(1);drop table users", Typed.NUMERIC)?

As far as I remember, setString () does not escape semicolons, right?

Dima

Barry Lind wrote:

>
>
> Fernando Nasser wrote:
>
>> Barry Lind wrote:
>>
>>> Oliver,
>>>
>>> Yes that will no longer work.  But syntactically it shouldn't
>>> anyway. You are passing a set of strings and saying the type is
>>> NUMERIC.  What will still work is passing a set of numeric values:
>>>
>>>    stmt.setObject(1, "(1, 2, 3)", Types.NUMERIC);
>>>
>>
>> Can we pass a set of strings?  Otherwise it is a half-way solution.
>>
>> stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);
>
>
> I am not sure what you are asking, but if you make the above call you
> will send the following to the server:
>
> where ... in (\'a1\', \'b2\', \'c3\') ...
>
> Which is as it has always been since Types.VARCHAR caused proper
> escaping.  The commited change causes the above to happen even when
> you say the type is Types.NUMERIC.
>
> I don't know what you mean by a half-way solution, the fix closes the
> security vulnerability and makes the behavior for Types.NUMERIC
> consistent with the behavior of Types.VARCHAR.
>
> thanks,
> --Barry
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>      subscribe-nomail command to majordomo@postgresql.org so that your
>      message can get through to the mailing list cleanly

Fernando,


Fernando Nasser wrote:
> What if my string (which is a string, not a list) contains the
> characters "('a1', 'b2', 'c3')"?   How do I set my parameter to such a
> string with setObject?

OK, now I understand your question.  This will still work, just like it
always has.  The single quotes will be escaped before sending them to
the backend and the result will be what you would expect.

So if the query was: insert into foo (bar) values (?)

stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);

would result in the following statement sent to the server:

insert into foo (bar) values ('(\'a1\', \'b2\', \'c3\')')

which will result in the value ('a1', 'b2', 'c3') being inserted.

thanks,
--Barry

Barry Lind wrote:
> Fernando,
>
>
> Fernando Nasser wrote:
>
>> What if my string (which is a string, not a list) contains the
>> characters "('a1', 'b2', 'c3')"?   How do I set my parameter to such a
>> string with setObject?
>
>
> OK, now I understand your question.  This will still work, just like it
> always has.  The single quotes will be escaped before sending them to
> the backend and the result will be what you would expect.
>
> So if the query was: insert into foo (bar) values (?)
>
> stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);
>
> would result in the following statement sent to the server:
>
> insert into foo (bar) values ('(\'a1\', \'b2\', \'c3\')')
>
> which will result in the value ('a1', 'b2', 'c3') being inserted.
>

OK, so far so good.  And my other question is:

Can we pass a set of strings?

stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);

will result into:

... where ... in (\'a1\', \'b2\', \'c3\') ...

while the proper syntax should be:

... where ... in ('a1', 'b2', 'c3') ...

or will the backend work even with the escaped quotes?

What was I refering to partial solution (or something of a sort) was the
fact that you can fill your IN predicate <in values list> if the
elements of the list are numeric values but not if the values where
VARCHARs.



--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Fernando Nasser wrote:
> Barry Lind wrote:
>
>> Fernando,
>>
>>
>> Fernando Nasser wrote:
>>
>>> What if my string (which is a string, not a list) contains the
>>> characters "('a1', 'b2', 'c3')"?   How do I set my parameter to such
>>> a string with setObject?
>>
>>
>>
>> OK, now I understand your question.  This will still work, just like
>> it always has.  The single quotes will be escaped before sending them
>> to the backend and the result will be what you would expect.
>>
>> So if the query was: insert into foo (bar) values (?)
>>
>> stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);
>>
>> would result in the following statement sent to the server:
>>
>> insert into foo (bar) values ('(\'a1\', \'b2\', \'c3\')')
>>
>> which will result in the value ('a1', 'b2', 'c3') being inserted.
>>
>
> OK, so far so good.  And my other question is:
>
> Can we pass a set of strings?

No.

>
> stmt.setObject(1, "('a1', 'b2', 'c3')", Types.VARCHAR);
>
> will result into:
>
> ... where ... in (\'a1\', \'b2\', \'c3\') ...
The actual result will be:
... where ... in '(\'a1\', \'b2\', \'c3\')' ...

which isn't valid sql syntax

if you used stmt.setObject(1, "('a1', 'b2', 'c3')", Types.NUMERIC);
you would get:
... where ... in (\'a1\', \'b2\', \'c3\') ...

which is somewhat closer but still not valid sql as the backend needs
unescaped single quotes.  Basically there isn't a way to get unescaped
single quotes through to the backend which is required for what you are
trying to do.  And the reason there isn't a way to get unescaped single
quotes through is that would allow a SQL injection attack.

thanks,
--Barry

I have commited a change that completely removes the ability to pass
anything other than a numeric value when using the setObject() calls for
types that claim to be numeric.  As Dmitry has pointed out any desire to
maintain the support for allowing "where ... in (?)" and being able to
pass a list of values for that single bind variable if flawed.  So the
latest patch completely closes the sql injection vulnerability by
preventing this not standard behavior.

thanks,
--Barry

Dmitry Tkach wrote:
>
> Ok... What about:
> select * from users where id in ?
> setObject (1, "(select setval ('users_id_seq', 1)"); //to screw up the
> PK sequence
>
> or...
>
> setObject (1, "(1) or true"); //to get a list of all the users and
> passwords
>
> or...
>
> setObject (1, "(1) union all select * from secret_table");
>