Derby
  1. Derby
  2. DERBY-1998

JDBC 4 isWrapper() and unWrap() methods allow unwrapping to Derby implementation classes.

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 10.2.1.6
    • Fix Version/s: None
    • Component/s: JDBC
    • Urgency:
      Normal

      Description

      I do not believe that the unwrapping mechanism of JDBC 4 should allow or encourage unwrapping a JDBC 4 object as one of Derby's pribvate implementation types.

      Since Derby does not wrap objects in any way I would say there is a stong case for saying that the isWrapper() method should always return false and the unWrap() method always throw an exception.

      For example in EmbedStatement40 this code will allow "unwrapping" to
      EmbedStatement40, EmbedStatement, EngineStatement, ConnectionChild, Object, Statement

      public boolean isWrapperFor(Class<?> interfaces) throws SQLException

      { checkStatus(); return interfaces.isInstance(this); }

      public <T> T unwrap(java.lang.Class<T> interfaces)
      throws SQLException{
      checkStatus();
      try

      { return interfaces.cast(this); }

      catch (ClassCastException cce)

      { throw newSQLException(SQLState.UNABLE_TO_UNWRAP,interfaces); }

      }

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        The javadoc for isWrapperFor() says
        (http://download.java.net/jdk6/docs/api/java/sql/Wrapper.html)

        > Returns true if this either implements the interface argument or is
        > directly or indirectly a wrapper for an object that does. Returns
        > false otherwise.

        I believe this means that always returning false is not compliant
        behaviour. However, I believe we could return false when the iface
        argument isn't an interface since the description of the iface
        argument says "a Class defining an interface." (Although it doesn't
        say explicitly that "interface" means Java language interface.)

        If EmbedStatement40.isWrapperFor() were implemented as

        public boolean isWrapperFor(Class<?> interfaces) throws SQLException

        { checkStatus(); return interfaces.isInterface() && interfaces.isInstance(this); }

        it would only return true for Wrapper, Statement and EngineStatement.

        I'm not sure I understand why this is flagged as a critical issue
        (which JIRA says should be used for crashes, loss of data and severe
        memory leak). Any user could just as easily get access to the same
        implementation classes with a cast

        EmbedStatement es = (EmbedStatement) stmt;

        which is no harder than

        EmbedStatement es = stmt.unwrap(EmbedStatement.class);

        If we think this is a critical issue, wouldn't it be better to ensure
        that the implementation classes that we return to the users are
        package protected? Then neither of the above constructs could be used.

        Show
        Knut Anders Hatlen added a comment - The javadoc for isWrapperFor() says ( http://download.java.net/jdk6/docs/api/java/sql/Wrapper.html ) > Returns true if this either implements the interface argument or is > directly or indirectly a wrapper for an object that does. Returns > false otherwise. I believe this means that always returning false is not compliant behaviour. However, I believe we could return false when the iface argument isn't an interface since the description of the iface argument says "a Class defining an interface." (Although it doesn't say explicitly that "interface" means Java language interface.) If EmbedStatement40.isWrapperFor() were implemented as public boolean isWrapperFor(Class<?> interfaces) throws SQLException { checkStatus(); return interfaces.isInterface() && interfaces.isInstance(this); } it would only return true for Wrapper, Statement and EngineStatement. I'm not sure I understand why this is flagged as a critical issue (which JIRA says should be used for crashes, loss of data and severe memory leak). Any user could just as easily get access to the same implementation classes with a cast EmbedStatement es = (EmbedStatement) stmt; which is no harder than EmbedStatement es = stmt.unwrap(EmbedStatement.class); If we think this is a critical issue, wouldn't it be better to ensure that the implementation classes that we return to the users are package protected? Then neither of the above constructs could be used.
        Hide
        Daniel John Debrunner added a comment -

        I think there's a slight difference between these two.

        EmbedStatement es = (EmbedStatement) stmt;

        EmbedStatement es = stmt.unwrap(EmbedStatement.class);

        The first is just how Java works.

        The second is Derby's driver api indicating that the returned statement objcet is a wrapper for EmbedStatement.
        That is not guaranteed by Derby either in a current release or in any future release, e.g. the object used to implement java.sql.Statement could change in the next release, even on a branch. So there is a very minor chance that an application could come to rely on it since its information obtained from Derby's driver.

        I'll drop this to major, I think that some of the changes suggested are good, just relying on interfaces.

        Maybe something like

        for Derby objects that implement Statement.

        ireturn clazz == java.sql.Statement.class;

        for Derby objects that implement PreparedStatement.

        return clazz == java.sql.Statement.class || clazz == java.sql.PreparedStatement.class

        I agree it would be good to get EmbedStatement and other similar objects not to be public.

        Show
        Daniel John Debrunner added a comment - I think there's a slight difference between these two. EmbedStatement es = (EmbedStatement) stmt; EmbedStatement es = stmt.unwrap(EmbedStatement.class); The first is just how Java works. The second is Derby's driver api indicating that the returned statement objcet is a wrapper for EmbedStatement. That is not guaranteed by Derby either in a current release or in any future release, e.g. the object used to implement java.sql.Statement could change in the next release, even on a branch. So there is a very minor chance that an application could come to rely on it since its information obtained from Derby's driver. I'll drop this to major, I think that some of the changes suggested are good, just relying on interfaces. Maybe something like for Derby objects that implement Statement. ireturn clazz == java.sql.Statement.class; for Derby objects that implement PreparedStatement. return clazz == java.sql.Statement.class || clazz == java.sql.PreparedStatement.class I agree it would be good to get EmbedStatement and other similar objects not to be public.
        Hide
        Knut Anders Hatlen added a comment -

        Triaged for 10.5.2.

        Show
        Knut Anders Hatlen added a comment - Triaged for 10.5.2.

          People

          • Assignee:
            Unassigned
            Reporter:
            Daniel John Debrunner
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development