Bug #16791 NPE in MysqlDataSourceFactory
Submitted: 25 Jan 2006 22:02 Modified: 26 Jul 2006 18:17
Reporter: Maz Rashid Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / J Severity:S3 (Non-critical)
Version:All OS:Any (All)
Assigned to: CPU Architecture:Any

[25 Jan 2006 22:02] Maz Rashid
Description:
There is a minor but cumbersome bug in the class com.mysql.jdbc.jdbc2.optional.MysqlDataSourceFactory:

All parameter passed by the Reference object (ref) may be null. Thus it is not needed to pass server and port if the url is provided.

But the implementation has several lines like:

String portNumberAsString = (String) ref.get("port").getContent();

where ref.get("port") may be null (no option delivered).
At this point this class generates a NullPointerException.

Please have a look how Oracle or Apache DBCP is handling the DataSourceFactory.

Best regards
Maz

How to repeat:
just provide a ref with missing parameter. Only add url and explicitUrl.
(BTW: Nobody else is using explicitUrl as pre requisite for setting an url)

Suggested fix:
String portNumberAsString = (String)ref.get("port")==null?null: ref.get("port").getContent();

And same for the other parameters.
[25 Jan 2006 22:22] Mark Matthews
The explicitURL field is because the datasource has to determine whether you've _specified_ a URL via setURL(), or whether it's creating one based on properties you've set on the datasource.
[25 Jan 2006 22:38] Maz Rashid
To be honest, this should be an internal state. You should set it at the moment the url parameter is provided.

But this is not the topic of the bug report. More important are the *NPEs*.
As it is now all parameter has to be provided.
[25 Jan 2006 22:48] Mark Matthews
explicitURL _is_ an internal property. There are no publicly-visible accessors or mutators for it (it does get passed as a reference in case the datasource gets "serialized" though), so I don't get where you're coming from, sorry.
[26 Jan 2006 13:40] Maz Rashid
Dear Mark,

thnaks for taking the time for this discussion. great.
The datasource works fine, but I am talking about the DataSourceFactory.
There you have this block:

			String explicitUrlAsString = (String) ref.get("explicitUrl")
					.getContent();

			if (explicitUrlAsString != null) {
				if (Boolean.valueOf(explicitUrlAsString).booleanValue()) {
					dataSource.setUrl((String) ref.get("url").getContent());
				}
			}

where the parameter explicitUrl has to be present in the Reference inorder to set the url.

I just changed the source and attached my suggestion to this comment. It solves the NPE problem and does not contain the explicitURL as external parameter. I hope this will help.

Thanks a lot for your efforts
Best regards
Maz
[10 Feb 2006 6:51] Mark Kirkwood
I just ran into this with 5.0.0 beta. There seem to be two additional problems:

1) "username" property is never picked up by MysqlDataSourceFactory. This seems to be because NonRegisteringDriver.java defines:

USER_PROPERTY_KEY = "user" 

I suspect this should be:

USER_PROPERTY_KEY = "username"

2) "url" is not (always) picked up by MysqlDataSourceFactory - this seems to be because the code essentially does:

ref.get("url") only if ref.get("explicitUrl") != null
.
[10 Feb 2006 8:38] Mark Kirkwood
FWIW - here is the patch I used to get it working for me (this makes the variable names etc consistent with the documentation for using the connector with tomcat). I expect that there are much cleaner ways to sort this out! but hopefully this is useful. best wishes!

---patch here--->
--- src/com/mysql/jdbc/jdbc2/optional/MysqlDataSourceFactory.java.orig  Fri Feb 10 21:18:21 2006
+++ src/com/mysql/jdbc/jdbc2/optional/MysqlDataSourceFactory.java   Fri Feb 10 21:18:52 2006
@@ -84,7 +84,8 @@

            int portNumber = 3306;

-           String portNumberAsString = (String) ref.get("port").getContent();
+           String portNumberAsString = (String) (ref.get("port")==null?null:
+               ref.get("port").getContent());

            if (portNumberAsString != null) {
                portNumber = Integer.parseInt(portNumberAsString);
@@ -92,39 +93,61 @@

            dataSource.setPort(portNumber);

-           String user = (String) ref.get(
-                   NonRegisteringDriver.USER_PROPERTY_KEY).getContent();
+           String user = (String) (ref.get(
+                   NonRegisteringDriver.USER_PROPERTY_KEY)==null?null: 
+                   ref.get(
+                   NonRegisteringDriver.USER_PROPERTY_KEY).getContent());
+

            if (user != null) {
                dataSource.setUser(user);
            }

-           String password = (String) ref.get(
-                   NonRegisteringDriver.PASSWORD_PROPERTY_KEY).getContent();
+           String password = (String) (ref.get(
+                   NonRegisteringDriver.PASSWORD_PROPERTY_KEY)==null?null:
+                   ref.get(
+                   NonRegisteringDriver.PASSWORD_PROPERTY_KEY).getContent());

            if (password != null) {
                dataSource.setPassword(password);
            }

-           String serverName = (String) ref.get("serverName").getContent();
+           String serverName = (String) (ref.get("serverName")==null?null:
+                   ref.get("serverName").getContent());

            if (serverName != null) {
                dataSource.setServerName(serverName);
            }

-           String databaseName = (String) ref.get("databaseName").getContent();
+           String databaseName = (String) (ref.get("databaseName")==null?null:
+                   ref.get("databaseName").getContent());

            if (databaseName != null) {
                dataSource.setDatabaseName(databaseName);
            }

-           String explicitUrlAsString = (String) ref.get("explicitUrl")
-                   .getContent();
+           String explicitUrlAsString = (String) (ref.get("explicitUrl")==null?null:
+                   ref.get("explicitUrl").getContent());

            if (explicitUrlAsString != null) {
                if (Boolean.valueOf(explicitUrlAsString).booleanValue()) {
                    dataSource.setUrl((String) ref.get("url").getContent());
                }
+           }
+
+           String theUrl = null;
+
+           if (explicitUrlAsString == null ) {
+
+               theUrl = (String) (ref.get("url")==null?null: 
+                       ref.get("url").getContent());
+               if (theUrl != null) {
+                   dataSource.setUrl(theUrl);
+               }
+           }
+
+           if (theUrl == null && explicitUrlAsString == null && databaseName == null) {
+               throw new RuntimeException("All three are null!!!");
            }

            dataSource.setPropertiesViaRef(ref);
--- src/com/mysql/jdbc/NonRegisteringDriver.java.orig   Fri Feb 10 21:19:52 2006
+++ src/com/mysql/jdbc/NonRegisteringDriver.java    Fri Feb 10 21:20:05 2006
@@ -110,7 +110,7 @@
     * Key used to retreive the username value from the properties instance
     * passed to the driver.
     */
-   public static final String USER_PROPERTY_KEY = "user";
+   public static final String USER_PROPERTY_KEY = "username";

    /**
     * Gets the drivers major version number
[16 Jun 2006 19:37] Mark Matthews
> I just ran into this with 5.0.0 beta. There seem to be two additional problems:
> 
> 1) "username" property is never picked up by MysqlDataSourceFactory. This seems
> to be because NonRegisteringDriver.java defines:
> 
> USER_PROPERTY_KEY = "user" 
> 
> I suspect this should be:
> 
> USER_PROPERTY_KEY = "username"

It shouldn't, as the JDBC specfication states that the name of this standard property for datasources is "user" on page 12 of the JDBC-2.0 standard extensions specification.
[16 Jun 2006 20:23] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/7781