Derby
  1. Derby
  2. DERBY-4815

Override mechanism for modules.properties works backwards

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: 10.7.1.1
    • Component/s: Services
    • Labels:
      None

      Description

      It is possible to override the properties in org/apache/derby/modules.properties by putting your own version of it somewhere on the classpath. BaseMonitor.getDefaultModuleProperties() apparently intends to use values from the first modules.properties file that mentions a property:

      String key = (String) newKeys.nextElement();
      if( moduleList.contains( key))
      // RESOLVE how do we localize messages before we have finished initialization?
      report( "Ignored duplicate property " + key + " in " + modulesPropertiesURL.toString());
      else
      moduleList.setProperty( key, otherList.getProperty( key));

      However, moduleList.contains(key) doesn't look for a key in moduleList, it looks for a property value. This code should have used containsKey() instead.

      Beacuse of this, the last modules.properties on the classpath will take precedence over the ones earlier on the classpath. So if you for example have two different versions of derby.jar in the classpath, the engine will use the classes from the first jar and modules.properties from the last jar.

      1. d4815.diff
        0.9 kB
        Knut Anders Hatlen

        Activity

        Knut Anders Hatlen created issue -
        Knut Anders Hatlen made changes -
        Field Original Value New Value
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Knut Anders Hatlen added a comment -

        Here's a patch that changes the call from contains(key) to containsKey(key) so that the code works as intended. I couldn't think of a good regression case for this bug, since we don't currently have any modules whose different implementations have user-visible differences.

        I have verified, though, that if I boot a debug version of Derby when I have multiple versions of derby.jar in the classpath, I now get messages like these printed to derby.log:

        Tue Oct 26 11:28:50 CEST 2010 Thread[main,5,main] Ignored duplicate property derby.module.dataDictionary in jar:file:/code/derby/trunk0/jars/sane/derby.jar!/org/apache/derby/modules.properties
        Tue Oct 26 11:28:50 CEST 2010 Thread[main,5,main] Ignored duplicate property derby.module.lockManagerJ1 in jar:file:/code/derby/trunk0/jars/sane/derby.jar!/org/apache/derby/modules.properties
        (...)

        Without the fix, instead of ignoring the duplicate properties, it would have silently used the property values from the last derby.jar in the classpath, whereas the classes would have been loaded from the first derby.jar in the classpath.

        All the regression tests ran cleanly with the patch.

        Show
        Knut Anders Hatlen added a comment - Here's a patch that changes the call from contains(key) to containsKey(key) so that the code works as intended. I couldn't think of a good regression case for this bug, since we don't currently have any modules whose different implementations have user-visible differences. I have verified, though, that if I boot a debug version of Derby when I have multiple versions of derby.jar in the classpath, I now get messages like these printed to derby.log: Tue Oct 26 11:28:50 CEST 2010 Thread [main,5,main] Ignored duplicate property derby.module.dataDictionary in jar: file:/code/derby/trunk0/jars/sane/derby.jar!/org/apache/derby/modules.properties Tue Oct 26 11:28:50 CEST 2010 Thread [main,5,main] Ignored duplicate property derby.module.lockManagerJ1 in jar: file:/code/derby/trunk0/jars/sane/derby.jar!/org/apache/derby/modules.properties (...) Without the fix, instead of ignoring the duplicate properties, it would have silently used the property values from the last derby.jar in the classpath, whereas the classes would have been loaded from the first derby.jar in the classpath. All the regression tests ran cleanly with the patch.
        Knut Anders Hatlen made changes -
        Attachment d4815.diff [ 12458060 ]
        Knut Anders Hatlen made changes -
        Issue & fix info [Patch Available]
        Hide
        Dag H. Wanvik added a comment -

        Looks right to me. +1

        Show
        Dag H. Wanvik added a comment - Looks right to me. +1
        Hide
        Knut Anders Hatlen added a comment -

        Thanks for looking at the patch, Dag.
        Committed revision 1027847.

        Show
        Knut Anders Hatlen added a comment - Thanks for looking at the patch, Dag. Committed revision 1027847.
        Knut Anders Hatlen made changes -
        Issue & fix info [Patch Available]
        Fix Version/s 10.7.1.0 [ 12314971 ]
        Resolution Fixed [ 1 ]
        Status In Progress [ 3 ] Resolved [ 5 ]
        Rick Hillegas made changes -
        Fix Version/s 10.7.1.1 [ 12315564 ]
        Fix Version/s 10.7.1.0 [ 12314971 ]
        Knut Anders Hatlen made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Gavin made changes -
        Workflow jira [ 12521357 ] Default workflow, editable Closed status [ 12800470 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        32d 16h 11m 1 Knut Anders Hatlen 26/Oct/10 10:15
        In Progress In Progress Resolved Resolved
        22h 26m 1 Knut Anders Hatlen 27/Oct/10 08:41
        Resolved Resolved Closed Closed
        78d 9h 36m 1 Knut Anders Hatlen 13/Jan/11 17:18

          People

          • Assignee:
            Knut Anders Hatlen
            Reporter:
            Knut Anders Hatlen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development