From c34fca4c18265ccde6cb3fa715d2f2558a60add7 Mon Sep 17 00:00:00 2001
From: kkolinko <kkolinko@13f79535-47bb-0310-9956-ffa450edef68>
Date: Thu, 9 Jun 2011 13:20:09 +0000
Subject: [PATCH] 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
---
 java/org/apache/juli/ClassLoaderLogManager.java    | 33 +++++++++++++---------
 .../org/apache/juli/TestClassLoaderLogManager.java | 11 ++++++--
 webapps/docs/changelog.xml                         |  7 ++++-
 3 files changed, 35 insertions(+), 16 deletions(-)

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>
-- 
2.11.0