Thread: Patch for possible PSQLException bug

Patch for possible PSQLException bug

From
Tarjei Skorgenes
Date:
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

Attachment

Re: Patch for possible PSQLException bug

From
Barry Lind
Date:
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