Automatically adjust web.forwardSlash on Tomcat containers

One issue that frequently arises in the discussion group and the bug
tracker is how Tomcat automatically re-encodes %2f as '/' which breaks
url parameters with %2f.  After documenting this in half a dozen places
it still comes up.  Clearly I haven't done enough.

Gitblit will now act on, instead of just report, an improperly configured
web.forwardSlash character on Tomcat containers.  This will make Gitblit
"just work" for more users and will make the world a better place.

Change-Id: I344428804070a2d6082022cf6b80e2a3d83cea84

diff --git a/src/main/java/com/gitblit/GitBlit.java b/src/main/java/com/gitblit/GitBlit.java
index f191d6a661f0f6cd8abe223e401cabfba6bc291a..97372e11b7f1ce78b54d68a217764d392ef9f64e 100644 (file)
--- a/src/main/java/com/gitblit/GitBlit.java
+++ b/src/main/java/com/gitblit/GitBlit.java
@@ -3554,8 +3554,6 @@ public class GitBlit implements ServletContextListener {
                configureFanout();
                configureGitDaemon();
                configureCommitCache();
-
-               ContainerUtils.CVE_2007_0450.test();
        }
 
        protected void configureMailExecutor() {
@@ -3817,6 +3815,10 @@ public class GitBlit implements ServletContextListener {
                                FileSettings settings = new FileSettings(localSettings.getAbsolutePath());
                                configureContext(settings, base, true);
                        }
+
+                       // WAR or Express is likely to be running on a Tomcat.
+                       // Test for the forward-slash/%2F issue and auto-adjust settings.
+                       ContainerUtils.CVE_2007_0450.test(settings);
                }
 
                settingsModel = loadSettingModels();

diff --git a/src/main/java/com/gitblit/utils/ContainerUtils.java b/src/main/java/com/gitblit/utils/ContainerUtils.java
index 613bf97acdfbf50485596476699a951bd8219a84..d0dfcd1f5c7710d91812bcd61abf9bd936c2561e 100644 (file)
--- a/src/main/java/com/gitblit/utils/ContainerUtils.java
+++ b/src/main/java/com/gitblit/utils/ContainerUtils.java
@@ -22,7 +22,7 @@ import java.lang.reflect.Method;
 import org.slf4j.Logger;\r
 import org.slf4j.LoggerFactory;\r
 \r
-import com.gitblit.GitBlit;\r
+import com.gitblit.IStoredSettings;\r
 import com.gitblit.Keys;\r
 \r
 /**\r
@@ -41,37 +41,18 @@ public class ContainerUtils
      * @see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450\r
      * @author jpyeron\r
      */\r
-    public static class CVE_2007_0450\r
-    {\r
+    public static class CVE_2007_0450 {\r
         /**\r
          * This method will test for know issues in certain containers where %2F\r
          * is blocked from use in URLs. It will emit a warning to the logger if\r
          * the configuration of Tomcat causes the URL processing to fail on %2F.\r
          */\r
-        public static void test()\r
-        {\r
-            if (GitBlit.getBoolean(Keys.web.mountParameters, true)\r
-                    && ((GitBlit.getChar(Keys.web.forwardSlashCharacter, '/')) == '/' || (GitBlit.getChar(\r
-                            Keys.web.forwardSlashCharacter, '/')) == '\\'))\r
-            {\r
-                try\r
-                {\r
-                    if (GitBlit.isGO())\r
-                        ;\r
-                    else if (logCVE_2007_0450Tomcat())\r
-                        ;\r
-                    // else if (logCVE_2007_0450xxx());\r
-                    else\r
-                    {\r
-                        LOGGER.info("Unknown container, cannot check for CVE-2007-0450 aplicability");\r
-                    }\r
-                }\r
-                catch (Throwable t)\r
-                {\r
-                    LOGGER.warn("Failure in checking for CVE-2007-0450 aplicability", t);\r
-                }\r
+        public static void test(IStoredSettings settings) {\r
+               boolean mounted = settings.getBoolean(Keys.web.mountParameters, true);\r
+               char fsc = settings.getChar(Keys.web.forwardSlashCharacter, '/');\r
+            if (mounted && ((fsc == '/') || (fsc == '\\'))) {\r
+               logCVE_2007_0450Tomcat(settings);\r
             }\r
-\r
         }\r
 \r
         /**\r
@@ -84,10 +65,8 @@ public class ContainerUtils
          * @return true if it recognizes Tomcat, false if it does not recognize\r
          *         Tomcat\r
          */\r
-        private static boolean logCVE_2007_0450Tomcat()\r
-        {\r
-            try\r
-            {\r
+        private static boolean logCVE_2007_0450Tomcat(IStoredSettings settings) {\r
+            try {\r
                 byte[] test = "http://server.domain:8080/context/servlet/param%2fparam".getBytes();\r
 \r
                 // ByteChunk mb=new ByteChunk();\r
@@ -95,37 +74,34 @@ public class ContainerUtils
                 Object mb = cByteChunk.newInstance();\r
 \r
                 // mb.setBytes(test, 0, test.length);\r
-                Method mByteChunck_setBytes = cByteChunk.getMethod("setBytes", byte[].class, int.class, int.class);\r
-                mByteChunck_setBytes.invoke(mb, test, 0, test.length);\r
+                Method setBytes = cByteChunk.getMethod("setBytes", byte[].class, int.class, int.class);\r
+                setBytes.invoke(mb, test, 0, test.length);\r
 \r
                 // UDecoder ud=new UDecoder();\r
                 Class<?> cUDecoder = Class.forName("org.apache.tomcat.util.buf.UDecoder");\r
                 Object ud = cUDecoder.newInstance();\r
 \r
                 // ud.convert(mb,false);\r
-                Method mUDecoder_convert = cUDecoder.getMethod("convert", cByteChunk, boolean.class);\r
+                Method convert = cUDecoder.getMethod("convert", cByteChunk, boolean.class);\r
 \r
-                try\r
-                {\r
-                    mUDecoder_convert.invoke(ud, mb, false);\r
-                }\r
-                catch (InvocationTargetException e)\r
-                {\r
-                    if (e.getTargetException() != null && e.getTargetException() instanceof CharConversionException)\r
-                    {\r
+                try {\r
+                    convert.invoke(ud, mb, false);\r
+                } catch (InvocationTargetException e) {\r
+                    if (e.getTargetException() != null && e.getTargetException() instanceof CharConversionException) {\r
                         LOGGER.warn("You are using a Tomcat-based server and your current settings will prevent grouped repositories, forks, personal repositories, and tree navigation from working properly. Please review the FAQ for details about running Gitblit on Tomcat. http://gitblit.com/faq.html and http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450");\r
+                        LOGGER.warn("Overriding {} and setting to {}", Keys.web.forwardSlashCharacter, "!");\r
+                        settings.overrideSetting(Keys.web.forwardSlashCharacter, "!");\r
                         return true;\r
                     }\r
                     throw e;\r
                 }\r
-            }\r
-            catch (Throwable t)\r
-            {\r
+            } catch (Throwable t) {\r
                 // The apache url decoder internals are different, this is not a\r
                 // Tomcat matching the failure pattern for CVE-2007-0450\r
                 if (t instanceof ClassNotFoundException || t instanceof NoSuchMethodException\r
-                        || t instanceof IllegalArgumentException)\r
+                        || t instanceof IllegalArgumentException) {\r
                     return false;\r
+                }\r
                 LOGGER.debug("This is a tomcat, but the test operation failed somehow", t);\r
             }\r
             return true;\r