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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
