This is an automated email from the ASF dual-hosted git repository.
rmaucher 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 1a2742b Fix additional minor issues
1a2742b is described below
commit 1a2742bb7953a3c4c8a1001832174dc1039598be
Author: remm <[email protected]>
AuthorDate: Tue Jun 9 10:10:27 2026 +0200
Fix additional minor issues
Also CodeQL fixes.
---
.../org/apache/tomcat/jakartaee/CacheEntry.java | 5 +-
.../apache/tomcat/jakartaee/ManifestConverter.java | 6 +-
.../org/apache/tomcat/jakartaee/Migration.java | 47 ++++---
.../apache/tomcat/jakartaee/MigrationCache.java | 2 +-
.../apache/tomcat/jakartaee/AntHandlerTest.java | 5 +
.../tomcat/jakartaee/MigrationCacheTest.java | 18 ++-
.../org/apache/tomcat/jakartaee/MigrationTest.java | 136 +++++++++++++--------
7 files changed, 135 insertions(+), 84 deletions(-)
diff --git a/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
b/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
index 0c6e127..eed7794 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/CacheEntry.java
@@ -100,7 +100,8 @@ class CacheEntry {
try {
fos.close();
} catch (IOException e) {
- logger.log(Level.WARNING,
sm.getString("cacheEntry.closeFail"), e);
+ rollbackStore();
+ throw new IOException(sm.getString("cacheEntry.closeFail"), e);
}
fos = null;
}
@@ -110,6 +111,7 @@ class CacheEntry {
// Ensure parent directory exists
File parentDir = cacheFile.getParentFile();
if (!parentDir.mkdirs() && !parentDir.exists()) {
+ rollbackStore();
throw new IOException(sm.getString("cache.cannotCreate",
parentDir.getAbsolutePath()));
}
// Move file to final cache location (atomic if possible)
@@ -121,6 +123,7 @@ class CacheEntry {
Files.move(tempFile.toPath(), cacheFile.toPath(),
StandardCopyOption.REPLACE_EXISTING);
}
} catch (Exception e) {
+ rollbackStore();
throw new IOException(sm.getString("cacheEntry.tempRenameFail",
tempFile, cacheFile), e);
}
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
b/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
index 9196c98..aadb9b0 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
@@ -178,16 +178,16 @@ public class ManifestConverter implements Converter {
private String processOSGIHeader(String value, String header, String
replacement) throws BundleException {
List<String> packages = new ArrayList<>();
ManifestElement[] elements = ManifestElement.parseHeader(header,
value);
+ boolean modified = false;
for (ManifestElement element : elements) {
if (element.getValue().startsWith(JAKARTA_SERVLET)) {
String oldVersion =
element.getAttribute(Constants.VERSION_ATTRIBUTE);
if (oldVersion != null) {
- // Only replace the version attribute, not occurrences of
the version
- // string in other attributes or directives
String escaped = Pattern.quote(oldVersion);
String result =
element.toString().replaceFirst("(;version=\\\")" + escaped + "(\\\")",
"$1" + replacement + "$2");
packages.add(result);
+ modified = true;
} else {
packages.add(element.toString());
}
@@ -195,7 +195,7 @@ public class ManifestConverter implements Converter {
packages.add(element.toString());
}
}
- if (packages.isEmpty()) {
+ if (!modified) {
return value;
}
return String.join(",", packages);
diff --git a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
index bebae17..7db242c 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
@@ -254,26 +254,29 @@ public class Migration {
destination.getAbsolutePath(), profile.toString()));
long t1 = System.nanoTime();
- if (source.isDirectory()) {
- if ((destination.exists() && destination.isDirectory()) ||
destination.mkdirs()) {
- migrateDirectory(source, destination);
- } else {
- throw new IOException(sm.getString("migration.mkdirError",
destination.getAbsolutePath()));
- }
- } else {
- // Single file
- File parentDestination =
destination.getAbsoluteFile().getParentFile();
- if (parentDestination.exists() || parentDestination.mkdirs()) {
- migrateFile(source, destination);
+ try {
+ if (source.isDirectory()) {
+ if ((destination.exists() && destination.isDirectory()) ||
destination.mkdirs()) {
+ migrateDirectory(source, destination);
+ } else {
+ throw new IOException(sm.getString("migration.mkdirError",
destination.getAbsolutePath()));
+ }
} else {
- throw new IOException(sm.getString("migration.mkdirError",
parentDestination.getAbsolutePath()));
+ // Single file
+ File parentDestination =
destination.getAbsoluteFile().getParentFile();
+ if (parentDestination.exists() || parentDestination.mkdirs()) {
+ migrateFile(source, destination);
+ } else {
+ throw new IOException(sm.getString("migration.mkdirError",
parentDestination.getAbsolutePath()));
+ }
}
- }
- state = State.COMPLETE;
+ } finally {
+ state = State.COMPLETE;
- // Finalize cache operations (save metadata and prune expired entries)
- if (cache != null) {
- cache.pruneCache();
+ // Finalize cache operations (save metadata and prune expired
entries)
+ if (cache != null) {
+ cache.pruneCache();
+ }
}
logger.log(Level.INFO, sm.getString("migration.done",
@@ -302,7 +305,7 @@ public class Migration {
if (src.equals(dest)) {
if (src.length() > TEMP_FILE_THRESHOLD) {
// For very large files, use a temp file instead of memory
- File tempFile = File.createTempFile("jakartaee-migration-",
".tmp");
+ File tempFile = createTempFile();
tempFile.deleteOnExit();
try (InputStream is = new FileInputStream(src); OutputStream
os = new FileOutputStream(tempFile)) {
if (migrateStream(src.getAbsolutePath(), is, os)) {
@@ -512,7 +515,7 @@ public class Migration {
} else {
for (Converter converter : converters) {
if (converter.accepts(name)) {
- convertedStream = converter.convert(name, src, dest,
profile);
+ convertedStream = converter.convert(name, src, dest,
profile);
break;
}
}
@@ -556,6 +559,10 @@ public class Migration {
}
}
+ private static File createTempFile() throws IOException {
+ return File.createTempFile("jakartaee-migration-", ".tmp");
+ }
+
/**
* Output stream that tracks the CRC32 checksum and byte count of written
data.
* For data exceeding TEMP_FILE_THRESHOLD, automatically switches from an
in-memory
@@ -607,7 +614,7 @@ public class Migration {
private void maybeSwitchToFile() throws IOException {
if (buffer != null && buffer.size() > TEMP_FILE_THRESHOLD &&
fileOutput == null) {
- tempFile = File.createTempFile("jakartaee-migration-", ".tmp");
+ tempFile = createTempFile();
tempFile.deleteOnExit();
fileOutput = new FileOutputStream(tempFile);
buffer.writeTo(fileOutput);
diff --git a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
index 0cf5f32..a890f9f 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
@@ -93,7 +93,7 @@ public class MigrationCache {
*/
public MigrationCache(File cacheDir, int retentionDays) throws IOException
{
if (cacheDir == null) {
- throw new
IllegalStateException(sm.getString("cache.nullDirectory"));
+ throw new
IllegalArgumentException(sm.getString("cache.nullDirectory"));
}
this.retentionDays = retentionDays;
diff --git a/src/test/java/org/apache/tomcat/jakartaee/AntHandlerTest.java
b/src/test/java/org/apache/tomcat/jakartaee/AntHandlerTest.java
index 7ac22cb..bcf4e95 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/AntHandlerTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/AntHandlerTest.java
@@ -189,5 +189,10 @@ public class AntHandlerTest {
logLevels.add(level);
logThrown.add(throwable);
}
+
+ @Override
+ public TestTask clone() {
+ return new TestTask();
+ }
}
}
diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
b/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
index ac41d57..7ac1864 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationCacheTest.java
@@ -250,7 +250,7 @@ public class MigrationCacheTest {
try {
new MigrationCache(null, 30);
fail("Should throw IllegalStateException for null directory");
- } catch (IllegalStateException e) {
+ } catch (IllegalArgumentException e) {
assertTrue("Error message should mention null",
e.getMessage().contains("null") || e.getMessage().contains("Null"));
}
}
@@ -258,7 +258,7 @@ public class MigrationCacheTest {
@Test
public void testCacheNotDirectory() throws Exception {
File regularFile = new File(tempCacheDir, "regular-file.txt");
- regularFile.createNewFile();
+ Files.createFile(regularFile.toPath());
try {
new MigrationCache(regularFile, 30);
@@ -332,12 +332,11 @@ public class MigrationCacheTest {
public void testCacheTempFileCleanup() throws Exception {
// Create a temp file that should be cleaned up
File tempFile = new File(tempCacheDir, "temp-" +
java.util.UUID.randomUUID() + ".tmp");
- tempFile.createNewFile();
+ Files.createFile(tempFile.toPath());
assertTrue("Temp file should exist before cleanup", tempFile.exists());
// Create cache - should clean up temp files
- @SuppressWarnings("unused")
- MigrationCache unused = new MigrationCache(tempCacheDir, 30);
+ new MigrationCache(tempCacheDir, 30);
assertFalse("Temp file should be cleaned up on cache init",
tempFile.exists());
}
@@ -396,8 +395,7 @@ public class MigrationCacheTest {
}
// Should handle corrupt metadata gracefully
- @SuppressWarnings("unused")
- MigrationCache unused = new MigrationCache(tempCacheDir, 30);
+ new MigrationCache(tempCacheDir, 30);
}
@Test
@@ -411,8 +409,7 @@ public class MigrationCacheTest {
}
// Should handle invalid dates gracefully
- @SuppressWarnings("unused")
- MigrationCache unused = new MigrationCache(tempCacheDir, 30);
+ new MigrationCache(tempCacheDir, 30);
}
@Test
@@ -470,6 +467,7 @@ public class MigrationCacheTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testCacheFinalizeCacheOperations() throws Exception {
MigrationCache cache = new MigrationCache(tempCacheDir, 30);
@@ -496,7 +494,7 @@ public class MigrationCacheTest {
File subdir = new File(tempCacheDir, "ab");
subdir.mkdirs();
File cachedFile = new File(subdir, "abcdef1234567890.jar");
- cachedFile.createNewFile();
+ Files.createFile(cachedFile.toPath());
MigrationCache cache = new MigrationCache(tempCacheDir, 30);
String stats = cache.getStats();
diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
index 45d7a9c..256eeb0 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
@@ -29,6 +29,7 @@ import java.util.zip.CRC32;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
+import org.apache.commons.compress.archivers.zip.ZipFile;
import org.apache.commons.io.FileUtils;
import org.junit.After;
import org.junit.Assert;
@@ -794,22 +795,30 @@ public class MigrationTest {
}
// Parse the nested JAR from bytes
- org.apache.commons.compress.archivers.zip.ZipFile nestedZipFile =
new org.apache.commons.compress.archivers.zip.ZipFile(
- new
org.apache.commons.compress.utils.SeekableInMemoryByteChannel(nestedJarBytes));
- org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
- nestedZipFile.getEntry("nested.txt");
- assertNotNull("nested.txt should exist in nested JAR",
nestedTextEntry);
-
- byte[] nestedTextBytes = new byte[(int) nestedTextEntry.getSize()];
- try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
- is.read(nestedTextBytes);
+ File tempNestedJar = File.createTempFile("nested", ".jar");
+ tempNestedJar.deleteOnExit();
+ Files.write(tempNestedJar.toPath(), nestedJarBytes);
+ try (org.apache.commons.compress.archivers.zip.ZipFile
nestedZipFile =
+ ZipFile.builder().setFile(tempNestedJar).get()) {
+ org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
+ nestedZipFile.getEntry("nested.txt");
+ assertNotNull("nested.txt should exist in nested JAR",
nestedTextEntry);
+
+ byte[] nestedTextBytes = new byte[(int)
nestedTextEntry.getSize()];
+ try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
+ int totalRead = 0;
+ while (totalRead < nestedTextBytes.length) {
+ int read = is.read(nestedTextBytes, totalRead,
nestedTextBytes.length - totalRead);
+ if (read == -1) break;
+ totalRead += read;
+ }
+ }
+ String migratedNestedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
+ assertTrue("Nested content should be migrated",
+ migratedNestedContent.contains("jakarta.servlet"));
+ assertFalse("Nested content should not contain javax",
+ migratedNestedContent.contains("javax.servlet"));
}
- String migratedNestedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
- assertTrue("Nested content should be migrated",
- migratedNestedContent.contains("jakarta.servlet"));
- assertFalse("Nested content should not contain javax",
- migratedNestedContent.contains("javax.servlet"));
- nestedZipFile.close();
}
}
@@ -873,18 +882,26 @@ public class MigrationTest {
}
}
- org.apache.commons.compress.archivers.zip.ZipFile
nestedZipFile = new org.apache.commons.compress.archivers.zip.ZipFile(
- new
org.apache.commons.compress.utils.SeekableInMemoryByteChannel(nestedJarBytes));
- org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
- nestedZipFile.getEntry("nested.txt");
- byte[] nestedTextBytes = new byte[(int)
nestedTextEntry.getSize()];
- try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
- is.read(nestedTextBytes);
+ File tempNestedJar = File.createTempFile("nested", ".jar");
+ tempNestedJar.deleteOnExit();
+ Files.write(tempNestedJar.toPath(), nestedJarBytes);
+ try (org.apache.commons.compress.archivers.zip.ZipFile
nestedZipFile =
+ ZipFile.builder().setFile(tempNestedJar).get()) {
+ org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
+ nestedZipFile.getEntry("nested.txt");
+ byte[] nestedTextBytes = new byte[(int)
nestedTextEntry.getSize()];
+ try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
+ int totalRead = 0;
+ while (totalRead < nestedTextBytes.length) {
+ int read = is.read(nestedTextBytes, totalRead,
nestedTextBytes.length - totalRead);
+ if (read == -1) break;
+ totalRead += read;
+ }
+ }
+ String migratedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
+ assertTrue("Nested content should be migrated in " +
warTarget.getName(),
+ migratedContent.contains("jakarta.servlet"));
}
- String migratedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
- assertTrue("Nested content should be migrated in " +
warTarget.getName(),
- migratedContent.contains("jakarta.servlet"));
- nestedZipFile.close();
}
}
}
@@ -958,7 +975,12 @@ public class MigrationTest {
byte[] textBytes = new byte[(int) textEntry.getSize()];
try (InputStream is = jar.getInputStream(textEntry)) {
- is.read(textBytes);
+ int totalRead = 0;
+ while (totalRead < textBytes.length) {
+ int read = is.read(textBytes, totalRead, textBytes.length
- totalRead);
+ if (read == -1) break;
+ totalRead += read;
+ }
}
String migratedText = new String(textBytes,
StandardCharsets.ISO_8859_1);
assertTrue("Text should be migrated",
migratedText.contains("jakarta.servlet"));
@@ -1059,18 +1081,26 @@ public class MigrationTest {
}
}
- org.apache.commons.compress.archivers.zip.ZipFile nestedZipFile =
new org.apache.commons.compress.archivers.zip.ZipFile(
- new
org.apache.commons.compress.utils.SeekableInMemoryByteChannel(nestedJarBytes));
- org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
- nestedZipFile.getEntry("nested.txt");
- byte[] nestedTextBytes = new byte[(int) nestedTextEntry.getSize()];
- try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
- is.read(nestedTextBytes);
+ File tempNestedJar = File.createTempFile("nested", ".jar");
+ tempNestedJar.deleteOnExit();
+ Files.write(tempNestedJar.toPath(), nestedJarBytes);
+ try (org.apache.commons.compress.archivers.zip.ZipFile
nestedZipFile =
+ ZipFile.builder().setFile(tempNestedJar).get()) {
+ org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
+ nestedZipFile.getEntry("nested.txt");
+ byte[] nestedTextBytes = new byte[(int)
nestedTextEntry.getSize()];
+ try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
+ int totalRead = 0;
+ while (totalRead < nestedTextBytes.length) {
+ int read = is.read(nestedTextBytes, totalRead,
nestedTextBytes.length - totalRead);
+ if (read == -1) break;
+ totalRead += read;
+ }
+ }
+ String migratedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
+ assertTrue("Nested content should be migrated",
+ migratedContent.contains("jakarta.servlet"));
}
- String migratedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
- assertTrue("Nested content should be migrated",
- migratedContent.contains("jakarta.servlet"));
- nestedZipFile.close();
}
}
@@ -1115,18 +1145,26 @@ public class MigrationTest {
}
}
- org.apache.commons.compress.archivers.zip.ZipFile nestedZipFile =
new org.apache.commons.compress.archivers.zip.ZipFile(
- new
org.apache.commons.compress.utils.SeekableInMemoryByteChannel(nestedJarBytes));
- org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
- nestedZipFile.getEntry("nested.txt");
- byte[] nestedTextBytes = new byte[(int) nestedTextEntry.getSize()];
- try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
- is.read(nestedTextBytes);
+ File tempNestedJar = File.createTempFile("nested", ".jar");
+ tempNestedJar.deleteOnExit();
+ Files.write(tempNestedJar.toPath(), nestedJarBytes);
+ try (org.apache.commons.compress.archivers.zip.ZipFile
nestedZipFile =
+ ZipFile.builder().setFile(tempNestedJar).get()) {
+ org.apache.commons.compress.archivers.zip.ZipArchiveEntry
nestedTextEntry =
+ nestedZipFile.getEntry("nested.txt");
+ byte[] nestedTextBytes = new byte[(int)
nestedTextEntry.getSize()];
+ try (InputStream is =
nestedZipFile.getInputStream(nestedTextEntry)) {
+ int totalRead = 0;
+ while (totalRead < nestedTextBytes.length) {
+ int read = is.read(nestedTextBytes, totalRead,
nestedTextBytes.length - totalRead);
+ if (read == -1) break;
+ totalRead += read;
+ }
+ }
+ String migratedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
+ assertTrue("Nested content should be migrated",
+ migratedContent.contains("jakarta.servlet"));
}
- String migratedContent = new String(nestedTextBytes,
StandardCharsets.ISO_8859_1);
- assertTrue("Nested content should be migrated",
- migratedContent.contains("jakarta.servlet"));
- nestedZipFile.close();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
