Re: [cp-patches] FYI: Avoid recursion in java.util.Properties

[Roman Kennke]

Hi Tom,
Tom Tromey wrote:

> Roman> This fix 'outsources' the functionality of the getProperty methods
> Roman> into a private method. This avoids infinite recursion if one of the
> Roman> getProperty methods is overridden in subclasses.
>
> While this is a totally sensible way to write code, I worry that there
> are classes out there that derive from Properties and just override
> one of these methods, "working" because they happen to match how the
> JDK delegates internally.
>
> In this situation I would recommend a test case to determine this; and
> then an @specnote or the like documenting the result.

Thank you for this hint. I wrote and checked in a testcase that checks 
which method calls which. It shows that getProperty(String, String) 
shoould call getProperty(String). We did it just the other way round. 
The attached patch fixes it. Now correctly as I hope :-)

2005-07-27  Roman Kennke  <address@hidden>

        * java/util/Properties.java
        (getPropertyInternal): Removed.
        (getProperty(String)): Search for property here instead of
        getProperty(String, String).
        (getProperty(String,String)): Call getProperty(String).

/Roman

Index: java/util/Properties.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/Properties.java,v
retrieving revision 1.31
retrieving revision 1.32
diff -u -r1.31 -r1.32
--- java/util/Properties.java   26 Jul 2005 14:25:37 -0000      1.31
+++ java/util/Properties.java   27 Jul 2005 10:42:40 -0000      1.32
@@ -398,30 +398,6 @@
 
     writer.flush ();
   }
-  
-  /**
-   *  Internal method called by getProperty() methods. This avoids 
-   *  recursive calls if getProperty() methods are overwritten in 
-   *  a subclass.
-   *
-   * @param key the key for the property to fetch
-   * @param defaultValue the defaultValue or <code>null</code> if there
-   *        is no default value
-   */
-  private String getPropertyInternal(String key, String defaultValue)
-  {
-    Properties prop = this;
-    // Eliminate tail recursion.
-    do
-      {
-        String value = (String) prop.get(key);
-        if (value != null)
-          return value;
-        prop = prop.defaults;
-      }
-    while (prop != null);
-    return defaultValue;    
-  }
 
   /**
    * Gets the property with the specified key in this property list.
@@ -438,7 +414,17 @@
    */
   public String getProperty(String key)
   {
-    return getPropertyInternal(key, null);
+    Properties prop = this;
+    // Eliminate tail recursion.
+    do
+      {
+        String value = (String) prop.get(key);
+        if (value != null)
+          return value;
+        prop = prop.defaults;
+      }
+    while (prop != null);
+    return null;
   }
 
   /**
@@ -457,7 +443,10 @@
    */
   public String getProperty(String key, String defaultValue)
   {
-    return getPropertyInternal(key, defaultValue);
+    String prop = getProperty(key);
+    if (prop == null)
+      prop = defaultValue;
+    return prop;
   }
 
   /**