Re: Bug report: NullPointerException from Driver.connect when passed a Properties with non-string values

My proposed patch is this

if (url.startsWith("jdbc:postgresql:")) {
return null;
}

Right at the top of getConnection, before we do any real work.

Thoughts ?

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca

On Mon, Jan 28, 2013 at 3:23 PM, Dave Cramer <pg(at)fastcrypt(dot)com> wrote:

> Nathaniel,
>
> It's ironic to note that Properties.getProperty() returns null even if
> there is a value in the hashtable for the key.
>
> It suggests that it will only return a string.
>
> Either way the solution is to check to make sure the url is for us before
> loading up the properties.
>
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
> On Mon, Jan 28, 2013 at 2:43 PM, Nathaniel Waisbrot <
> waisbrot(at)highfleet(dot)com> wrote:
>
>> Thanks for the responses, dmp and Dave.
>> I agree that it's rather surprising to be getting non-String values for
>> the Driver, but I don't think it's actually *required* by any spec that
>> they be Strings. The references I'm looking at are:
>>
>> The connect method of Driver, which takes a Properties:
>>
>> http://docs.oracle.com/javase/6/docs/api/java/sql/Driver.html#connect%28java.lang.String,%20java.util.Properties%29
>>
>> The Properties class:
>> http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html
>>
>> Which warns that:
>> "Because Properties inherits from Hashtable, the put and putAll
>> methods can be applied to a Properties object. Their use is strongly
>> discouraged as they allow the caller to insert entries whose keys or values
>> are not Strings. The setProperty method should be used instead. If the
>> store or save method is called on a "compromised" Properties object that
>> contains a non-String key or value, the call will fail. Similarly, the call
>> to the propertyNames or list method will fail if it is called on a
>> "compromised" Properties object that contains a non-String key."
>>
>>
>> Frustratingly, you need to actually look at the source code of Properties
>> to see some undocumented behavior of getProperty:
>>
>> public String getProperty(String key) {
>> Object oval = super.get(key);
>> String sval = (oval instanceof String) ? (String)oval : null;
>> return ((sval == null) && (defaults != null)) ?
>> defaults.getProperty(key) : sval;
>> }
>>
>>
>> Note that middle line--the reason that my example program fails is
>> because an Object is not of type String, and so getProperty is substituting
>> a null value.
>>
>> I've attached a candidate patch for you: waisbrot-exception.patch checks
>> the result of getProperty() and throws a PSQLException if it is null. This
>> allows drivers that want to deal with non-string Properties to get a turn
>> with the URL, but will cause DriverManager to report the error if no Driver
>> can perform the connection.
>>
>>
>>
>>
>>
>>
>>
>>
>> On Jan 28, 2013, at 1:40 PM, Dave Cramer <pg(at)fastcrypt(dot)com> wrote:
>>
>> > Well ... another interesting stretching of the spec.
>> >
>> > Driver PropertyInfo values are required to be Strings as per the spec.
>> http://docs.oracle.com/javase/1.3/docs/api/java/sql/DriverPropertyInfo.html
>> >
>> > Your test case is actually a bit misleading as getProperty returns null
>> because getString on the empty object returns null.
>> >
>> > while I'm inclined to accept a patch to fix this, this is clearly 'out
>> of spec'
>> >
>> > Dave Cramer
>> >
>> > dave.cramer(at)credativ(dot)ca
>> > http://www.credativ.ca
>> >
>> >
>> > On Mon, Jan 28, 2013 at 1:16 PM, dmp <danap(at)ttc-cmc(dot)net> wrote:
>> > Hello Nathaniel,
>> >
>> > Any very robust program will try to handle anything the user throws at
>> it.
>> >
>> > In this case you have passed a null value as an argument to the method
>> > and it as acted according by giving you a NUllPointerException. Perhaps
>> > wrapping this as SQLException may be more easily caught by your app.,
>> but
>> > would be deceptive in response.
>> >
>> > Perhaps you could submit a patch for consideration, to the solution you
>> > desire. The code may be obtained at GitHub.
>> >
>> > git clone git://github.com/pgjdbc/pgjdbc.git
>> >
>> > danap.
>> >
>> >
>> >
>> > Nathaniel Waisbrot wrote:
>> > I found this while working with another JDBC driver (Stels XML driver).
>> The Postgresql driver's connect() method is expecting the passed
>> Properties object to have only string values, but this is not actually
>> guaranteed to be the case. I think that the PG driver should wrap the
>> NullPointerException in a SQLException, which would allow the DriverManager
>> to attempt to use other JDBC drivers to make the connection.
>> >
>> > Here is a simple Java program which will reproduce the problem:
>> >
>> > ==================
>> > import java.sql.DriverManager;
>> > import java.util.Properties;
>> >
>> > public class PGTest {
>> > public static void main(String[] args) throws Exception {
>> > Class.forName("org.postgresql.Driver");
>> > Properties info = new Properties();
>> > info.put("foo", new Object()); //
>> info.getPropert("foo") will return null
>> > DriverManager.getConnection("foo:bar//baz", info);
>> > }
>> > }
>> > ==================
>> >
>> > and here is the stack trace produced by running that program:
>> >
>> > ==================
>> > Exception in thread "main" java.lang.NullPointerException
>> > at java.util.Hashtable.put(Hashtable.java:542)
>> > at java.util.Properties.setProperty(Properties.java:161)
>> > at org.postgresql.Driver.connect(Driver.java:244)
>> > at java.sql.DriverManager.getConnection(DriverManager.java:579)
>> > at java.sql.DriverManager.getConnection(DriverManager.java:190)
>> > at PGTest.main(PGTest.java:9)
>> > ==================
>> >
>> > I'm using postgresql-9.2-1002.jdbc4.jar, with Java 7, running on Mac OS
>> 10.8.2. Output of java -version:
>> > java version "1.7.0_07"
>> > Java(TM) SE Runtime Environment (build 1.7.0_07-b10)
>> > Java HotSpot(TM) 64-Bit Server VM (build 23.3-b01, mixed mode)
>> >
>> >
>> >
>> >
>> >
>> >
>> > --
>> > Sent via pgsql-jdbc mailing list (pgsql-jdbc(at)postgresql(dot)org)
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgsql-jdbc
>> >
>>
>>
>>
>