Re: SQL_CURSOR_TYPE prepare execute issue - Mailing list pgsql-odbc

Hi,

As long as you are confident that the test suite is checking this code path that is fine, I just wanted to be sure that
washappening. 

I hadn't thought of
  select x-?
You are right that is a problem.
What does the server prepare code path do?
I did a quick check, sever side prepare, with the log turned on and it shows the following
  LOG:  execute <unnamed>: select 1-$1
  DETAIL:  parameters: $1 = '-1'
  LOG:  execute <unnamed>: select 1-$1
  DETAIL:  parameters: $1 = '1'
But if the bound string is '-1' then I get
  ERROR:  invalid input syntax for integer: "'-1'"   i.e. double quote,single quote,-,1,single quote,double quote
  STATEMENT:  select 'wibble',1-$1
Also get invalid syntax error if it is just a minus sign.

ok, server side prepare log when string is just 6 shows
  LOG:  execute <unnamed>: select '555'>$1
  DETAIL:  parameters: $1 = '6'
and this return one row with a value of 0
so this is doing select '555'>'6';

So to be consistent with what the server side prepare does, I think it should quote the value.
You should check this as I may have been getting confused.
I realise this may have some implications for existing programs but given that sometimes it will be server side prepare
andsometimes client side it is likely to cause more issues if the results are different. 

PS. Sorry about top posting. Outlook Web App provides no other option.

Regards,
Jeremy
________________________________________
From: Heikki Linnakangas [hlinnakangas@vmware.com]
Sent: 15 January 2015 10:52
To: Faith, Jeremy; pgsql-odbc@postgresql.org
Subject: Re: [ODBC] SQL_CURSOR_TYPE prepare execute issue

On 01/14/2015 06:56 PM, Faith, Jeremy wrote:
> Hi,
>
> Thanks for the fix.
>
> I think the test code should call
>    SQLSetStmtOption(hstmt,SQL_CURSOR_TYPE,SQL_CURSOR_STATIC);
> after SQLAllocHandle. As otherwise even with the 9.03.03 version of the ODBC driver the correct error occurs unless
UseServerSidePrepare=0in the odbc.ini file. That is I am not sure the changed code is being used by the test as it is. 

It is common practice to run the test suite with different settings. At
least I do it, I'm not sure of others, though :-). I created a makefile
target "installcheck-all" specifically for that.

There are some subtle differences in the behaviour with
UserServerSidePrepare=0/1 in what datatype the server chooses for the
parameters, so I'd like to continue testing both and not force the
non-server-side codepath with SQLSetStmtOption.

> I have had a quick look over the change and it looks ok to me. Something of a clean up and simplification as well.
> If I understand it correctly, the only things that don't get quoted are SQL_INTEGER and SQL_SMALLINT that pass the
newvalid_int_literal() test. 
> The only thing I can see that could pass that test and not be a valid integer would be a single minus char i.e. "-"
> not sure if there is anyway that could be vulnerable though.

Ah, good catch. That is definitely a problem. Consider:

SELECT * FROM foo WHERE 1-? > 0

If you replace ? with -, it becomes "--", which comments out the rest of
the query. That's actually a problem with any negative number.

It would be tempting to just always quote the value, but that again
would lead to subtle changes in the datatype that the server chooses.
There is a difference between:

SELECT '555' > '6';

and

SELECT '555' > 6;

We'd have to force the datatype with something like:

SELECT '555' > '6'::int4;

Perhaps that's the safest thing to do. It might cause subtle changes in
behaviour too, if e.g. you pass a number larger than what fits in an
int4. The server will upgrade it to an int8 or numeric constant if it's
just a numeric literal, but if you force it to int4, you'll get an
error. Now, you can certainly argue that that's a good thing, as you
shouldn't have specified SQL_INTEGER for an over-sized value in the
first place. So perhaps we should just bite the bullet and do that.

Another argument is that '6'::int4 looks more ugly than just 6. Not a
big deal, but still.

Another option is to use parens around a negative number. So it would
become:

SELECT '555' > (-6)

or

SELECT * FROM foo WHERE 1-(-6) > 0

That would be closest to the current behaviour. Does anyone see a
problem with that?

- Heikki

Tyco Safety Products/CEM Systems Ltd.

________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you
arenot the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any
actionin respect of any information contained in it. If you have received this e-mail in error, please notify the
senderimmediately by e-mail and immediately destroy this e-mail and its attachments.