Copilot commented on code in PR #102:
URL: https://github.com/apache/synapse/pull/102#discussion_r3099435745
##########
modules/core/src/main/java/org/apache/synapse/JmxAdapter.java:
##########
@@ -204,6 +205,17 @@ public void setJmxInformation(JmxInformation
jmxInformation) {
private Map<String, Object> createContextMap() {
Map<String, Object> env = new HashMap<String, Object>();
+ String remoteSerialFilterPattern =
jmxInformation.getRemoteSerialFilterPattern();
+ if (remoteSerialFilterPattern != null &&
remoteSerialFilterPattern.trim().length() > 0) {
+ // Deserialization limits are applied via
JmxSerializationFilterSupport (JEP 415 factory),
+ // not via jmx.remote.rmi.server.serial.filter.pattern in the
connector env.
+
JmxSerializationFilterSupport.configureForJmxPattern(remoteSerialFilterPattern,
log);
+ if (log.isDebugEnabled()) {
+ log.debug("Configured JEP 290 deserialization filter for
remote JMX: "
Review Comment:
The debug log message says "Configured JEP 290 deserialization filter" but
the surrounding comments and `JmxSerializationFilterSupport` are explicitly
using the JEP 415 serial filter factory mechanism. Please align the wording
(e.g., reference JEP 415, or "JEP 290/415") to avoid confusion when debugging.
```suggestion
log.debug("Configured JEP 415 deserialization filter factory
for remote JMX: "
```
##########
docs/JDK17-build-report.md:
##########
@@ -0,0 +1,159 @@
+# Apache Synapse — JDK 17 iterative build report
+
+**Branch:** `dev-thisara`
+**Goal:** Build the project with Java 17, skipping modules that fail due to
Java version mismatch (without modifying failing source), and rebuild until a
distribution is produced.
+
+**JDK used:** `JAVA_HOME=<JDK17_HOME>`
+
+---
+
+## Build iterations
+
+### Iteration 0 — Full reactor (`mvn clean install -DskipTests`)
+
+**Result:** FAILURE at `synapse-securevault` (compile).
+
+**Root cause:** Google ErrorProne is wired as a javac plugin
(`-Xplugin:ErrorProne`) with ErrorProne 2.10.0. On JDK 9+, the compiler APIs
live in the `jdk.compiler` module and are not accessible to the unnamed module
where ErrorProne loads, causing:
+
+```text
+java.lang.IllegalAccessError: class
com.google.errorprone.BaseErrorProneJavaCompiler ... cannot access class
com.sun.tools.javac.api.BasicJavacTask (in module jdk.compiler) because module
jdk.compiler does not export com.sun.tools.javac.api to unnamed module
+```
+
+This is a **toolchain / JDK encapsulation** issue, not application source.
+
+**How we proceed (without editing failing module source):** Retry with JVM
`add-opens` for the compiler so ErrorProne can attach (see Iteration 1).
Alternative would be disabling ErrorProne via POM/profile (not used here first).
+
+---
+
+### Iteration 1 — `mvn clean install -DskipTests` + `MAVEN_OPTS`
(`--add-opens` for `jdk.compiler`)
+
+**Result:** Progressed past securevault; **FAILURE** at
`synapse-amqp-transport`.
+
Review Comment:
The report states "Synapse targets Java 17+", but the repository build
configuration still targets Java 8 (`pom.xml` has
`<java.version>1.8</java.version>`) and `synapse.sh` warns it’s tested up to
Java 8. Please correct the report to match the actual supported baseline, or
update the build baseline consistently across the project if Java 17+ is now
the goal.
##########
modules/core/src/main/java/org/apache/synapse/jmx/JmxSerializationFilterSupport.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.synapse.jmx;
+
+import org.apache.commons.logging.Log;
+
+import java.io.ObjectInputFilter;
+import java.util.function.BinaryOperator;
Review Comment:
This class imports and uses `java.io.ObjectInputFilter`, which is only
available starting in Java 9. The build is currently configured for Java 8
(`pom.xml` sets `<java.version>1.8</java.version>`), so compiling/running on
Java 8 will fail. Either raise the project bytecode/runtime baseline to 9+ (and
update docs/scripts accordingly) or gate this code behind reflection /
multi-release JAR / separate module so Java 8 builds remain compatible.
##########
modules/distribution/pom.xml:
##########
@@ -127,10 +127,11 @@
<groupId>org.apache.synapse</groupId>
<artifactId>synapse-vfs-transport</artifactId>
</dependency>
- <dependency>
+ <!-- Skipped in JDK 17 full build: synapse-amqp-transport is Java 6
source/target; see docs/JDK17-build-report.md -->
+ <!--dependency>
<groupId>org.apache.synapse</groupId>
<artifactId>synapse-amqp-transport</artifactId>
- </dependency>
+ </dependency-->
Review Comment:
This dependency is now commented out unconditionally, which means the
produced distribution will no longer include AMQP transport even for builds
that do compile it. If the intent is only to support JDK 17 builds that exclude
the module, consider gating this via a Maven profile/property (e.g., `-Pjdk17`
or `-DskipAmqpTransport`) instead of permanently removing it from the
distribution POM.
##########
modules/core/src/main/java/org/apache/synapse/jmx/JmxSerializationFilterSupport.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.synapse.jmx;
+
+import org.apache.commons.logging.Log;
+
+import java.io.ObjectInputFilter;
+import java.util.function.BinaryOperator;
+
+/**
+ * Installs JEP 415 context-specific deserialization control for remote JMX
without using
+ * {@code ObjectInputFilter.Config.setSerialFilter} for the entire JVM.
+ * <p>
+ * Synapse targets Java 17+; the configured JMX pattern is merged with the
stream filter only when
+ * the stack indicates deserialization for {@code javax.management.remote} /
{@code com.sun.jmx.remote}.
+ */
+public final class JmxSerializationFilterSupport {
+
+ private JmxSerializationFilterSupport() {
+ }
+
+ /**
+ * Installs a {@link ObjectInputFilter.Config#setSerialFilterFactory
serial filter factory}
+ * that merges the given pattern for JMX/RMI deserialization stacks.
+ *
+ * @param pattern non-empty filter pattern (same string as {@code
synapse.jmx.remote.serial.filter.pattern})
+ * @param log logger (typically {@link org.apache.synapse.JmxAdapter})
+ */
+ public static void configureForJmxPattern(String pattern, Log log) {
+ if (pattern == null || pattern.trim().isEmpty()) {
+ return;
+ }
+ try {
+ ObjectInputFilter jmxFilter =
ObjectInputFilter.Config.createFilter(pattern);
+ BinaryOperator<ObjectInputFilter> factory = (curr, next) -> {
+ ObjectInputFilter base = next != null ? next : curr;
+ if (base == null) {
+ base = ObjectInputFilter.Config.getSerialFilter();
+ }
+ if (!isJmxRemoteDeserializationStack()) {
+ return base;
+ }
+ return ObjectInputFilter.merge(jmxFilter, base);
+ };
+ ObjectInputFilter.Config.setSerialFilterFactory(factory);
Review Comment:
`ObjectInputFilter.Config.setSerialFilterFactory(...)` installs a JVM-wide
factory; the implementation calls `Thread.currentThread().getStackTrace()` on
every deserialization to decide whether to merge filters. That can add
significant overhead to all deserialization in the process when this feature is
enabled. Consider switching to `StackWalker` for a lighter check and/or
narrowing the condition so the expensive stack inspection is avoided unless
there’s strong evidence the call is coming from RMI/JMX.
--
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]
