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 e5740d1 Use the file threshold pattern for nested archives
e5740d1 is described below
commit e5740d16a3fcc8ae716096964e281a0a91a36af0
Author: remm <remm@meteor>
AuthorDate: Fri Sep 4 15:06:17 2026 +0200
Use the file threshold pattern for nested archives
Improve source and destination manipulation robustness.
Robustness improvements for the fake streams.
From code review.
Improve test coverage to cover all the different cases (coverage is now
good except for unexpected IO exceptions and similar problems).
Co authored with OpenCode.
---
CHANGES.md | 3 +
.../org/apache/tomcat/jakartaee/Migration.java | 360 ++++++++++++++++-----
.../apache/tomcat/jakartaee/MigrationCache.java | 39 ++-
.../tomcat/jakartaee/LocalStrings.properties | 7 +-
.../org/apache/tomcat/jakartaee/MigrationTest.java | 196 +++++++++++
5 files changed, 515 insertions(+), 90 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index aebd1d4..950ac5b 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -5,6 +5,9 @@
- Do not buffer very large STORED zip entries when processing them in
streaming mode. (remm)
- Various minor fixes from code review. (remm)
- Improve logging handling for Ant tasks. (remm)
+- Do not buffer very large nested archives when processing them in streaming
mode. (remm)
+- Improve cache robustness. (remm)
+- Improve robustness of source and destination manipulation operations when
migrating. (remm)
## 1.0.12
- Add Maven Wrapper Plugin to manage the Maven wrapper. (markt)
diff --git a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
index 7db242c..7c695cb 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
@@ -24,7 +24,10 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.attribute.FileTime;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashSet;
@@ -194,8 +197,8 @@ public class Migration {
}
/**
- * Set source file.
- * @param source the source file
+ * Set source file or directory.
+ * @param source the source file or directory
*/
public void setSource(File source) {
if (!source.canRead()) {
@@ -206,8 +209,8 @@ public class Migration {
}
/**
- * Set destination file.
- * @param destination the destination file
+ * Set destination file or directory.
+ * @param destination the destination file or directory
*/
public void setDestination(File destination) {
this.destination = destination;
@@ -254,28 +257,44 @@ public class Migration {
destination.getAbsolutePath(), profile.toString()));
long t1 = System.nanoTime();
+ boolean failed = false;
try {
if (source.isDirectory()) {
- if ((destination.exists() && destination.isDirectory()) ||
destination.mkdirs()) {
- migrateDirectory(source, destination);
- } else {
- throw new IOException(sm.getString("migration.mkdirError",
destination.getAbsolutePath()));
+ if (!destination.exists()) {
+ if (!destination.mkdirs()) {
+ throw new
IOException(sm.getString("migration.mkdirError",
+ destination.getAbsolutePath()));
+ }
+ }
+ if (!destination.isDirectory()) {
+ throw new IOException(sm.getString("migration.mkdirError",
+ destination.getAbsolutePath()));
}
+ migrateDirectory(source, destination);
} else {
// 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()));
+ if (!parentDestination.exists() &&
!parentDestination.mkdirs()) {
+ throw new IOException(sm.getString("migration.mkdirError",
+ parentDestination.getAbsolutePath()));
}
+ migrateFile(source, destination);
}
+ } catch (IOException e) {
+ failed = true;
+ throw e;
} finally {
- state = State.COMPLETE;
+ state = failed ? State.NOT_STARTED : State.COMPLETE;
- // Finalize cache operations (save metadata and prune expired
entries)
+ // Finalize cache operations (save metadata and prune expired
entries).
+ // A failure here must not mask a migration failure or cause a
+ // successful migration to be reported as failed.
if (cache != null) {
- cache.pruneCache();
+ try {
+ cache.pruneCache();
+ } catch (IOException e) {
+ logger.log(Level.WARNING,
sm.getString("migration.cachePruneFailed"), e);
+ }
}
}
@@ -284,17 +303,25 @@ public class Migration {
}
private void migrateDirectory(File src, File dest) throws IOException {
- // Won't return null because src is known to be a directory
+ // May return null if src ceases to be a directory (e.g. it is
+ // removed) between the isDirectory() check and this call
String[] files = src.list();
+ if (files == null) {
+ throw new IOException(sm.getString("migration.listError",
src.getAbsolutePath()));
+ }
for (String file : files) {
File srcFile = new File(src, file);
File destFile = new File(dest, profile.convert(file));
if (srcFile.isDirectory()) {
- if ((destFile.exists() && destFile.isDirectory()) ||
destFile.mkdir()) {
- migrateDirectory(srcFile, destFile);
- } else {
+ if (!destFile.exists()) {
+ if (!destFile.mkdir()) {
+ throw new
IOException(sm.getString("migration.mkdirError", destFile.getAbsolutePath()));
+ }
+ }
+ if (!destFile.isDirectory()) {
throw new IOException(sm.getString("migration.mkdirError",
destFile.getAbsolutePath()));
}
+ migrateDirectory(srcFile, destFile);
} else {
migrateFile(srcFile, destFile);
}
@@ -337,7 +364,13 @@ public class Migration {
} else {
try (InputStream is = new FileInputStream(src);
OutputStream os = new FileOutputStream(dest)) {
- converted = migrateStream(src.getAbsolutePath(), is, os);
+ if (migrateStream(src.getAbsolutePath(), is, os)) {
+ converted = true;
+ }
+ } catch (IOException e) {
+ // Remove the partially written destination file
+ dest.delete();
+ throw e;
}
}
}
@@ -454,41 +487,54 @@ public class Migration {
Util.copy(src, dest);
logger.log(Level.INFO, sm.getString("migration.skip", name));
} else if (isArchive(name)) {
- // Only cache nested archives (e.g., JARs inside WARs), not
top-level files
- // Top-level files will have absolute paths starting with a path
separator
- boolean isNestedArchive = !name.startsWith("/") &&
!name.startsWith("\\");
+ // Only cache nested archives (e.g., JARs inside WARs), not
top-level
+ // files which will have absolute paths
+ boolean isNestedArchive = !new File(name).isAbsolute();
CacheEntry cacheEntry = null;
+ SourceSpool sourceSpool = null;
if (isNestedArchive && cache != null) {
- // Buffer source to compute hash and check cache
- ByteArrayOutputStream buffer = new ByteArrayOutputStream();
- IOUtils.copy(src, buffer);
- byte[] sourceBytes = buffer.toByteArray();
+ // Spool source so the cache hash can be computed and, on a
cache
+ // miss, the source can be re-read for conversion. Data above
+ // TEMP_FILE_THRESHOLD is spooled to a temp file to avoid
+ // unbounded memory usage.
+ sourceSpool = new SourceSpool(profile);
+ try {
+ IOUtils.copy(src, sourceSpool);
+ } catch (IOException e) {
+ sourceSpool.discard();
+ throw e;
+ }
+ String hash = sourceSpool.getHash();
- // Get cache entry (computes hash and marks as accessed)
- cacheEntry = cache.getCacheEntry(sourceBytes, profile);
+ // Get cache entry (marks as accessed)
+ cacheEntry = cache.getCacheEntry(hash);
if (cacheEntry.exists()) {
- // Cache hit! Copy cached result to dest and return
- logger.log(Level.INFO, sm.getString("cache.hit", name,
cacheEntry.getHash()));
- cacheEntry.copyToDestination(dest);
+ try {
+ // Cache hit! Copy cached result to dest and return
+ logger.log(Level.INFO, sm.getString("cache.hit", name,
hash));
+ cacheEntry.copyToDestination(dest);
+ } finally {
+ sourceSpool.discard();
+ }
// Although it is from the cache, this still counts as
converting the source
return true;
}
- // Cache miss - use buffered source for conversion
- logger.log(Level.FINE, sm.getString("cache.miss", name,
cacheEntry.getHash()));
- src = new ByteArrayInputStream(sourceBytes);
+ // Cache miss - use spooled source for conversion
+ logger.log(Level.FINE, sm.getString("cache.miss", name, hash));
+ src = sourceSpool.toInputStream();
}
// Process archive - stream directly to destination (and cache if
needed)
- OutputStream targetOutputStream = dest;
- if (cacheEntry != null) {
- // Tee output to both destination and cache temp file
- targetOutputStream = new
org.apache.commons.io.output.TeeOutputStream(dest, cacheEntry.beginStore());
- }
-
try {
+ OutputStream targetOutputStream = dest;
+ if (cacheEntry != null) {
+ // Tee output to both destination and cache temp file
+ targetOutputStream = new
org.apache.commons.io.output.TeeOutputStream(dest, cacheEntry.beginStore());
+ }
+
if (zipInMemory) {
logger.log(Level.INFO,
sm.getString("migration.archive.memory", name));
convertedStream = migrateArchiveInMemory(src,
targetOutputStream);
@@ -505,12 +551,19 @@ public class Migration {
logger.log(Level.FINE, sm.getString("cache.store",
cacheEntry.getHash(),
Long.valueOf(cacheEntry.getFileSize())));
}
- } catch (IOException e) {
+ } catch (Exception e) {
// Rollback cache on error
if (cacheEntry != null) {
cacheEntry.rollbackStore();
}
+ if (e instanceof IOException) {
+ throw (IOException) e;
+ }
throw e;
+ } finally {
+ if (sourceSpool != null) {
+ sourceSpool.discard();
+ }
}
} else {
for (Converter converter : converters) {
@@ -564,29 +617,20 @@ public class Migration {
}
/**
- * 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
- * buffer to a temporary file to avoid excessive memory usage. Used for
computing
- * CRC and size of STORED zip entries during streaming migration.
+ * An output stream that spools written data into an in-memory buffer,
+ * switching to a temporary file once the data exceeds
+ * TEMP_FILE_THRESHOLD, to avoid unbounded memory usage. Subclasses track
+ * properties of the spooled data (e.g., a checksum) using
+ * {@link #update(int)} and {@link #update(byte[], int, int)} and consume
+ * or release the spooled data using {@link #writeTo(OutputStream)},
+ * {@link #toInputStream()} or {@link #discard()}.
*/
- private static class CrcSizeTrackingOutputStream extends OutputStream {
+ private abstract static class SpoolingOutputStream extends OutputStream {
- private final CRC32 crc = new CRC32();
- private long size;
- private final OutputStream destStream;
private ByteArrayOutputStream buffer = new ByteArrayOutputStream();
private FileOutputStream fileOutput;
private File tempFile;
-
- /**
- * Create stream that computes its bytes as CRC while streaming them
- * to the specified stream on close.
- * @param destStream the destination stream to write the bytes to
- */
- CrcSizeTrackingOutputStream(OutputStream destStream) {
- super();
- this.destStream = destStream;
- }
+ private FileInputStream tempFileIs;
@Override
public void write(int b) throws IOException {
@@ -596,8 +640,7 @@ public class Migration {
buffer.write(b);
maybeSwitchToFile();
}
- crc.update(b);
- size++;
+ update(b);
}
@Override
@@ -608,12 +651,27 @@ public class Migration {
buffer.write(b, off, len);
maybeSwitchToFile();
}
- crc.update(b, off, len);
- size += len;
+ update(b, off, len);
}
+ /**
+ * Track the written byte.
+ * @param b the byte that was written
+ * @throws IOException if an I/O error occurs
+ */
+ protected abstract void update(int b) throws IOException;
+
+ /**
+ * Track the written bytes.
+ * @param b the bytes that were written
+ * @param off the starting offset in the byte array
+ * @param len the number of bytes that were written
+ * @throws IOException if an I/O error occurs
+ */
+ protected abstract void update(byte[] b, int off, int len) throws
IOException;
+
private void maybeSwitchToFile() throws IOException {
- if (buffer != null && buffer.size() > TEMP_FILE_THRESHOLD &&
fileOutput == null) {
+ if (buffer.size() > TEMP_FILE_THRESHOLD && fileOutput == null) {
tempFile = createTempFile();
tempFile.deleteOnExit();
fileOutput = new FileOutputStream(tempFile);
@@ -623,35 +681,175 @@ public class Migration {
}
}
- public long getSize() {
- return size;
- }
-
- public long getCrc() {
- return crc.getValue();
- }
-
- @Override
- public void close() throws IOException {
+ /**
+ * Write the spooled data to the specified stream and release the
+ * spooled data.
+ * @param dest the stream to write the spooled data to
+ * @throws IOException if an I/O error occurs
+ */
+ protected void writeTo(OutputStream dest) throws IOException {
if (fileOutput != null) {
+ IOException closeException = null;
try {
fileOutput.close();
+ } catch (IOException e) {
+ closeException = e;
} finally {
fileOutput = null;
- try (FileInputStream fis = new FileInputStream(tempFile)) {
- IOUtils.copy(fis, destStream);
- } finally {
- tempFile.delete();
- }
+ }
+ try (FileInputStream fis = new FileInputStream(tempFile)) {
+ IOUtils.copy(fis, dest);
+ } finally {
+ tempFile.delete();
+ tempFile = null;
+ }
+ if (closeException != null) {
+ throw closeException;
}
} else if (buffer != null) {
try {
- buffer.writeTo(destStream);
+ buffer.writeTo(dest);
} finally {
buffer.close();
buffer = null;
}
}
}
+
+ /**
+ * Get an input stream over the spooled data. The spooled data is
+ * retained until {@link #discard()} is called.
+ * @return an input stream over the spooled data
+ * @throws IOException if an I/O error occurs
+ */
+ protected InputStream toInputStream() throws IOException {
+ if (fileOutput != null) {
+ fileOutput.close();
+ fileOutput = null;
+ tempFileIs = new FileInputStream(tempFile);
+ return tempFileIs;
+ }
+ return new ByteArrayInputStream(buffer.toByteArray());
+ }
+
+ /**
+ * Release all spooled data. Safe to call multiple times.
+ */
+ protected void discard() {
+ if (fileOutput != null) {
+ try {
+ fileOutput.close();
+ } catch (IOException e) {
+ // Ignore
+ }
+ fileOutput = null;
+ }
+ if (tempFileIs != null) {
+ try {
+ tempFileIs.close();
+ } catch (IOException e) {
+ // Ignore
+ }
+ tempFileIs = null;
+ }
+ if (tempFile != null) {
+ tempFile.delete();
+ tempFile = null;
+ }
+ buffer = null;
+ }
+ }
+
+ /**
+ * Output stream that tracks the CRC32 checksum and byte count of written
+ * data, spooling to a temporary file when the data exceeds
+ * TEMP_FILE_THRESHOLD to avoid excessive memory usage. On close, the
+ * spooled data is written to the destination stream. Used for computing
+ * the CRC and size of STORED zip entries during streaming migration.
+ */
+ private static class CrcSizeTrackingOutputStream extends
SpoolingOutputStream {
+
+ private final CRC32 crc = new CRC32();
+ private long size;
+ private final OutputStream destStream;
+
+ /**
+ * Create stream that computes the CRC and size of its bytes as they
+ * are written and writes those bytes to the specified stream on
+ * close.
+ * @param destStream the destination stream to write the bytes to
+ */
+ CrcSizeTrackingOutputStream(OutputStream destStream) {
+ this.destStream = destStream;
+ }
+
+ @Override
+ protected void update(int b) {
+ crc.update(b);
+ size++;
+ }
+
+ @Override
+ protected void update(byte[] b, int off, int len) {
+ crc.update(b, off, len);
+ size += len;
+ }
+
+ public long getSize() {
+ return size;
+ }
+
+ public long getCrc() {
+ return crc.getValue();
+ }
+
+ @Override
+ public void close() throws IOException {
+ writeTo(destStream);
+ }
+ }
+
+ /**
+ * Spools archive source data while computing the SHA-256 hash used to
+ * key the migration cache. The hash includes the profile name and must
+ * be computed the same way as MigrationCache computes cache hashes.
+ */
+ private static class SourceSpool extends SpoolingOutputStream {
+
+ private final MessageDigest digest;
+
+ SourceSpool(EESpecProfile profile) throws IOException {
+ try {
+ digest = MessageDigest.getInstance("SHA-256");
+ // Include profile name in hash to differentiate between
profiles
+
digest.update(profile.toString().getBytes(StandardCharsets.UTF_8));
+ } catch (NoSuchAlgorithmException e) {
+ throw new IOException(sm.getString("cache.hashError"), e);
+ }
+ }
+
+ @Override
+ protected void update(int b) {
+ digest.update((byte) b);
+ }
+
+ @Override
+ protected void update(byte[] b, int off, int len) {
+ digest.update(b, off, len);
+ }
+
+ /**
+ * Get the hash of the spooled data. Must only be called once all data
+ * has been written.
+ * @return the hash as a hex string
+ */
+ String getHash() {
+ byte[] hashBytes = digest.digest();
+ StringBuilder sb = new StringBuilder();
+ for (byte b : hashBytes) {
+ sb.append(String.format("%02x", Integer.valueOf(b & 0xFF)));
+ }
+ return sb.toString();
+ }
}
}
diff --git a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
index a890f9f..436a822 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/MigrationCache.java
@@ -35,6 +35,7 @@ import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
+import java.util.regex.Pattern;
/**
* Cache for storing and retrieving pre-converted archive files.
@@ -76,8 +77,10 @@ public class MigrationCache {
private static final Logger logger =
Logger.getLogger(MigrationCache.class.getCanonicalName());
private static final StringManager sm =
StringManager.getManager(MigrationCache.class);
+
private static final String METADATA_FILE = "cache-metadata.txt";
private static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ISO_LOCAL_DATE;
+ private static final Pattern HASH_PATTERN =
Pattern.compile("[0-9a-f]{64}");
private final File cacheDir;
private final int retentionDays;
@@ -88,13 +91,18 @@ public class MigrationCache {
* Construct a new migration cache.
*
* @param cacheDir the directory to store cached files
- * @param retentionDays the number of days to retain cached files
+ * @param retentionDays the number of days to retain cached files (minimum
1)
+ * @throws IllegalArgumentException if cacheDir is null or retentionDays is
+ * less than 1
* @throws IOException if the cache directory cannot be created
*/
public MigrationCache(File cacheDir, int retentionDays) throws IOException
{
if (cacheDir == null) {
throw new
IllegalArgumentException(sm.getString("cache.nullDirectory"));
}
+ if (retentionDays < 1) {
+ throw new
IllegalArgumentException(sm.getString("cache.invalidRetentionDays",
Integer.valueOf(retentionDays)));
+ }
this.retentionDays = retentionDays;
this.cacheMetadata = new ConcurrentHashMap<>();
@@ -170,6 +178,10 @@ public class MigrationCache {
String[] parts = line.split("\\|");
if (parts.length == 2) {
String hash = parts[0];
+ if (!HASH_PATTERN.matcher(hash).matches()) {
+ logger.log(Level.WARNING,
sm.getString("cache.metadata.invalidLine", line));
+ continue;
+ }
try {
LocalDate lastAccessed = LocalDate.parse(parts[1],
DATE_FORMATTER);
cacheMetadata.put(hash, lastAccessed);
@@ -240,10 +252,22 @@ public class MigrationCache {
* @return a CacheEntry object with all operations for this entry
* @throws IOException if an I/O error occurs
*/
- public CacheEntry getCacheEntry(byte[] sourceBytes, EESpecProfile profile)
throws IOException {
+ public synchronized CacheEntry getCacheEntry(byte[] sourceBytes,
EESpecProfile profile) throws IOException {
// Compute hash once (includes profile)
- String hash = computeHash(sourceBytes, profile);
+ return getCacheEntry(computeHash(sourceBytes, profile));
+ }
+
+ /**
+ * Get a cache entry for a pre-computed hash.
+ *
+ * @param hash the pre-computed hash of the profile name and the
+ * pre-conversion archive content (computed the same way as
+ * {@link #computeHash(byte[], EESpecProfile)})
+ * @return a CacheEntry object with all operations for this entry
+ * @throws IOException if an I/O error occurs
+ */
+ public synchronized CacheEntry getCacheEntry(String hash) throws
IOException {
// Get cache file location
File cachedFile = getCacheFile(hash);
boolean exists = cachedFile.exists();
@@ -308,7 +332,7 @@ public class MigrationCache {
*
* @throws IOException if an I/O error occurs
*/
- public void clear() throws IOException {
+ public synchronized void clear() throws IOException {
deleteDirectory(cacheDir);
cacheMetadata.clear();
if (!cacheDir.mkdirs() && !cacheDir.exists()) {
@@ -372,7 +396,7 @@ public class MigrationCache {
*
* @throws IOException if an I/O error occurs
*/
- public void pruneCache() throws IOException {
+ public synchronized void pruneCache() throws IOException {
LocalDate cutoffDate = LocalDate.now().minusDays(retentionDays);
int prunedCount = 0;
long prunedSize = 0;
@@ -420,12 +444,13 @@ public class MigrationCache {
/**
* Finalize cache operations - save metadata and perform cleanup.
- * Should be called after migration completes.
*
* @throws IOException if an I/O error occurs
+ * @deprecated Use {@link #pruneCache()} instead. This method is retained
+ * for backward compatibility but performs redundant
operations.
*/
@Deprecated
- public void finalizeCacheOperations() throws IOException {
+ public synchronized void finalizeCacheOperations() throws IOException {
// Save updated metadata
saveMetadata();
diff --git
a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
index 783a361..d4d0958 100644
--- a/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
+++ b/src/main/resources/org/apache/tomcat/jakartaee/LocalStrings.properties
@@ -16,12 +16,14 @@
cacheEntry.rollbackDeleteFailed=Failed to delete temporary cache file [{0}]
during rollback
classConverter.converted=Migrated class [{0}]
+classConverter.invalidClass=Invalid class file [{0}]
classConverter.noConversion=No conversion necessary for [{0}]
classConverter.skipName=Skip conversion of class usage from the [{0}]
namespace to [{1}] as it is not accessible to the classloader
migration.archive.complete=Migration finished for archive [{0}]
migration.archive.memory=Migration starting for archive [{0}] using in memory
copy
migration.archive.stream=Migration starting for archive [{0}] using streaming
+migration.cachePruneFailed=Failed to finalize the migration cache
migration.cannotReadSource=Cannot read source location [{0}]
migration.notCompleted=Migration has not completed
migration.alreadyRunning=Migration is already running
@@ -29,6 +31,7 @@ migration.done=Migration completed successfully in [{0}]
milliseconds
migration.error=Error performing migration
migration.execute=Performing migration from source [{0}] to destination [{1}]
with Jakarta EE specification profile [{2}]
migration.jdk8303866=Due to size of [{0}], migrated JAR will fail if used in a
JDK without the fix for https://bugs.openjdk.org/browse/JDK-8303866 - Using an
in memory migration rather than a streaming migration may work-around the issue.
+migration.listError=Error listing source directory [{0}]
migration.mkdirError=Error creating destination directory [{0}]
migration.skip=Migration skipped for archive [{0}] because it is excluded (the
archive was copied unchanged)
migration.skipSignatureFile=Drop cryptographic signature file [{0}]
@@ -69,8 +72,6 @@ where options includes:\n\
\ Number of days to retain cached files (default: 30, minimum:
1).\n\
\ Cache entries not accessed within this period will be removed.
-migration.warnSignatureRemoval=Removed cryptographic signature from JAR file
-
migrationTask.invalidProfile=Specified profile [{0}] is invalid
migrationTask.noDest=No destination parameter specified
migrationTask.noSource=Invalid or missing source [{0}] specified
@@ -81,6 +82,7 @@ textConverter.converted=Migrated text file [{0}]
textConverter.noConversion=No conversion necessary for [{0}]
manifestConverter.converted=Migrated manifest file [{0}]
+manifestConverter.manifestError=Unable to parse manifest for [{0}], no
conversion performed
manifestConverter.updated=Updated manifest file [{0}]
manifestConverter.updatedVersion=Updated manifest version to [{0}]
manifestConverter.removeSignature=Remove cryptographic signature for [{0}]
@@ -92,6 +94,7 @@ cache.notDirectory=[{0}] is not a directory
cache.nullDirectory=The cache storage directory may not be null
cache.enabled=Migration cache enabled at [{0}] with {1} day retention period
cache.hit=Cache hit for archive [{0}] (hash: {1})
+cache.invalidRetentionDays=retentionDays must be at least 1, was [{0}]
cache.miss=Cache miss for archive [{0}] (hash: {1})
cache.store=Stored converted archive in cache (hash: {0}, size: {1} bytes)
cache.hashError=Error computing hash for cache
diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
index 17e7af0..c275d39 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
@@ -789,6 +789,131 @@ public class MigrationTest {
}
}
+ @Test
+ public void testMigrateNestedLargeArchiveWithCacheStreaming() throws
Exception {
+ // A nested archive larger than TEMP_FILE_THRESHOLD (10MB) forces the
+ // SourceSpool to switch from its in-memory buffer to a temp file
+ // while computing the cache hash and to re-read the spooled source
+ // from that temp file on a cache miss.
+ byte[] largeContent = new byte[11 * 1024 * 1024]; // 11MB
+ for (int i = 0; i < largeContent.length; i++) {
+ largeContent[i] = (byte) (i % 256);
+ }
+ File largeNestedJar = createLargeNestedJar(largeContent);
+ assertTrue("Nested JAR should exceed the temp file threshold",
+ largeNestedJar.length() > 10 * 1024 * 1024);
+
+ File warFile = createWarWithNestedJar(largeNestedJar,
"large-spooled.war");
+ File cacheDir = tempFolder.newFolder("large-spooled-cache");
+ File warTarget = tempFolder.newFile("large-spooled-migrated.war");
+
+ Migration migration = new Migration();
+ migration.setSource(warFile);
+ migration.setDestination(warTarget);
+ migration.setCache(new MigrationCache(cacheDir, 30));
+ migration.setZipInMemory(false); // Streaming mode
+ migration.execute();
+
+ assertTrue("Target WAR should exist", warTarget.exists());
+ assertTrue("Nested JAR should have been converted",
migration.hasConverted());
+
+ // Cache miss: the converted nested JAR should be stored in the cache
+ // using the hash computed from the spooled (temp file) source
+ assertNotNull("Converted nested JAR should be stored in the cache",
findCachedJar(cacheDir));
+
+ verifyNestedJarContentMigrated(warTarget, "WEB-INF/lib/nested.jar",
"jakarta.servlet");
+ verifyLargeNestedEntryPreserved(warTarget, largeContent);
+ }
+
+ @Test
+ public void testMigrateNestedLargeArchiveWithCacheInMemory() throws
Exception {
+ // As testMigrateNestedLargeArchiveWithCacheStreaming but with
+ // in-memory archive processing, which also spools the nested archive
+ // to a temp file once it exceeds TEMP_FILE_THRESHOLD (10MB).
+ byte[] largeContent = new byte[11 * 1024 * 1024]; // 11MB
+ for (int i = 0; i < largeContent.length; i++) {
+ largeContent[i] = (byte) (i % 256);
+ }
+ File largeNestedJar = createLargeNestedJar(largeContent);
+ assertTrue("Nested JAR should exceed the temp file threshold",
+ largeNestedJar.length() > 10 * 1024 * 1024);
+
+ File warFile = createWarWithNestedJar(largeNestedJar,
"large-spooled-memory.war");
+ File cacheDir = tempFolder.newFolder("large-spooled-memory-cache");
+ File warTarget =
tempFolder.newFile("large-spooled-memory-migrated.war");
+
+ Migration migration = new Migration();
+ migration.setSource(warFile);
+ migration.setDestination(warTarget);
+ migration.setCache(new MigrationCache(cacheDir, 30));
+ migration.setZipInMemory(true); // In-memory mode
+ migration.execute();
+
+ assertTrue("Target WAR should exist", warTarget.exists());
+ assertTrue("Nested JAR should have been converted",
migration.hasConverted());
+
+ assertNotNull("Converted nested JAR should be stored in the cache",
findCachedJar(cacheDir));
+
+ verifyNestedJarContentMigrated(warTarget, "WEB-INF/lib/nested.jar",
"jakarta.servlet");
+ verifyLargeNestedEntryPreserved(warTarget, largeContent);
+ }
+
+ @Test
+ public void testMigrateNestedLargeArchiveWithCacheHit() throws Exception {
+ // Two WARs with the same nested JAR larger than TEMP_FILE_THRESHOLD
+ // (10MB). The first migration is a cache miss that stores the
+ // converted nested JAR. The second migration must hit the cache using
+ // a hash computed from the temp file based SourceSpool.
+ byte[] largeContent = new byte[11 * 1024 * 1024]; // 11MB
+ for (int i = 0; i < largeContent.length; i++) {
+ largeContent[i] = (byte) (i % 256);
+ }
+ File largeNestedJar = createLargeNestedJar(largeContent);
+
+ File warFile1 = createWarWithNestedJar(largeNestedJar,
"large-hit-1.war");
+ File cacheDir = tempFolder.newFolder("large-hit-cache");
+
+ // First migration - cache miss
+ File warTarget1 = tempFolder.newFile("large-hit-1-migrated.war");
+ Migration migration1 = new Migration();
+ migration1.setSource(warFile1);
+ migration1.setDestination(warTarget1);
+ MigrationCache cache = new MigrationCache(cacheDir, 30);
+ migration1.setCache(cache);
+ migration1.setZipInMemory(false);
+ migration1.execute();
+
+ assertTrue("First target WAR should exist", warTarget1.exists());
+ File cachedJar = findCachedJar(cacheDir);
+ assertNotNull("Converted nested JAR should be stored in the cache",
cachedJar);
+
+ // Replace the cached content with a canary: if the second migration
+ // hits the cache, the nested JAR in the second WAR must be a byte
+ // for byte copy of the canary.
+ byte[] canary = "cached large nested
jar".getBytes(StandardCharsets.ISO_8859_1);
+ Files.write(cachedJar.toPath(), canary);
+
+ // Second migration - should hit the cache for the nested JAR
+ File warFile2 = createWarWithNestedJar(largeNestedJar,
"large-hit-2.war");
+ File warTarget2 = tempFolder.newFile("large-hit-2-migrated.war");
+ Migration migration2 = new Migration();
+ migration2.setSource(warFile2);
+ migration2.setDestination(warTarget2);
+ migration2.setCache(cache);
+ migration2.setZipInMemory(false);
+ migration2.execute();
+
+ assertTrue("Second target WAR should exist", warTarget2.exists());
+
+ // First WAR must have the migrated nested content
+ verifyNestedJarContentMigrated(warTarget1, "WEB-INF/lib/nested.jar",
"jakarta.servlet");
+
+ // Second WAR's nested JAR must be served from the cache (the canary)
+ File nestedJarInWar = extractNestedJarFromWar(warTarget2,
"WEB-INF/lib/nested.jar");
+ assertArrayEquals("Nested JAR should be served from the cache", canary,
+ Files.readAllBytes(nestedJarInWar.toPath()));
+ }
+
private File createWarWithNestedJar(File nestedJar, String warName) throws
Exception {
File warFile = tempFolder.newFile(warName);
byte[] nestedJarBytes = Files.readAllBytes(nestedJar.toPath());
@@ -818,6 +943,77 @@ public class MigrationTest {
return nestedJar;
}
+ private File createLargeNestedJar(byte[] largeContent) throws Exception {
+ // Create a nested JAR with a large STORED entry (so the JAR exceeds
+ // TEMP_FILE_THRESHOLD) plus a text entry with javax references so the
+ // migration converts the archive
+ File largeNestedJar = tempFolder.newFile("large-nested.jar");
+ try (FileOutputStream fos = new FileOutputStream(largeNestedJar);
+
org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream zos =
+ new
org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream(fos)) {
+ org.apache.commons.compress.archivers.zip.ZipArchiveEntry
largeEntry =
+ new
org.apache.commons.compress.archivers.zip.ZipArchiveEntry("large-data.bin");
+
largeEntry.setMethod(org.apache.commons.compress.archivers.zip.ZipArchiveEntry.STORED);
+ largeEntry.setSize(largeContent.length);
+ CRC32 crc = new CRC32();
+ crc.update(largeContent);
+ largeEntry.setCrc(crc.getValue());
+ zos.putArchiveEntry(largeEntry);
+ zos.write(largeContent);
+ zos.closeArchiveEntry();
+
+ org.apache.commons.compress.archivers.zip.ZipArchiveEntry
textEntry =
+ new
org.apache.commons.compress.archivers.zip.ZipArchiveEntry("nested.txt");
+ zos.putArchiveEntry(textEntry);
+
zos.write("javax.servlet.http.HttpServlet".getBytes(StandardCharsets.ISO_8859_1));
+ zos.closeArchiveEntry();
+ }
+ return largeNestedJar;
+ }
+
+ private File extractNestedJarFromWar(File warFile, String nestedEntryName)
throws Exception {
+ try (JarFile war = new JarFile(warFile)) {
+ JarEntry nestedEntry = war.getJarEntry(nestedEntryName);
+ assertNotNull("Nested JAR should exist in " + warFile.getName(),
nestedEntry);
+ byte[] nestedJarBytes =
readAllBytes(war.getInputStream(nestedEntry), (int) nestedEntry.getSize());
+ File nestedJar = File.createTempFile("nested-large", ".jar");
+ nestedJar.deleteOnExit();
+ Files.write(nestedJar.toPath(), nestedJarBytes);
+ return nestedJar;
+ }
+ }
+
+ private void verifyLargeNestedEntryPreserved(File warFile, byte[]
expectedLargeContent) throws Exception {
+ File nestedJarInWar = extractNestedJarFromWar(warFile,
"WEB-INF/lib/nested.jar");
+ try (JarFile nestedJar = new JarFile(nestedJarInWar)) {
+ JarEntry largeEntry = nestedJar.getJarEntry("large-data.bin");
+ assertNotNull("Large entry should exist in the migrated nested
JAR", largeEntry);
+ assertEquals("Large entry size should be preserved",
expectedLargeContent.length,
+ largeEntry.getSize());
+ byte[] readContent =
readAllBytes(nestedJar.getInputStream(largeEntry), (int) largeEntry.getSize());
+ assertArrayEquals("Large entry content should be preserved",
expectedLargeContent, readContent);
+ }
+ }
+
+ private File findCachedJar(File cacheDir) {
+ File[] subdirs = cacheDir.listFiles();
+ if (subdirs != null) {
+ for (File subdir : subdirs) {
+ if (subdir.isDirectory()) {
+ File[] files = subdir.listFiles();
+ if (files != null) {
+ for (File file : files) {
+ if (file.isFile() &&
file.getName().endsWith(".jar")) {
+ return file;
+ }
+ }
+ }
+ }
+ }
+ }
+ return null;
+ }
+
private void verifyHelloCGIMigrated(File jarFileTarget) throws Exception {
File cgiapiFile = new File("target/test-classes/cgi-api.jar");
try (URLClassLoader classloader = new URLClassLoader(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
