gemmellr commented on code in PR #1534: URL: https://github.com/apache/activemq/pull/1534#discussion_r2527730013
########## activemq-compat/src/main/java/org/apache/activemq/compat/sm/SecurityManagerShim.java: ########## @@ -0,0 +1,199 @@ +/* + * 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.activemq.compat.sm; + +import javax.security.auth.Subject; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; + +/* + * SecurityManager related shim. + * This specific class uses legacy methods toward usage on Java 17 - 23. + * + * The API of this class must be kept the same as the Java 24+ implementation + * variant of this class which can be found at: + * src/main/java24/org/apache/activemq/artemis/utils/sm/SecurityManagerShim.java Review Comment: All of the Javadoc in this class is inaccurate now as you havent updated it to reflect how/where it is being used. ########## activemq-compat/pom.xml: ########## @@ -0,0 +1,95 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>org.apache.activemq</groupId> + <artifactId>activemq-parent</artifactId> + <version>6.2.1-SNAPSHOT</version> + </parent> + + <artifactId>activemq-compat</artifactId> + <packaging>bundle</packaging> + <name>ActiveMQ :: Compat</name> + <description>The ActiveMQ Compatibility (MRJAR)</description> + + <build> + <plugins> + <plugin> + <groupId>org.apache.felix</groupId> + <artifactId>maven-bundle-plugin</artifactId> + <extensions>true</extensions> + <inherited>true</inherited> Review Comment: Inherited being true seems to make little sense here, no children and its true by default ########## activemq-compat/pom.xml: ########## @@ -0,0 +1,95 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>org.apache.activemq</groupId> + <artifactId>activemq-parent</artifactId> + <version>6.2.1-SNAPSHOT</version> + </parent> + + <artifactId>activemq-compat</artifactId> + <packaging>bundle</packaging> + <name>ActiveMQ :: Compat</name> + <description>The ActiveMQ Compatibility (MRJAR)</description> + + <build> + <plugins> + <plugin> + <groupId>org.apache.felix</groupId> + <artifactId>maven-bundle-plugin</artifactId> + <extensions>true</extensions> + <inherited>true</inherited> + <configuration> + <instructions> + <Import-Package> + !java.*, + !javax.naming*, + !javax.net*, + !org.apache.activemq*, + !com.google.errorprone.annotations, + !com.google.errorprone.annotations.concurrent, + com.thoughtworks.xstream.*;resolution:="optional", + javax.jmdns.*;resolution:="optional", + * + </Import-Package> + <Private-Package> + com.google.errorprone.annotations, + com.google.errorprone.annotations.concurrent + </Private-Package> Review Comment: At least some of these seem like they should be superfluous. ########## activemq-compat/pom.xml: ########## @@ -0,0 +1,95 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>org.apache.activemq</groupId> + <artifactId>activemq-parent</artifactId> + <version>6.2.1-SNAPSHOT</version> + </parent> + + <artifactId>activemq-compat</artifactId> + <packaging>bundle</packaging> + <name>ActiveMQ :: Compat</name> + <description>The ActiveMQ Compatibility (MRJAR)</description> + + <build> + <plugins> + <plugin> + <groupId>org.apache.felix</groupId> + <artifactId>maven-bundle-plugin</artifactId> + <extensions>true</extensions> + <inherited>true</inherited> + <configuration> + <instructions> + <Import-Package> + !java.*, + !javax.naming*, + !javax.net*, + !org.apache.activemq*, + !com.google.errorprone.annotations, + !com.google.errorprone.annotations.concurrent, + com.thoughtworks.xstream.*;resolution:="optional", + javax.jmdns.*;resolution:="optional", + * + </Import-Package> + <Private-Package> + com.google.errorprone.annotations, + com.google.errorprone.annotations.concurrent + </Private-Package> + <Multi-Release>true</Multi-Release> + <_noee>true</_noee> + </instructions> + </configuration> + </plugin> + </plugins> + </build> + + <profiles> + <profile> + <id>jdk21-plus</id> + <activation> + <jdk>[21,)</jdk> + </activation> + <build> + <plugins> + <plugin> + <artifactId>maven-compiler-plugin</artifactId> + <executions> + <execution> + <id>java21-compile</id> + <phase>compile</phase> + <goals> + <goal>compile</goal> + </goals> + <configuration> + <release>21</release> <!-- Specific Java version for alternative classes --> Review Comment: I probably wouldnt change anything for 21 users personally...but if going with 21 for this, then you may want to avoid, or split out, the rest of the shim beyond the Subject bit since if the methods are ever used the current approach would break anyone actually using the SecurityManager on 21-23 currently. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information, visit: https://activemq.apache.org/contact
