This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new e54267a  Fix 64735 ServletContext.addJspFile() always fails with 
SecurityManager
e54267a is described below

commit e54267a3f71d29f9f850ed9966e7be9612f219d4
Author: Kyle Stiemann <kyle.stiem...@contrastsecurity.com>
AuthorDate: Thu Sep 10 16:47:21 2020 -0400

    Fix 64735 ServletContext.addJspFile() always fails with SecurityManager
---
 build.xml                                          |  13 ++
 .../catalina/core/ApplicationContextFacade.java    |   5 +
 ...estApplicationContextFacadeSecurityManager.java | 148 +++++++++++++++++++++
 .../util/security/SecurityManagerBaseTest.java     |  50 +++++++
 webapps/docs/changelog.xml                         |   5 +
 5 files changed, 221 insertions(+)

diff --git a/build.xml b/build.xml
index c5c79cb..1ab4af5 100644
--- a/build.xml
+++ b/build.xml
@@ -1978,8 +1978,21 @@
             <exclude name="**/*Performance.java" 
if="${test.excludePerformance}" />
             <!-- Exclude tests that Gump can't compile -->
             <exclude name="org/apache/tomcat/buildutil/**" />
+            <!--
+              Avoid running tests which require the SecurityManager with other
+              tests. See below for more details.
+            -->
+            <exclude name="**/*SecurityManager.java" />
           </fileset>
         </batchtest>
+        <!--
+           Run tests which require the SecurityManager in their own forked
+           batch to ensure that global/system security settings don't pollute
+           other tests. See SecurityManagerBaseTest.java for more details.
+        -->
+        <batchtest todir="${test.reports}" unless="test.entry" fork="true">
+          <fileset dir="test" includes="**/SecurityManager.java" 
excludes="${test.exclude}" />
+        </batchtest>
       </junit>
     </sequential>
   </macrodef>
diff --git a/java/org/apache/catalina/core/ApplicationContextFacade.java 
b/java/org/apache/catalina/core/ApplicationContextFacade.java
index 5dd3a37..ef004c6 100644
--- a/java/org/apache/catalina/core/ApplicationContextFacade.java
+++ b/java/org/apache/catalina/core/ApplicationContextFacade.java
@@ -117,6 +117,11 @@ public class ApplicationContextFacade implements 
ServletContext {
         classCache.put("getAttribute", clazz);
         classCache.put("log", clazz);
         classCache.put("setSessionTrackingModes", new Class[]{Set.class} );
+        classCache.put("addJspFile", new Class[]{String.class, String.class});
+        classCache.put("declareRoles", new Class[]{String[].class});
+        classCache.put("setSessionTimeout", new Class[]{int.class});
+        classCache.put("setRequestCharacterEncoding", new 
Class[]{String.class});
+        classCache.put("setResponseCharacterEncoding", new 
Class[]{String.class});
     }
 
 
diff --git 
a/test/org/apache/catalina/core/TestApplicationContextFacadeSecurityManager.java
 
b/test/org/apache/catalina/core/TestApplicationContextFacadeSecurityManager.java
new file mode 100644
index 0000000..fc5a5d5
--- /dev/null
+++ 
b/test/org/apache/catalina/core/TestApplicationContextFacadeSecurityManager.java
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.core;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.stream.Collectors;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.catalina.security.SecurityUtil;
+import org.apache.tomcat.util.security.SecurityManagerBaseTest;
+import org.easymock.EasyMock;
+import org.easymock.IExpectationSetters;
+import org.easymock.internal.LastControl;
+
+@RunWith(Parameterized.class)
+public final class TestApplicationContextFacadeSecurityManager extends 
SecurityManagerBaseTest {
+
+    /**
+     * @return {@link Collection} of non-static, non-object, public {@link
+     * Method}s in {@link ApplicationContextFacade} to be run with the the Java
+     * 2 {@link SecurityManager} been enabled.
+     */
+    @Parameterized.Parameters(name = "{index}: method={0}")
+    public static Collection<Method> publicApplicationContextFacadeMethods() {
+        return Arrays.stream(ApplicationContextFacade.class.getMethods())
+                .filter(method -> !Modifier.isStatic(method.getModifiers()))
+                .filter(method -> {
+                    try {
+                        Object.class.getMethod(method.getName(), 
method.getParameterTypes());
+                        return false;
+                    } catch (final NoSuchMethodException e) {
+                        return true;
+                    }
+                })
+                .collect(Collectors.toList());
+    }
+
+
+    private static Object[] getDefaultParams(final Method method) {
+        final int paramsCount = method.getParameterCount();
+        final Object[] params = new Object[paramsCount];
+        final Class<?>[] paramTypes = method.getParameterTypes();
+        for (int i = 0; i < params.length; i++) {
+            params[i] = getDefaultValue(paramTypes[i]);
+        }
+        return params;
+    }
+
+
+    @SuppressWarnings("unchecked")
+    private static <T> T getDefaultValue(final Class<T> clazz) {
+        return !isVoid(clazz) ? (T) Array.get(Array.newInstance(clazz, 1), 0) 
: null;
+    }
+
+
+    private static <T> boolean isVoid(Class<T> clazz) {
+        return void.class.equals(clazz) || Void.class.equals(clazz);
+    }
+
+
+    @Parameter(0)
+    public Method methodToTest;
+
+
+    /**
+     * Test for
+     * <a href="https://bz.apache.org/bugzilla/show_bug.cgi?id=64735";>Bug
+     * 64735</a> which confirms that {@link ApplicationContextFacade} behaves
+     * correctly when the Java 2 {@link SecurityManager} has been enabled.
+     *
+     * @throws NoSuchMethodException     Should never happen
+     * @throws IllegalAccessException    Should never happen
+     * @throws InvocationTargetException Should never happen
+     */
+    @Test
+    public void testBug64735()
+            throws NoSuchMethodException, IllegalAccessException, 
InvocationTargetException {
+        Assert.assertTrue(SecurityUtil.isPackageProtectionEnabled());
+
+        // Mock the ApplicationContext that we provide to the 
ApplicationContextFacade.
+        final ApplicationContext mockAppContext = 
EasyMock.createMock(ApplicationContext.class);
+        final Method expectedAppContextMethod =
+                ApplicationContext.class.getMethod(
+                        methodToTest.getName(),
+                        methodToTest.getParameterTypes());
+
+        // Expect that only the provided method which is being tested will be 
called exactly once.
+        final IExpectationSetters<Object> expectationSetters;
+        if (isVoid(expectedAppContextMethod.getReturnType())) {
+            expectedAppContextMethod.invoke(mockAppContext, 
getDefaultParams(methodToTest));
+            expectationSetters = EasyMock.expectLastCall();
+        } else {
+            expectationSetters =
+                    EasyMock.expect(expectedAppContextMethod.invoke(
+                            mockAppContext, getDefaultParams(methodToTest)));
+        }
+        expectationSetters
+                .andAnswer(() -> {
+                    Assert.assertEquals(
+                            expectedAppContextMethod,
+                            LastControl.getCurrentInvocation().getMethod());
+                    return 
getDefaultValue(expectedAppContextMethod.getReturnType());
+                }).once();
+        EasyMock.replay(mockAppContext);
+        EasyMock.verifyUnexpectedCalls(mockAppContext);
+
+        // Invoke the method on ApplicationContextFacade. Fail if any 
unexpected exceptions are
+        // thrown.
+        try {
+            methodToTest.invoke(
+                    new ApplicationContextFacade(mockAppContext),
+                    getDefaultParams(methodToTest));
+        } catch (final IllegalAccessException | InvocationTargetException e) {
+            throw new AssertionError(
+                    "Failed to call " +
+                            methodToTest +
+                            " with SecurityManager enabled.",
+                    e);
+        }
+
+        // Verify that the method called through to the wrapped 
ApplicationContext correctly.
+        EasyMock.verifyRecording();
+    }
+}
diff --git a/test/org/apache/tomcat/util/security/SecurityManagerBaseTest.java 
b/test/org/apache/tomcat/util/security/SecurityManagerBaseTest.java
new file mode 100644
index 0000000..af7ff8f
--- /dev/null
+++ b/test/org/apache/tomcat/util/security/SecurityManagerBaseTest.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.security;
+
+import java.security.Permission;
+
+import org.apache.catalina.security.SecurityUtil;
+
+/**
+ * Base test class for unit tests which require the Java 2 {@link
+ * SecurityManager} to be enabled. Tests that extend this class must be run in 
a
+ * forked SecurityManager test batch since this class modifies global {@link
+ * System} settings which may interfere with other tests. On static class
+ * initialization, this class sets up the {@code "package.definition"} and
+ * {@code "package.access"} system properties and adds a no-op SecurityManager
+ * which does not check permissions. These settings are required in order to
+ * make {@link org.apache.catalina.Globals#IS_SECURITY_ENABLED} and {@link
+ * SecurityUtil#isPackageProtectionEnabled()} return true.
+ */
+public abstract class SecurityManagerBaseTest {
+    static {
+        System.setProperty("package.definition", "test");
+        System.setProperty("package.access", "test");
+        System.setSecurityManager(new SecurityManager() {
+            @Override
+            public void checkPermission(final Permission permission) {
+                // no-op
+            }
+
+            @Override
+            public void checkPermission(final Permission permission, Object 
context) {
+                // no-op
+            }
+        });
+    }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b6ee041..bf5b40c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -90,6 +90,11 @@
         database. (kfujino)
       </fix>
       <fix>
+        <bug>64735</bug>: Ensure that none of the methods on a
+        <code>ServletContext</code> instance always fail when running under a
+        SecurityManager. Pull request provided by Kyle Stiemann. (markt)
+      </fix>
+      <fix>
         <bug>64765</bug>: Ensure that the number of currently processing 
threads
         is tracked correctly when a web application is undeployed, long running
         requests are being processed and


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to