Thread: Re: [INTERFACES] New code for JDBC driver
> No response for my query since yesterday. > > I've changed the code in Connection.escapeSQL() to look for semicolons > outside '' and ""'s. > > It's not been well tested, but it seems to work. Someone might want to add > it to the source tree, or to their own local versions if they wish. > OK, here is the diff, I think. Can jdbc people comment on it? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 *** Connection.java Wed Jun 6 20:09:32 2001 --- /bjm/x Sat Jun 30 12:35:39 2001 *************** *** 931,937 **** --- 21,123 ---- index = sql.indexOf("{d"); } //System.out.println ("modified SQL: " + sql); + + + + + //---- Added by Arsalan + String query = sql; + + //2== uninitialised 1==true 0==false + int openTick = 2; + int openQuote = 2; + char oneChar; + + // query = query.toLowerCase(); + + if(query.indexOf(";") == -1) // no semi's at all + { return sql; } + for(int i=0; i<query.length(); i++) + { + oneChar = query.charAt(i); + + if((oneChar == ';' && openTick == 0 && openQuote != 1) || (oneChar == ';' && openQuote == 0 && openTick != 1)) + { + throw new SQLException("Found an external SEMICOLON!! at i = "+i); + } + + if(oneChar == '\'') //is a ' + { + + System.out.println("in '"); + int j = i; + if(++j < query.length()) + { + if(query.charAt(j) == '\'') //is an escape + { + i++; //skip them + continue; + } + else //genuine tick! + { + if(openTick == 0) + { + openTick = 1; + } + else if(openTick == 1) + { + openTick = 0; + } + else if(openTick == 2) //initialise it + { + openTick = 1; + } + } + } + } + + + if(oneChar == '"') //is a " + { + System.out.println("in \""); + int j = i; + if(++j < query.length()) + { + if(query.charAt(j) == '"') //is an escape + { + i++; //skip them + continue; + } + else //genuine quote! + { + if(openQuote == 0) + { + openQuote = 1; + } + else if(openQuote == 1) + { + openQuote = 0; + } + else if(openQuote == 2) //initialise it + { + openQuote = 1; + } + } + } + } + + + } + + + + + //------Addition ends + + + + return sql; }
Can you give us some background. What is this trying to solve? From the look of it the purpose is to remove embedded semicolons and quotes. From my POV this should be handled by the backend. My reasons are as follows: 1) Any attempt at parsing the query by the driver will slow the driver down. 2) If the purpose of parsing is to make an incorrect query correct; then I would prefer the backend fail and make me fix my code. Dave ----- Original Message ----- From: "Bruce Momjian" <pgman@candle.pha.pa.us> To: "Arsalan Zaidi" <azaidi@directi.com> Cc: "PostgreSQL jdbc list" <pgsql-jdbc@postgresql.org> Sent: Saturday, June 30, 2001 12:42 PM Subject: [JDBC] Re: [INTERFACES] New code for JDBC driver > > No response for my query since yesterday. > > > > I've changed the code in Connection.escapeSQL() to look for semicolons > > outside '' and ""'s. > > > > It's not been well tested, but it seems to work. Someone might want to add > > it to the source tree, or to their own local versions if they wish. > > > > OK, here is the diff, I think. Can jdbc people comment on it? > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 853-3000 > + If your life is a hard drive, | 830 Blythe Avenue > + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 > ---------------------------------------------------------------------------- ---- > *** Connection.java Wed Jun 6 20:09:32 2001 > --- /bjm/x Sat Jun 30 12:35:39 2001 > *************** > *** 931,937 **** > --- 21,123 ---- > index = sql.indexOf("{d"); > } > file://System.out.println ("modified SQL: " + sql); > + > + > + > + > + file://---- Added by Arsalan > + String query = sql; > + > + file://2== uninitialised 1==true 0==false > + int openTick = 2; > + int openQuote = 2; > + char oneChar; > + > + // query = query.toLowerCase(); > + > + if(query.indexOf(";") == -1) // no semi's at all > + { > return sql; > } > > + for(int i=0; i<query.length(); i++) > + { > + oneChar = query.charAt(i); > + > + if((oneChar == ';' && openTick == 0 && openQuote != 1) || (oneChar == ';' && openQuote == 0 && openTick != 1)) > + { > + throw new SQLException("Found an external SEMICOLON!! at i = "+i); > + } > + > + if(oneChar == '\'') file://is a ' > + { > + > + System.out.println("in '"); > + int j = i; > + if(++j < query.length()) > + { > + if(query.charAt(j) == '\'') file://is an escape > + { > + i++; file://skip them > + continue; > + } > + else file://genuine tick! > + { > + if(openTick == 0) > + { > + openTick = 1; > + } > + else if(openTick == 1) > + { > + openTick = 0; > + } > + else if(openTick == 2) file://initialise it > + { > + openTick = 1; > + } > + } > + } > + } > + > + > + if(oneChar == '"') file://is a " > + { > + System.out.println("in \""); > + int j = i; > + if(++j < query.length()) > + { > + if(query.charAt(j) == '"') file://is an escape > + { > + i++; file://skip them > + continue; > + } > + else file://genuine quote! > + { > + if(openQuote == 0) > + > + openQuote = 1; > + } > + else if(openQuote == 1) > + { > + openQuote = 0; > + } > + else if(openQuote == 2) file://initialise it > + { > + openQuote = 1; > + } > + } > + } > + } > + > + > + } > + > + > + > + > + file://------Addition ends > + > + > + > + return sql; > } > ---------------------------------------------------------------------------- ---- > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org >
> Can you give us some background. What is this trying to solve? > >From the look of it the purpose is to remove embedded semicolons and quotes. > >From my POV this should be handled by the backend. > > My reasons are as follows: > > 1) Any attempt at parsing the query by the driver will slow the driver down. > 2) If the purpose of parsing is to make an incorrect query correct; then I > would prefer the backend fail and make me fix my code. psql does this kind of query spliting but for different reasons. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Arsalan,
I am having a hard time understanding what this patch is supposed to do.
What do you mean by "look for semicolons outside '' and ""'s"? It is
valid in postgresql to have semicolons outside quotes, so I don't
understand what you are trying to do.
After looking at the JDBC spec on what escape processing is supposed to
do (section 11 in the jdbc 1.2 spec), it is intended to be a database
independent way to specify certain constructs that are not yet
implemented in a standard way across databases. All of these special
constructs are of the form: {name parameters}
For example {d '2001-06-30'} is a way to specify a date value regardless
of the underlying databases date format. Apparently the JDBC spec uses
the same logic as is defined by ODBC for the same purposes.
thanks,
--Barry
Bruce Momjian wrote:
>>No response for my query since yesterday.
>>
>>I've changed the code in Connection.escapeSQL() to look for semicolons
>>outside '' and ""'s.
>>
>>It's not been well tested, but it seems to work. Someone might want to add
>>it to the source tree, or to their own local versions if they wish.
>>
>>
>
> OK, here is the diff, I think. Can jdbc people comment on it?
>
>
>
> ------------------------------------------------------------------------
>
> *** Connection.java Wed Jun 6 20:09:32 2001
> --- /bjm/x Sat Jun 30 12:35:39 2001
> ***************
> *** 931,937 ****
> --- 21,123 ----
> index = sql.indexOf("{d");
> }
> //System.out.println ("modified SQL: " + sql);
> +
> +
> +
> +
> + //---- Added by Arsalan
> + String query = sql;
> +
> + //2== uninitialised 1==true 0==false
> + int openTick = 2;
> + int openQuote = 2;
> + char oneChar;
> +
> + // query = query.toLowerCase();
> +
> + if(query.indexOf(";") == -1) // no semi's at all
> + {
> return sql;
> }
>
> + for(int i=0; i<query.length(); i++)
> + {
> + oneChar = query.charAt(i);
> +
> + if((oneChar == ';' && openTick == 0 && openQuote != 1) || (oneChar == ';' && openQuote == 0 && openTick
!=1))
> + {
> + throw new SQLException("Found an external SEMICOLON!! at i = "+i);
> + }
> +
> + if(oneChar == '\'') //is a '
> + {
> +
> + System.out.println("in '");
> + int j = i;
> + if(++j < query.length())
> + {
> + if(query.charAt(j) == '\'') //is an escape
> + {
> + i++; //skip them
> + continue;
> + }
> + else //genuine tick!
> + {
> + if(openTick == 0)
> + {
> + openTick = 1;
> + }
> + else if(openTick == 1)
> + {
> + openTick = 0;
> + }
> + else if(openTick == 2) //initialise it
> + {
> + openTick = 1;
> + }
> + }
> + }
> + }
> +
> +
> + if(oneChar == '"') //is a "
> + {
> + System.out.println("in \"");
> + int j = i;
> + if(++j < query.length())
> + {
> + if(query.charAt(j) == '"') //is an escape
> + {
> + i++; //skip them
> + continue;
> + }
> + else //genuine quote!
> + {
> + if(openQuote == 0)
> + {
> + openQuote = 1;
> + }
> + else if(openQuote == 1)
> + {
> + openQuote = 0;
> + }
> + else if(openQuote == 2) //initialise it
> + {
> + openQuote = 1;
> + }
> + }
> + }
> + }
> +
> +
> + }
> +
> +
> +
> +
> + //------Addition ends
> +
> +
> +
> + return sql;
> }
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
> /bjm/diff
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> 7bit
>
>
> ------------------------------------------------------------------------
> Part 1.3
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> binary
>
>
Arsalan,
I am having a hard time understanding what this patch is supposed to do.
What do you mean by "look for semicolons outside '' and ""'s"? It is
valid in postgresql to have semicolons outside quotes, so I don't
understand what you are trying to do.
After looking at the JDBC spec on what escape processing is supposed to
do (section 11 in the jdbc 1.2 spec), it is intended to be a database
independent way to specify certain constructs that are not yet
implemented in a standard way across databases. All of these special
constructs are of the form: {name parameters}
For example {d '2001-06-30'} is a way to specify a date value regardless
of the underlying databases date format. Apparently the JDBC spec uses
the same logic as is defined by ODBC for the same purposes.
thanks,
--Barry
Bruce Momjian wrote:
>>No response for my query since yesterday.
>>
>>I've changed the code in Connection.escapeSQL() to look for semicolons
>>outside '' and ""'s.
>>
>>It's not been well tested, but it seems to work. Someone might want to add
>>it to the source tree, or to their own local versions if they wish.
>>
>>
>
> OK, here is the diff, I think. Can jdbc people comment on it?
>
>
>
> ------------------------------------------------------------------------
>
> *** Connection.java Wed Jun 6 20:09:32 2001
> --- /bjm/x Sat Jun 30 12:35:39 2001
> ***************
> *** 931,937 ****
> --- 21,123 ----
> index = sql.indexOf("{d");
> }
> //System.out.println ("modified SQL: " + sql);
> +
> +
> +
> +
> + //---- Added by Arsalan
> + String query = sql;
> +
> + //2== uninitialised 1==true 0==false
> + int openTick = 2;
> + int openQuote = 2;
> + char oneChar;
> +
> + // query = query.toLowerCase();
> +
> + if(query.indexOf(";") == -1) // no semi's at all
> + {
> return sql;
> }
>
> + for(int i=0; i<query.length(); i++)
> + {
> + oneChar = query.charAt(i);
> +
> + if((oneChar == ';' && openTick == 0 && openQuote != 1) || (oneChar == ';' && openQuote == 0 && openTick
!=1))
> + {
> + throw new SQLException("Found an external SEMICOLON!! at i = "+i);
> + }
> +
> + if(oneChar == '\'') //is a '
> + {
> +
> + System.out.println("in '");
> + int j = i;
> + if(++j < query.length())
> + {
> + if(query.charAt(j) == '\'') //is an escape
> + {
> + i++; //skip them
> + continue;
> + }
> + else //genuine tick!
> + {
> + if(openTick == 0)
> + {
> + openTick = 1;
> + }
> + else if(openTick == 1)
> + {
> + openTick = 0;
> + }
> + else if(openTick == 2) //initialise it
> + {
> + openTick = 1;
> + }
> + }
> + }
> + }
> +
> +
> + if(oneChar == '"') //is a "
> + {
> + System.out.println("in \"");
> + int j = i;
> + if(++j < query.length())
> + {
> + if(query.charAt(j) == '"') //is an escape
> + {
> + i++; //skip them
> + continue;
> + }
> + else //genuine quote!
> + {
> + if(openQuote == 0)
> + {
> + openQuote = 1;
> + }
> + else if(openQuote == 1)
> + {
> + openQuote = 0;
> + }
> + else if(openQuote == 2) //initialise it
> + {
> + openQuote = 1;
> + }
> + }
> + }
> + }
> +
> +
> + }
> +
> +
> +
> +
> + //------Addition ends
> +
> +
> +
> + return sql;
> }
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
> /bjm/diff
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> 7bit
>
>
> ------------------------------------------------------------------------
> Part 1.3
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> binary
>
>
Arsalan, Why do you consider semi colons outside of quotes to be an error condition? It is legal under postgresql to execute multiple SQL statements under one call to the server. For example you can execute: update foo set bar = ?; select * from foo; In one call to the server by setting the 'query' on the Statement or Prepared statement object to be the entire string above. As you can see from this example it is certainly therefore legal to have a semi-colon outside of quotes. As to your second point about escapeSQL escaping quotes, I don't see anywhere in the JDBC spec that says that type of behavior is supposed to happen. Do you have an example where you feel the behavior of the escapeSQL method isn't conforming to the JDBC spec with regards to quote handling? thanks, --Barry
On Mon, 2 Jul 2001, Barry Lind wrote: > Arsalan, > > Why do you consider semi colons outside of quotes to be an error condition? > > It is legal under postgresql to execute multiple SQL statements under > one call to the server. For example you can execute: > > update foo set bar = ?; select * from foo; This behaviour is also legal for Informix databases. Peter Wiley
Arsalan,
First your assumption of what is meant by 'escape processing' in the
JDBC java docs is incorrect. If you read through the JDBC specs on the
sun site you will see that escape processing is specifically dealing
with parsing special constructs that are all of the form: {keyword
...parameters...} (note the requirement for the curly braces). It has
nothing to do with quotes, semi-colons or other special characters.
Now to your situation. The obvious solution is to use prepared
statements in your middle tier having the client provide the values from
the text fields to be bound into the query.
> "update users set name='"+name+"'"
So the client sends you the 'name' value and the middle tier uses a
preparedstatement to bind that value to the update.
I am assuming that the client program isn't generating the entire SQL
statement itself and sending that to the middle tier as that would open
up all sorts of security problems with non-authorized clients sending
whatever SQL they wanted to the middle tier to execute.
If for some reason you really can't use prepared statements, then it is
the responsibility of the code that is building the SQL string to
correctly deal with issues of escaping special characters that might be
in the value of a variable.
thanks,
--Barry
Arsalan Zaidi wrote:
>>Why do you consider semi colons outside of quotes to be an error
>>
> condition?
>
>>It is legal under postgresql to execute multiple SQL statements under
>>one call to the server. For example you can execute:
>>
>>update foo set bar = ?; select * from foo;
>>
>
> I agree that it probably is. Unfortunately, as I mentioned in my previous
> post,no one is going to be entering in SQL queries directly through our
> interface. We're not doing it and if someone is, he's up to no good. e.g.
>
> "update users set name='"+name+"'"
>
> If there's a text box in which you enter your name for the first query, I
> can some fun if I enter in
>
> Arsalan'';update funds set money = 100000 where userid =10
>
> Am I right? It's situations like these that I'm trying to prevent. Is there
> a better way?
>
> The client will not be under our control. We're providing an API in the
> middle which is called by our resellers. We have to perform some checking on
> our side.
>
>
>>As to your second point about escapeSQL escaping quotes, I don't see
>>anywhere in the JDBC spec that says that type of behavior is supposed to
>>happen. Do you have an example where you feel the behavior of the
>>escapeSQL method isn't conforming to the JDBC spec with regards to quote
>>handling?
>>
>
> In the JDBC docs (from the Sun docs, java.sql -> Statement), it says that
> setEscapeProcessing() will turn on the auto escaping of queries. In the
> code, setEscapeProcessing sets escapeProcessing to true. In executeQuery, if
> escapeProcessing is true, connection.escapeSQL is called. In
> connection.escapeSQL, the string's quotes are *not* escaped. In fact, if I
> remember in an earlier query, all there was, was a lone return sql;
> statement.
>
> I'm not familiar with the specs, so I could be wrong, but going on the docs
> available to me, it seems to me that connection.escapeSQL should do more
> than it's doing...
>
> Thanks for listening! :-)
>
> --Arsalan.
>
>
>
Similar problems can arise even if the end user has no bad intentions. In the example Arsanal mentioned, what if Sinead O'Connor is a user? Wouldn't this result in the following query? update users set name='Sinead O'Connor which gives the error: ERROR: parser: parse error at or near "connor" So I guess a solution would be to escape *quotes* and not *semicolons out of quotes*, which is the solution I use in my programs and on which comments are invited . This also prevents the malicious use Arsanal is talking about, doesn't it? However the PreparedStatement solution (which I haven't tried) seems to be more elegant. This thing seems pretty elementary. Isn't it covered in a FAQ somewhere? ----- Original Message ----- From: Barry Lind <barry@xythos.com> To: Arsalan Zaidi <azaidi@directi.com> Cc: PostgreSQL jdbc list <pgsql-jdbc@postgresql.org> Sent: Tuesday, July 03, 2001 6:18 PM Subject: Re: [INTERFACES] New code for JDBC driver > > Now to your situation. The obvious solution is to use prepared > statements in your middle tier having the client provide the values from > the text fields to be bound into the query. > > > "update users set name='"+name+"'" > > So the client sends you the 'name' value and the middle tier uses a > preparedstatement to bind that value to the update. > > Arsalan Zaidi wrote: > > > I agree that it probably is. Unfortunately, as I mentioned in my previous > > post,no one is going to be entering in SQL queries directly through our > > interface. We're not doing it and if someone is, he's up to no good. e.g. > > > > "update users set name='"+name+"'" > > > > If there's a text box in which you enter your name for the first query, I > > can some fun if I enter in > > > > Arsalan';update funds set money = 100000 where userid =10 > >
* "George Koras" <gkoras@cres.gr> wrote:
| So I guess a solution would be to escape *quotes* and not *semicolons out of
| quotes*, which is the solution I use in my programs and on which comments
| are invited . This also prevents the malicious use Arsanal is talking about,
| doesn't it?
|
| However the PreparedStatement solution (which I haven't tried) seems to be
| more elegant.
|
PreparedStatement is the right solution for this. If you don't trust
your input SQL either use that or do custom escaping on before sending
the SQL to the driver.
I wouldn't like to add another performance bottleneck, especially when it is
not mandated by the spec. The JDBC driver for Sybase works the same way.
regards,
Gunnar
--
Gunnar Rønning - gunnar@polygnosis.com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
Yes, I agree that it certainly has to be done before SQL is sent to the driver, i.e. in the middle tier! Is it a performance bottleneck? Would PreparedStatement be more efficient? ----- Original Message ----- From: Gunnar Rψnning <gunnar@polygnosis.com> To: George Koras <gkoras@cres.gr> Cc: Barry Lind <barry@xythos.com>; Arsalan Zaidi <azaidi@directi.com>; PostgreSQL jdbc list <pgsql-jdbc@postgresql.org> Sent: Thursday, July 05, 2001 1:13 PM Subject: Re: [JDBC] Re: [INTERFACES] New code for JDBC driver > * "George Koras" <gkoras@cres.gr> wrote: > > | So I guess a solution would be to escape *quotes* and not *semicolons out of > | quotes*, which is the solution I use in my programs and on which comments > | are invited . This also prevents the malicious use Arsanal is talking about, > | doesn't it? > | > | However the PreparedStatement solution (which I haven't tried) seems to be > | more elegant. > | > > PreparedStatement is the right solution for this. If you don't trust > your input SQL either use that or do custom escaping on before sending > the SQL to the driver. > > I wouldn't like to add another performance bottleneck, especially when it is > not mandated by the spec. The JDBC driver for Sybase works the same way. > > regards, > > Gunnar > -- > Gunnar Rψnning - gunnar@polygnosis.com > Senior Consultant, Polygnosis AS, http://www.polygnosis.com/ >
* "George Koras" <gkoras@cres.gr> wrote: | | Yes, I agree that it certainly has to be done before SQL is sent to the | driver, i.e. in the middle tier! | Is it a performance bottleneck? Would PreparedStatement be more efficient? | Depends on your application, but doing escaping on all queries would certainly increase the overhead for some queries. With PreparedStatement you only escape the part of the query that is string input, so, yes, it would be more efficient. -- Gunnar Rønning - gunnar@polygnosis.com Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
> > Why do you consider semi colons outside of quotes to be an error condition? > > It is legal under postgresql to execute multiple SQL statements under > one call to the server. For example you can execute: > > update foo set bar = ?; select * from foo; I agree that it probably is. Unfortunately, as I mentioned in my previous post,no one is going to be entering in SQL queries directly through our interface. We're not doing it and if someone is, he's up to no good. e.g. "update users set name='"+name+"'" If there's a text box in which you enter your name for the first query, I can some fun if I enter in Arsalan'';update funds set money = 100000 where userid =10 Am I right? It's situations like these that I'm trying to prevent. Is there a better way? The client will not be under our control. We're providing an API in the middle which is called by our resellers. We have to perform some checking on our side. > > As to your second point about escapeSQL escaping quotes, I don't see > anywhere in the JDBC spec that says that type of behavior is supposed to > happen. Do you have an example where you feel the behavior of the > escapeSQL method isn't conforming to the JDBC spec with regards to quote > handling? In the JDBC docs (from the Sun docs, java.sql -> Statement), it says that setEscapeProcessing() will turn on the auto escaping of queries. In the code, setEscapeProcessing sets escapeProcessing to true. In executeQuery, if escapeProcessing is true, connection.escapeSQL is called. In connection.escapeSQL, the string's quotes are *not* escaped. In fact, if I remember in an earlier query, all there was, was a lone return sql; statement. I'm not familiar with the specs, so I could be wrong, but going on the docs available to me, it seems to me that connection.escapeSQL should do more than it's doing... Thanks for listening! :-) --Arsalan.
> I agree that it probably is. Unfortunately, as I mentioned in my previous
> post,no one is going to be entering in SQL queries directly through our
> interface. We're not doing it and if someone is, he's up to no good. e.g.
>
> "update users set name='"+name+"'"
>
> If there's a text box in which you enter your name for the first query, I
> can some fun if I enter in
>
> Arsalan'';update funds set money = 100000 where userid =10
>
> Am I right? It's situations like these that I'm trying to prevent. Is there
> a better way?
If I'm not using prepared statements to tend to use something like:
String statement = "update users set name='" + encode(name) + "'";
Where encode is defined as:
/**
* Encode a string suitable for being placed in a query.
*/
final String encode(String dirtyString) {
StringBuffer cleanString = new StringBuffer("");
for (int i = 0; i < dirtyString.length(); i++) {
char c = dirtyString.charAt(i);
cleanString.append(c);
if (c == '\'') {
cleanString.append(c);
}
}
return cleanString.toString();
}
Hope this helps,
Michael Stephenson