From: kkolinko <kkolinko@13f79535-47bb-0310-9956-ffa450edef68>
Date: Thu, 9 Jun 2011 13:20:09 +0000 (+0000)
Subject: https://issues.apache.org/bugzilla/show_bug.cgi?id=51249
X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=c34fca4c18265ccde6cb3fa715d2f2558a60add7;p=tomcat7.0

https://issues.apache.org/bugzilla/show_bug.cgi?id=51249
Reimplement system properties replacement code in ClassLoaderLogManager of JULI
1. Do not use recursion.
2. Do not stop on the first unrecognized property, but continue with the rest of the string.
3. Do not call System.getProperty() on an empty key, because it throws IllegalArgumentException. Threat "${}" as unrecognized property.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1133857 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/java/org/apache/juli/ClassLoaderLogManager.java b/java/org/apache/juli/ClassLoaderLogManager.java
index 248334f38..3367460d2 100644
--- a/java/org/apache/juli/ClassLoaderLogManager.java
+++ b/java/org/apache/juli/ClassLoaderLogManager.java
@@ -566,25 +566,32 @@ public class ClassLoaderLogManager extends LogManager {
      */
     protected String replace(String str) {
         String result = str;
-        int pos_start = result.indexOf("${");
-        if (pos_start != -1) {
-            int pos_end = result.indexOf('}', pos_start);
-            if (pos_end != -1) {
-                String propName = result.substring(pos_start + 2, pos_end);
-                String replacement = System.getProperty(propName);
+        int pos_start = str.indexOf("${");
+        if (pos_start >= 0) {
+            StringBuilder builder = new StringBuilder();
+            int pos_end = -1;
+            while (pos_start >= 0) {
+                builder.append(str, pos_end + 1, pos_start);
+                pos_end = str.indexOf('}', pos_start + 2);
+                if (pos_end < 0) {
+                    pos_end = pos_start - 1;
+                    break;
+                }
+                String propName = str.substring(pos_start + 2, pos_end);
+                String replacement = propName.length() > 0 ? System
+                        .getProperty(propName) : null;
                 if (replacement != null) {
-                    if(pos_start >0) {
-                        result = result.substring(0,pos_start) + 
-                            replacement + replace(result.substring(pos_end + 1));
-                    } else {                       
-                        result = replacement + replace(result.substring(pos_end + 1));
-                    }
+                    builder.append(replacement);
+                } else {
+                    builder.append(str, pos_start, pos_end + 1);
                 }
+                pos_start = str.indexOf("${", pos_end + 1);
             }
+            builder.append(str, pos_end + 1, str.length());
+            result = builder.toString();
         }
         return result;
     }
-    
 
     // ---------------------------------------------------- LogNode Inner Class
 
diff --git a/test/org/apache/juli/TestClassLoaderLogManager.java b/test/org/apache/juli/TestClassLoaderLogManager.java
index bb8695354..3ecf3e854 100644
--- a/test/org/apache/juli/TestClassLoaderLogManager.java
+++ b/test/org/apache/juli/TestClassLoaderLogManager.java
@@ -28,8 +28,8 @@ public class TestClassLoaderLogManager extends TestCase {
         ClassLoaderLogManager logManager = new ClassLoaderLogManager();
         assertEquals("", logManager.replace(""));
         assertEquals("${", logManager.replace("${"));
-        assertEquals("${undefinedsystemproperty}",
-                logManager.replace("${undefinedsystemproperty}"));
+        assertEquals("${undefinedproperty}",
+                logManager.replace("${undefinedproperty}"));
         assertEquals(
                 System.getProperty("line.separator")
                         + System.getProperty("path.separator")
@@ -46,6 +46,13 @@ public class TestClassLoaderLogManager extends TestCase {
         assertEquals(
                 "%{file.separator}" + System.getProperty("file.separator"),
                 logManager.replace("%{file.separator}${file.separator}"));
+        assertEquals(
+                System.getProperty("file.separator") + "${undefinedproperty}"
+                        + System.getProperty("file.separator"),
+                logManager
+                        .replace("${file.separator}${undefinedproperty}${file.separator}"));
+        assertEquals("${}" + System.getProperty("path.separator"),
+                logManager.replace("${}${path.separator}"));
     }
 
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ffac5c6ec..0cf128aa7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -76,6 +76,11 @@
         files in parallel. Apache Tomcat does not do this but products that
         embed it may. (markt)
       </fix>
+      <fix>
+        <bug>51249</bug>: Further improve system property replacement code
+        in ClassLoaderLogManager of Tomcat JULI to cover some corner cases.
+        (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">
@@ -140,7 +145,7 @@
       </fix>
       <fix>
         <bug>51249</bug>: Correct ClassLoaderLogManager system property
-        replacement code so properties of the form ${...}${...} can be used
+        replacement code so properties of the form "}${...}" can be used
         without error. (markt) 
       </fix>
       <fix>