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

From: Nathaniel Waisbrot <waisbrot(at)highfleet(dot)com>
To: Dave Cramer <pg(at)fastcrypt(dot)com>
Cc: dmp <danap(at)ttc-cmc(dot)net>, List <pgsql-jdbc(at)postgresql(dot)org>
Subject: Re: Bug report: NullPointerException from Driver.connect when passed a Properties with non-string values
Date: 2013-01-28 20:50:08
Message-ID: A0A5FDF1-1C2A-41B0-9911-BA5D5070303F@highfleet.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-jdbc

That's a good point. At least in my case, the URL I'm passing in isn't even for PG so it shouldn't need to care that the Properties are set up with weird non-String values.

I think this is a good patch.

You could add mine in, too, just to conform a little more rigidly to the JDBC spec (which says to throw SQLException if the driver wants to handle the URL but can't because of an error). But once the driver is only throwing exceptions for its own URLs, the problem becomes quite minor, I think.

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

> 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
> >
>
>
>
>

In response to

Browse pgsql-jdbc by date

  From Date Subject
Next Message Kevin Grittner 2013-01-28 20:53:52 Re: Bug report: NullPointerException from Driver.connect when passed a Properties with non-string values
Previous Message dmp 2013-01-28 20:43:46 Re: Bug report: NullPointerException from Driver.connect when passed a Properties with non-string values