Enable UEncoder instances to share safeChars BitSet while ensuring th…

…at the

shared BitSet is immutable to be threadsafe.

Change Response to use UEncoder instances with shared safeChars.


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

java/org/apache/catalina/connector/Response.java

@@ -50,6 +50,7 @@
 import org.apache.coyote.ActionCode;
 import org.apache.tomcat.util.buf.CharChunk;
 import org.apache.tomcat.util.buf.UEncoder;
+import org.apache.tomcat.util.buf.UEncoder.SafeCharsSet;
 import org.apache.tomcat.util.http.FastHttpDateFormat;
 import org.apache.tomcat.util.http.MimeHeaders;
 import org.apache.tomcat.util.http.parser.MediaTypeCache;
@@ -89,7 +90,6 @@ public class Response
     }
 
     public Response() {
-        urlEncoder.addSafeCharacter('/');
     }
 
 
@@ -241,7 +241,7 @@ public Context getContext() {
     /**
      * URL encoder.
      */
-    protected final UEncoder urlEncoder = new UEncoder();
+    protected final UEncoder urlEncoder = new UEncoder(SafeCharsSet.WITH_SLASH);
 
 
     /**

java/org/apache/tomcat/util/buf/UEncoder.java

@@ -32,21 +32,57 @@
  */
 public final class UEncoder {
 
+    public enum SafeCharsSet {
+        WITH_SLASH("/"), DEFAULT("");
+        private final BitSet safeChars;
+
+        private BitSet getSafeChars() {
+            return this.safeChars;
+        }
+
+        private SafeCharsSet(String additionalSafeChars) {
+            safeChars = initialSafeChars();
+            for (char c : additionalSafeChars.toCharArray()) {
+                safeChars.set(c);
+            }
+        }
+    }
+
     // Not static - the set may differ ( it's better than adding
     // an extra check for "/", "+", etc
     private BitSet safeChars=null;
     private C2BConverter c2b=null;
     private ByteChunk bb=null;
     private CharChunk cb=null;
     private CharChunk output=null;
+    private final boolean readOnlySafeChars;
 
     private final String ENCODING = "UTF8";
 
     public UEncoder() {
-        initSafeChars();
+        this.safeChars = initialSafeChars();
+        readOnlySafeChars = false;
     }
 
+    /**
+     * Create a UEncoder with an unmodifiable safe character set.
+     * <p>
+     * Calls to {@link UEncoder#addSafeCharacter(char) addSafeCharacter(char)}
+     * on instances created by this constructor will throw an
+     * {@link IllegalStateException}.
+     * 
+     * @param safeCharsSet
+     *            safe characters for this encoder
+     */
+    public UEncoder(SafeCharsSet safeCharsSet) {
+        this.safeChars = safeCharsSet.getSafeChars();
+        readOnlySafeChars = true;
+    }
+
     public void addSafeCharacter( char c ) {
+        if (readOnlySafeChars) {
+            throw new IllegalStateException("UEncoders safeChararacters are read only");
+        }
         safeChars.set( c );
     }
 
@@ -116,33 +152,34 @@ protected void urlEncode(CharChunk out, ByteChunk bb)
 
     // -------------------- Internal implementation --------------------
 
-    private void initSafeChars() {
-        safeChars=new BitSet(128);
+    private static BitSet initialSafeChars() {
+        BitSet initialSafeChars=new BitSet(128);
         int i;
         for (i = 'a'; i <= 'z'; i++) {
-            safeChars.set(i);
+            initialSafeChars.set(i);
         }
         for (i = 'A'; i <= 'Z'; i++) {
-            safeChars.set(i);
+            initialSafeChars.set(i);
         }
         for (i = '0'; i <= '9'; i++) {
-            safeChars.set(i);
+            initialSafeChars.set(i);
         }
         //safe
-        safeChars.set('$');
-        safeChars.set('-');
-        safeChars.set('_');
-        safeChars.set('.');
+        initialSafeChars.set('$');
+        initialSafeChars.set('-');
+        initialSafeChars.set('_');
+        initialSafeChars.set('.');
 
         // Dangerous: someone may treat this as " "
         // RFC1738 does allow it, it's not reserved
-        //    safeChars.set('+');
+        //    initialSafeChars.set('+');
         //extra
-        safeChars.set('!');
-        safeChars.set('*');
-        safeChars.set('\'');
-        safeChars.set('(');
-        safeChars.set(')');
-        safeChars.set(',');
+        initialSafeChars.set('!');
+        initialSafeChars.set('*');
+        initialSafeChars.set('\'');
+        initialSafeChars.set('(');
+        initialSafeChars.set(')');
+        initialSafeChars.set(',');
+        return initialSafeChars;
     }
 }

test/org/apache/tomcat/util/buf/TestUEncoder.java

@@ -20,6 +20,9 @@
 import java.io.IOException;
 
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.apache.tomcat.util.buf.UEncoder.SafeCharsSet;
 import org.junit.Test;
 
 /**
@@ -45,4 +48,27 @@ public void testEncodeURL() throws IOException {
         assertTrue(urlEncoder.encodeURL(s, 0, s.length())
                 .equals("%f0%90%90%81"));
     }
+
+    @Test
+    public void testEncodeURLWithSlashInit() throws IOException {
+        UEncoder urlEncoder = new UEncoder(SafeCharsSet.WITH_SLASH);
+
+        String s = "a+b/c/d+e.class";
+        assertTrue(urlEncoder.encodeURL(s, 0, s.length()).equals(
+                "a%2bb/c/d%2be.class"));
+        assertTrue(urlEncoder.encodeURL(s, 2, s.length() - 2).equals(
+                "b/c/d%2be.cla"));
+
+        try {
+            urlEncoder.addSafeCharacter('+');
+            fail();
+        } catch (IllegalStateException e) {
+            // OK
+        }
+
+        s = new String(new char[] { 0xD801, 0xDC01 });
+        assertTrue(urlEncoder.encodeURL(s, 0, s.length())
+                .equals("%f0%90%90%81"));
+    }
+
 }