This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git
The following commit(s) were added to refs/heads/main by this push:
new 187378a Fix bug that meant unchanged strings were treated as
converted.
187378a is described below
commit 187378aeef2606283910c13b2c646ca319d3382f
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Sep 8 11:36:20 2026 +0100
Fix bug that meant unchanged strings were treated as converted.
Identified by code review
Fix and test written by Claude/Sonnet 5.
---
.../apache/tomcat/jakartaee/ClassConverter.java | 28 ++++++++-
.../tomcat/jakartaee/ClassConverterTest.java | 70 ++++++++++++++++++++++
.../apache/tomcat/jakartaee/TesterConstants.java | 17 ++++++
3 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
index 1da58a6..319d9cc 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/ClassConverter.java
@@ -24,8 +24,12 @@ import java.io.OutputStream;
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.IllegalClassFormatException;
import java.security.ProtectionDomain;
+import java.util.ArrayList;
+import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.apache.bcel.classfile.ClassFormatException;
import org.apache.bcel.classfile.ClassParser;
@@ -48,6 +52,13 @@ public class ClassConverter implements Converter,
ClassFileTransformer {
private static final Logger logger =
Logger.getLogger(ClassConverter.class.getCanonicalName());
private static final StringManager sm =
StringManager.getManager(ClassConverter.class);
+ // Delimiters used to split a ConstantUtf8 value (e.g. a method
+ // descriptor) into independently convertible fragments. profile.convert()
+ // never matches or introduces these characters, so the delimiter
+ // sequence found in the original string is also valid for the converted
+ // string and can be reinserted verbatim when fragments are reassembled.
+ private static final Pattern FRAGMENT_DELIMITER = Pattern.compile("[;<]");
+
/**
* The configured spec profile.
*/
@@ -142,10 +153,19 @@ public class ClassConverter implements Converter,
ClassFileTransformer {
// independently so we never need to revert a
conversion.
String[] convertedFragments = newString.split(";|<",
-1);
String[] originalFragments = str.split(";|<", -1);
+ // Capture the delimiters split() discarded so they
can be
+ // reinserted below; there is exactly one fewer
delimiter
+ // than fragments.
+ List<String> delimiters = new ArrayList<>();
+ Matcher delimiterMatcher =
FRAGMENT_DELIMITER.matcher(str);
+ while (delimiterMatcher.find()) {
+ delimiters.add(delimiterMatcher.group());
+ }
StringBuilder result = new StringBuilder();
for (int fi = 0; fi < convertedFragments.length; fi++)
{
String convertedFragment = convertedFragments[fi];
String originalFragment = originalFragments[fi];
+ String fragmentToAppend = convertedFragment;
int pos =
convertedFragment.indexOf(profile.getTarget() + "/");
boolean dotMode = false;
if (pos < 0) {
@@ -165,11 +185,13 @@ public class ClassConverter implements Converter,
ClassFileTransformer {
convertedFragment.substring(pos).replace('/','.')));
}
// Use the original (unconverted) fragment
- result.append(originalFragment);
- continue;
+ fragmentToAppend = originalFragment;
}
}
- result.append(convertedFragment);
+ result.append(fragmentToAppend);
+ if (fi < delimiters.size()) {
+ result.append(delimiters.get(fi));
+ }
}
newString = result.toString();
if (newString.equals(str)) {
diff --git a/src/test/java/org/apache/tomcat/jakartaee/ClassConverterTest.java
b/src/test/java/org/apache/tomcat/jakartaee/ClassConverterTest.java
index d57a01b..6e16fa6 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/ClassConverterTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/ClassConverterTest.java
@@ -95,4 +95,74 @@ public class ClassConverterTest {
assertTrue(strings.contains("jakarta.servlet.CommonGatewayInterface"));
assertTrue(strings.contains("jakarta/servlet/CommonGatewayInterface"));
}
+
+
+ /**
+ * Multi-fragment constant (mimicking a method descriptor) where one
+ * fragment resolves in the jakarta namespace and must be converted, and
+ * the other does not and must be reverted. The ';' delimiters between
+ * fragments must be preserved either way.
+ *
+ * @throws Exception if the transformation of the test constants fails
+ */
+ @Test
+ public void testTransformMultiFragmentPartialRevertPreservesDelimiters()
throws Exception {
+ Set<String> strings = transformTesterConstants();
+
+ assertFalse("Fully unconverted value should not remain",
+ strings.contains(TesterConstants.MULTI_FRAGMENT_PARTIAL));
+ assertTrue("Convertible fragment should be converted and delimiters
preserved",
+
strings.contains("(Ljakarta/servlet/CommonGatewayInterface;Ljavax/servlet/DoesNotExist;)V"));
+ }
+
+
+ /**
+ * Multi-fragment constant where neither fragment resolves in the jakarta
+ * namespace, so every fragment is reverted. The reassembled value must be
+ * byte-for-byte identical to the original (delimiters included) and must
+ * not be reported as a change to the constant pool.
+ *
+ * @throws Exception if the transformation of the test constants fails
+ */
+ @Test
+ public void testTransformMultiFragmentFullRevertPreservesDelimiters()
throws Exception {
+ Set<String> strings = transformTesterConstants();
+
+ assertTrue("Fully reverted value must be reconstructed exactly,
delimiters included",
+ strings.contains(TesterConstants.MULTI_FRAGMENT_ALL_MISSING));
+ assertFalse("Delimiter-stripped corruption must not appear",
+
strings.contains("(Ljavax/servlet/DoesNotExistLjavax/servlet/DoesNotExist)V"));
+ }
+
+
+ private Set<String> transformTesterConstants() throws Exception {
+ byte[] original;
+
+ try (InputStream is =
this.getClass().getResourceAsStream("/org/apache/tomcat/jakartaee/TesterConstants.class");
+ ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ assertNotNull(is);
+ byte[] buf = new byte[1024];
+ int len;
+ while ((len = is.read(buf)) > 0) {
+ baos.write(buf, 0, len);
+ }
+ original = baos.toByteArray();
+ }
+
+ ClassConverter converter = new ClassConverter(EESpecProfiles.TOMCAT);
+ byte[] transformed =
converter.transform(this.getClass().getClassLoader(),
+ "org.apache.tomcat.jakartaee.TesterConstants", null, null,
original);
+
+ Set<String> strings = new HashSet<>();
+ ClassParser parser = new ClassParser(new
ByteArrayInputStream(transformed), "unknown");
+ JavaClass javaClass = parser.parse();
+ Constant[] constantPool =
javaClass.getConstantPool().getConstantPool();
+ for (int i = 0; i < constantPool.length; i++) {
+ if (constantPool[i] instanceof ConstantUtf8) {
+ ConstantUtf8 c = (ConstantUtf8) constantPool[i];
+ strings.add(c.getBytes());
+ }
+ }
+ return strings;
+ }
}
diff --git a/src/test/java/org/apache/tomcat/jakartaee/TesterConstants.java
b/src/test/java/org/apache/tomcat/jakartaee/TesterConstants.java
index 27acfb7..f64e9b3 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/TesterConstants.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/TesterConstants.java
@@ -27,6 +27,13 @@ package org.apache.tomcat.jakartaee;
* rewrite them. The {@code JAVA_NOT_PRESENT_} constants reference a class that
* does not exist in either namespace, so the converter must leave them
* unchanged.
+ * <p>
+ * The {@code MULTI_FRAGMENT_} constants mimic a bytecode method descriptor:
+ * multiple {@code ;}-delimited class name fragments in a single constant.
+ * They exercise the loader-guided per-fragment conversion/reversion in
+ * {@link ClassConverter#convertInternal}, which must preserve the {@code ;}
+ * delimiters between fragments regardless of which fragments are converted,
+ * reverted, or a mix of both.
*/
public class TesterConstants {
@@ -34,4 +41,14 @@ public class TesterConstants {
public static final String JAVA_PRESENT_PATH =
"javax/servlet/CommonGatewayInterface";
public static final String JAVA_NOT_PRESENT_DOT =
"javax.servlet.DoesNotExist";
public static final String JAVA_NOT_PRESENT_PATH =
"javax/servlet/DoesNotExist";
+
+ // One fragment resolves in jakarta (must convert), the other does not
+ // (must revert) - the ';' delimiters between and around them must survive.
+ public static final String MULTI_FRAGMENT_PARTIAL =
+
"(Ljavax/servlet/CommonGatewayInterface;Ljavax/servlet/DoesNotExist;)V";
+
+ // Neither fragment resolves in jakarta, so the whole value must revert to
+ // exactly the original string, delimiters included.
+ public static final String MULTI_FRAGMENT_ALL_MISSING =
+ "(Ljavax/servlet/DoesNotExist;Ljavax/servlet/DoesNotExist;)V";
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
