Re: Patch for possible PSQLException bug - Mailing list pgsql-jdbc

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.skorgenes@himolde.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