Re: Patch for possible PSQLException bug

Tarjei,

Thanks for finding and fixing. This one has been bugging me for a
while, but I haven't ever had the patience to track down what was the
cause. I will apply your patch later today.

thanks,
--Barry

Tarjei Skorgenes wrote:
> After trying the new SSL functionality in the driver I stumbled upon the
> following bug in PSQLException:
>
> The no-parameter constructor of SQLException is called at the beginning of every
> constructor in PSQLException. This can lead to a NullPointerException
> if the user has set the loglevel to something > 0. When loglevel is
> greater than 0 Driver.java calls DriverManager.setLogWriter(System.out).
> All messages are printed to this logwriter. The no-parameter
> constructor for SQLException prints the stacktrace of the current
> exception to this logwriter whenever it is instantiated. Since
> PSQLException inherits from SQLException the toString method of
> PSQLException will return null since the translate method won't have
> been called yet.
>
> As a consequence an attempt will be made to print the return value from
> toString() to the logwriter and this will result in a
> NullPointerException being thrown. This might be categorized as a bug in
> PrintWriter but any user trying to use the jdbc driver with loglevel > 0
> might stumble upon it.
>
> I've cooked up a JUnit testcase that checks for the error and I've made a
> simple one-liner patch to get toString() to return "" if message hasn't been
> initialized yet.
>
> --
> Tarjei Skorgenes
> tarjei(dot)skorgenes(at)himolde(dot)no
>
>
> ------------------------------------------------------------------------
>
> diff -uNr jdbc/org/postgresql/util/PSQLException.java jdbc-new/org/postgresql/util/PSQLException.java
> --- jdbc/org/postgresql/util/PSQLException.java 2003-02-27 06:45:44.000000000 +0100
> +++ jdbc-new/org/postgresql/util/PSQLException.java 2003-02-27 15:19:41.000000000 +0100
> @@ -111,6 +111,6 @@
> */
> public String toString()
> {
> - return message;
> + return message != null ? message : "";
> }
> }
>
>
> ------------------------------------------------------------------------
>
> diff -uNr jdbc/build.xml jdbc-new/build.xml
> --- jdbc/build.xml 2003-02-27 06:45:43.000000000 +0100
> +++ jdbc-new/build.xml 2003-02-27 15:24:06.000000000 +0100
> @@ -50,6 +50,9 @@
> <available property="datasource" classname="javax.sql.DataSource"/>
> <available property="ssl" classname="javax.net.ssl.SSLSocketFactory"/>
> <available property="junit" classname="junit.framework.Test" />
> + <condition property="utiltests">
> + <isset property="junit"/>
> + </condition>
> <condition property="jdbc2tests">
> <and>
> <isset property="jdbc2"/>
> @@ -278,8 +281,21 @@
> <property name="junit.ui" value="textui" />
>
>
> - <target name="test" depends="testjdbc2,testjdbc2optional,testjdbc3">
> - </target>
> + <target name="test" depends="testutil,testjdbc2,testjdbc2optional,testjdbc3">
> + </target>
> +
> + <target name="testutil" depends="jar" if="utiltests">
> + <javac srcdir="${srcdir}" destdir="${builddir}" debug="${debug}">
> + <include name="${package}/test/util/*" />
> + </javac>
> + <java fork="yes" classname="junit.${junit.ui}.TestRunner" taskname="junit" failonerror="true">
> + <arg value="org.postgresql.test.util.UtilTestSuite" />
> + <classpath>
> + <pathelement location="${builddir}" />
> + <pathelement path="${java.class.path}" />
> + </classpath>
> + </java>
> + </target>
>
> <target name="testjdbc2" depends="jar" if="jdbc2tests">
> <javac srcdir="${srcdir}" destdir="${builddir}" debug="${debug}">
> diff -uNr jdbc/org/postgresql/test/util/TestPSQLException.java jdbc-new/org/postgresql/test/util/TestPSQLException.java
> --- jdbc/org/postgresql/test/util/TestPSQLException.java 1970-01-01 01:00:00.000000000 +0100
> +++ jdbc-new/org/postgresql/test/util/TestPSQLException.java 2003-02-27 15:24:07.000000000 +0100
> @@ -0,0 +1,84 @@
> +package org.postgresql.test.util;
> +
> +import java.io.*;
> +import java.sql.*;
> +import junit.framework.TestCase;
> +import org.postgresql.util.PSQLException;
> +
> +public class TestPSQLException extends TestCase {
> + private PrintWriter oldWriter;
> + private MockWriter mockWriter;
> +
> + private static class MockWriter extends PrintWriter {
> + static class BogusWriter extends Writer {
> + public void flush() {}
> + public void close() {}
> + public void write(char[] cbuf, int off, int len) {}
> + }
> +
> + public MockWriter() {
> + super(new BogusWriter());
> + out = this;
> + }
> +
> + private boolean isNull = false;
> + private int methodCallNr = 1;
> +
> + // This method gets called from SQLExceptions constructor
> + // with the value of PSQLException.toString() as a parameter.
> + // We're only interested in the first call since that is
> + // the one containing the toString value
> + public void write(String val) {
> + if ((methodCallNr == 1) && (val == null)) {
> + isNull = true;
> + }
> +
> + methodCallNr++;
> + }
> +
> + public boolean isNull() {
> + return this.isNull;
> + }
> + }
> +
> + private void setLogWriter(PrintWriter pw) {
> + DriverManager.setLogWriter(pw);
> + }
> +
> + public TestPSQLException(String name) {
> + super(name);
> + }
> +
> + public void setUp() {
> + // Take a backup of the old logWriter
> + oldWriter = DriverManager.getLogWriter();
> +
> + // Set a new logwriter
> + mockWriter = new MockWriter();
> + setLogWriter(mockWriter);
> + }
> +
> + public void tearDown() {
> + // Restore the old logwriter
> + setLogWriter(oldWriter);
> +
> + oldWriter = null;
> + mockWriter = null;
> + }
> +
> + /**
> + * Checks if PSQLException.toString() returns null if it gets called from one
> + * of the constructors. This is checked by testing if an attempt was made to
> + * write a null value to the MockWriter
> + */
> + public void testToStringNotNullInConstructor() {
> + PSQLException ex = new PSQLException("Bogus exception", new Exception());
> +
> + // Did toString return null?
> + boolean isNull = mockWriter.isNull();
> +
> + boolean expected = false;
> + boolean actual = isNull;
> + assertEquals("toString should not return null if called from the constructor", expected, actual);
> + }
> +}
> diff -uNr jdbc/org/postgresql/test/util/UtilTestSuite.java jdbc-new/org/postgresql/test/util/UtilTestSuite.java
> --- jdbc/org/postgresql/test/util/UtilTestSuite.java 1970-01-01 01:00:00.000000000 +0100
> +++ jdbc-new/org/postgresql/test/util/UtilTestSuite.java 2003-02-27 15:24:27.000000000 +0100
> @@ -0,0 +1,24 @@
> +package org.postgresql.test.util;
> +
> +import junit.framework.TestSuite;
> +import junit.framework.TestCase;
> +import junit.framework.Test;
> +
> +/*
> + * Executes all known tests for classes in org.postgresql.util
> + */
> +public class UtilTestSuite extends TestSuite
> +{
> + /*
> + * The main entry point for JUnit
> + */
> + public static TestSuite suite()
> + {
> + TestSuite suite = new TestSuite();
> +
> + // PSQLException tests
> + suite.addTestSuite(TestPSQLException.class);
> +
> + return suite;
> + }
> +}
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html