Skip to content

Commit

Permalink
For Java 18+, Ant will no longer use the SecurityManager
Browse files Browse the repository at this point in the history
Starting Java 18, setting SecurityManager at runtime is disallowed by default https://bugs.openjdk.org/browse/JDK-8270380 in preparation of deprecation (for removal)
of the SecurityManager (https://openjdk.org/jeps/411). Ant (by default) internally used to create and set a SecurityManager. Among other things, one primary reason
was to intercept a call to System.exit() in a JVM within which the Ant process is running. Ant (through) its custom SecurityManager would then prevent the call to
System.exit() and instead throw a specific exception type that the Ant process knew how to handle. This is no longer feasible in Java 18+ and trying to allow usage
of SecurityManager in such Java runtime versions is neither easy nor adding any value. Ant did a release (1.10.13) which attempted to keep around the SecurityManager
usage for a few releases, by introducing some brittle and complex changes, but that is no longer feasible to maintain due to various other issues that it ended up
creating (that have been tracked in the Ant bugzilla).

This commit also skips some tests for Java 18+, those which required usage or testing of the SecurityManager
  • Loading branch information
jaikiran committed Aug 10, 2023
1 parent 8d61524 commit 689b6ea
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/main/org/apache/tools/ant/taskdefs/Java.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.tools.ant.types.RedirectorElement;
import org.apache.tools.ant.types.Reference;
import org.apache.tools.ant.util.KeepAliveInputStream;
import org.apache.tools.ant.util.SecurityManagerUtil;
import org.apache.tools.ant.util.StringUtils;

/**
Expand Down Expand Up @@ -202,7 +203,7 @@ && getCommandLine().getJar() != null) {
log("bootclasspath ignored when same JVM is used.",
Project.MSG_WARN);
}
if (perm == null) {
if (perm == null && SecurityManagerUtil.isSetSecurityManagerAllowed()) {
perm = new Permissions(true);
log("running " + this.getCommandLine().getClassname()
+ " with default permissions (exit forbidden)", Project.MSG_VERBOSE);
Expand Down
7 changes: 7 additions & 0 deletions src/main/org/apache/tools/ant/types/Permissions.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import org.apache.tools.ant.BuildException;
import org.apache.tools.ant.ExitException;
import org.apache.tools.ant.util.SecurityManagerUtil;

/**
* This class implements a security manager meant for usage by tasks that run inside the
Expand Down Expand Up @@ -98,6 +99,9 @@ public void addConfiguredRevoke(final Permissions.Permission perm) {
* @throws BuildException on error
*/
public synchronized void setSecurityManager() throws BuildException {
if (!SecurityManagerUtil.isSetSecurityManagerAllowed()) {
return;
}
origSm = System.getSecurityManager();
init();
System.setSecurityManager(new MySM());
Expand Down Expand Up @@ -169,6 +173,9 @@ private java.security.Permission createPermission(
* To be used by tasks that just finished executing the parts subject to these permissions.
*/
public synchronized void restoreSecurityManager() {
if (!SecurityManagerUtil.isSetSecurityManagerAllowed()) {
return;
}
active = false;
System.setSecurityManager(origSm);
}
Expand Down
30 changes: 30 additions & 0 deletions src/main/org/apache/tools/ant/util/SecurityManagerUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* 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
*
* https://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.tools.ant.util;

public final class SecurityManagerUtil {

private static final boolean isJava18OrHigher = JavaEnvUtils.isAtLeastJavaVersion("18");

public static boolean isSetSecurityManagerAllowed() {
if (isJava18OrHigher) {
return false;
}
return true;
}
}
19 changes: 18 additions & 1 deletion src/tests/junit/org/apache/tools/ant/taskdefs/JavaTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.tools.ant.input.DefaultInputHandler;
import org.apache.tools.ant.taskdefs.condition.JavaVersion;
import org.apache.tools.ant.util.FileUtils;
import org.apache.tools.ant.util.JavaEnvUtils;
import org.apache.tools.ant.util.TeeOutputStream;
import org.junit.Assume;
import org.junit.AssumptionViolatedException;
Expand Down Expand Up @@ -72,6 +73,18 @@ public class JavaTest {

private boolean runFatalTests = false;

private static final boolean allowedToIssueSystemExit;
private static final String SKIP_MSG_CAUSE_SYSTEM_EXIT_USE =
"Skipping test on current Java version " + JavaEnvUtils.getJavaVersion()
+ " because test calls System.exit() in non-forked VM";
static {
final JavaVersion javaVersion = new JavaVersion();
javaVersion.setAtMost("17");
// don't run tests which call System.exit() on a non-forked VM because
// Ant no longer sets a custom SecurityManager to prevent the VM exit
// for Java versions >= 18
allowedToIssueSystemExit = javaVersion.eval();
}

/**
* configure the project.
Expand Down Expand Up @@ -209,12 +222,14 @@ public void testRun() {
@Test
public void testRunFail() {
assumeTrue("Fatal tests have not been set to run", runFatalTests);
assumeTrue(SKIP_MSG_CAUSE_SYSTEM_EXIT_USE, allowedToIssueSystemExit);
buildRule.executeTarget("testRunFail");
}

@Test
public void testRunFailFoe() {
assumeTrue("Fatal tests have not been set to run", runFatalTests);
assumeTrue(SKIP_MSG_CAUSE_SYSTEM_EXIT_USE, allowedToIssueSystemExit);
thrown.expect(BuildException.class);
thrown.expectMessage("Java returned:");
buildRule.executeTarget("testRunFailFoe");
Expand Down Expand Up @@ -273,12 +288,14 @@ public void testResultPropertyZeroNoFork() {

@Test
public void testResultPropertyNonZeroNoFork() {
assumeTrue(SKIP_MSG_CAUSE_SYSTEM_EXIT_USE, allowedToIssueSystemExit);
buildRule.executeTarget("testResultPropertyNonZeroNoFork");
assertEquals("-1", buildRule.getProject().getProperty("exitcode"));
assertEquals("-1", buildRule.getProject().getProperty("exitcode"));
}

@Test
public void testRunFailWithFailOnError() {
assumeTrue(SKIP_MSG_CAUSE_SYSTEM_EXIT_USE, allowedToIssueSystemExit);
thrown.expect(BuildException.class);
thrown.expectMessage("Java returned:");
buildRule.executeTarget("testRunFailWithFailOnError");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
import org.apache.tools.ant.BuildException;
import org.apache.tools.ant.taskdefs.XSLTLiaison;
import org.apache.tools.ant.taskdefs.XSLTLogger;
import org.apache.tools.ant.taskdefs.condition.JavaVersion;
import org.apache.tools.ant.util.JAXPUtils;
import org.apache.tools.ant.util.JavaEnvUtils;
import org.junit.After;
import org.junit.Test;

Expand All @@ -60,6 +62,10 @@ public XSLTLiaison createLiaison() throws Exception {

@Test
public void testXalan2RedirectViaJDKFactory() throws Exception {
final JavaVersion javaVersion = new JavaVersion();
javaVersion.setAtMost("17");
assumeTrue("Test sets SecurityManager at runtime which is no longer supported" +
" on Java version: " + JavaEnvUtils.getJavaVersion(), javaVersion.eval());
try {
getClass().getClassLoader().loadClass("org.apache.xalan.lib.Redirect");
} catch (Exception exc) {
Expand Down Expand Up @@ -106,6 +112,10 @@ public void checkPermission(Permission perm) {

@Test
public void testXalan2RedirectViaXalan() throws Exception {
final JavaVersion javaVersion = new JavaVersion();
javaVersion.setAtMost("17");
assumeTrue("Test sets SecurityManager at runtime which is no longer supported" +
" on Java version: " + JavaEnvUtils.getJavaVersion(), javaVersion.eval());
try {
getClass().getClassLoader().loadClass("org.apache.xalan.lib.Redirect");
} catch (Exception exc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeNoException;
import static org.junit.Assume.assumeTrue;

import java.io.File;
import java.io.FileOutputStream;
Expand All @@ -29,13 +30,19 @@
import org.apache.tools.ant.DefaultLogger;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.taskdefs.Delete;
import org.apache.tools.ant.taskdefs.condition.JavaVersion;
import org.apache.tools.ant.types.FileSet;
import org.apache.tools.ant.util.JavaEnvUtils;
import org.junit.Test;

public class XMLResultAggregatorTest {

@Test
public void testFrames() throws Exception {
final JavaVersion javaVersion = new JavaVersion();
javaVersion.setAtMost("17");
assumeTrue("Test sets SecurityManager at runtime which is no longer supported" +
" on Java version: " + JavaEnvUtils.getJavaVersion(), javaVersion.eval());
// For now, skip this test on JDK 6 (and below); see below for why:
try {
Class.forName("java.nio.file.Files");
Expand Down
12 changes: 11 additions & 1 deletion src/tests/junit/org/apache/tools/ant/types/PermissionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
package org.apache.tools.ant.types;

import org.apache.tools.ant.ExitException;
import org.apache.tools.ant.taskdefs.condition.JavaVersion;
import org.apache.tools.ant.util.JavaEnvUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -27,6 +29,7 @@

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasProperty;
import static org.junit.Assume.assumeTrue;

/**
* JUnit 4 testcases for org.apache.tools.ant.types.Permissions.
Expand All @@ -40,6 +43,11 @@ public class PermissionsTest {

@Before
public void setUp() {
final JavaVersion javaVersion = new JavaVersion();
javaVersion.setAtMost("17");
assumeTrue("org.apache.tools.ant.types.Permissions no longer supported on Java version: "
+ JavaEnvUtils.getJavaVersion(), javaVersion.eval());

perms = new Permissions();
Permissions.Permission perm = new Permissions.Permission();
// Grant extra permissions to read and write the user.* properties and read to the
Expand Down Expand Up @@ -87,7 +95,9 @@ public void setUp() {

@After
public void tearDown() {
perms.restoreSecurityManager();
if (perms != null) {
perms.restoreSecurityManager();
}
}

/** Tests a permission that is granted per default. */
Expand Down

0 comments on commit 689b6ea

Please sign in to comment.