From 302818b2d23a4fad91829b7c6a7a6c2f45137f6f Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 14 Aug 2026 13:22:25 -0600 Subject: [PATCH 1/3] fix(contentlet): stop timestamp preservation from failing binary copies on shared storage (#37068) Copying a contentlet with a binary field aborted on NFS/EFS-backed asset volumes with "Cannot set the file time.". ESContentletAPIImpl.copyContentlet staged each binary through the two-arg commons-io FileUtils.copyFile, which defaults to preserveFileDate=true. The bytes were already copied at that point; only the trailing setTimes call failed, and commons-io turns that into an IOException. It surfaced as a WorkflowActionFailureException, killing the Copy action and any bulk copy that included the content. The destination is a brand new file under a throwaway UUID temp folder, so the source's modification time carries no meaning. Pass preserveFileDate = false, and REPLACE_EXISTING explicitly: that overload takes CopyOption as a varargs which would otherwise default to none, and Files.copy fails when the destination already exists - which happens when two binary fields on the same content type hold files with the same name. Real I/O failures still propagate as DotDataException. FileSystemStoragePersistenceAPIImpl.pushFile had the same defect writing into the asset bucket on the same shared volume, and is fixed alongside it. Co-Authored-By: Claude Opus 5 (1M context) --- .../business/ESContentletAPIImpl.java | 14 ++- .../FileSystemStoragePersistenceAPIImpl.java | 6 +- .../business/ESContentletAPIImplTest.java | 101 ++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index f98b62f4f97a..7834a46f0cef 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -210,6 +210,7 @@ import java.io.Serializable; import java.math.BigDecimal; import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -9579,10 +9580,15 @@ public Contentlet copyContentlet(final Contentlet sourceContentlet, fieldValue = srcFile.getName(); } destFile = new File(temporalFolder + File.separator + fieldValue); - if (!destFile.exists()) { - destFile.createNewFile(); - } - FileUtils.copyFile(srcFile, destFile); + // Do NOT preserve the source's file date. The destination is a brand + // new file under a throwaway temp folder, so its modification time + // carries no meaning, and preserving it fails outright on shared + // storage (NFS/EFS) where setting file times is not permitted, which + // would abort the whole copy even though the bytes were copied fine. + // REPLACE_EXISTING must be passed explicitly: this overload defaults + // to no CopyOption, and Files.copy fails if the destination exists. + FileUtils.copyFile(srcFile, destFile, false, + StandardCopyOption.REPLACE_EXISTING); newContentlet.setBinary(tempField.getVelocityVarName(), destFile); } } catch (final Exception e) { diff --git a/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java b/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java index 5dd05f066f79..a38893f9e60c 100644 --- a/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/storage/FileSystemStoragePersistenceAPIImpl.java @@ -27,6 +27,7 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -180,7 +181,10 @@ public Object pushFile(final String groupName, try { final File destBucketFile = Paths.get(groupDir.getCanonicalPath(), path.toLowerCase()).toFile(); - FileUtils.copyFile(file, destBucketFile); + // Do NOT preserve the source's file date: on shared storage (NFS/EFS) setting file + // times is not permitted and would fail the push even though the bytes were + // written fine. REPLACE_EXISTING keeps the previous overwrite semantics. + FileUtils.copyFile(file, destBucketFile, false, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e) { Logger.error(FileSystemStoragePersistenceAPIImpl.class, e.getMessage(), e); throw new DotDataException(e.getMessage(), e); diff --git a/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java b/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java index 700476a39e70..14d18253fa17 100644 --- a/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImplTest.java @@ -3506,6 +3506,107 @@ public void test_copy_contentlet() throws DotDataException, DotSecurityException assertEquals(respCont.getHost(), APILocator.systemHost().getIdentifier()); } + /** + * Method to test: {@link ESContentletAPIImpl#copyContentlet(Contentlet, User, boolean)} + * Given Scenario: A {@link ContentType} with a {@link BinaryField} is copied. + * ExpectedResult: The copy carries its own binary, byte-identical to the source. The copy must + * not depend on preserving the source file's modification time, which is not settable on + * shared storage (NFS/EFS) and used to abort the whole copy with "Cannot set the file time." + */ + @Test + public void copyContentletWithBinaryFieldKeepsFileContent() + throws DotDataException, DotSecurityException, IOException { + final Field binaryField = new FieldDataGen() + .name("binary") + .velocityVarName("binary") + .type(BinaryField.class) + .next(); + + final ContentType contentType = new ContentTypeDataGen() + .fields(list(binaryField)) + .nextPersisted(); + + final File testFile = createFile("images/test.jpg", ".jpg"); + final Contentlet contentlet = new ContentletDataGen(contentType.id()) + .setProperty(binaryField.variable(), testFile) + .nextPersisted(); + + final Contentlet copy = contentletAPI.copyContentlet(contentlet, user, false); + + assertNotEquals(contentlet.getIdentifier(), copy.getIdentifier()); + + final File copiedFile = APILocator.getContentletAPI() + .find(copy.getInode(), APILocator.systemUser(), false) + .getBinary(binaryField.variable()); + + assertNotNull("The copy must have its own binary file", copiedFile); + assertNotEquals("The copy must not point at the source file", + testFile.getAbsolutePath(), copiedFile.getAbsolutePath()); + FileTestUtil.compare(testFile, copiedFile); + } + + /** + * Method to test: {@link ESContentletAPIImpl#copyContentlet(Contentlet, User, boolean)} + * Given Scenario: A {@link ContentType} with two {@link BinaryField}s whose files share the + * same name. Both are staged into the same temporary folder, so the second copy writes to a + * path the first one already created. + * ExpectedResult: The copy succeeds. This guards the {@code REPLACE_EXISTING} option on the + * underlying copy call — without it the second field fails with FileAlreadyExistsException. + */ + @Test + public void copyContentletWithTwoBinaryFieldsSharingFileName() + throws DotDataException, DotSecurityException, IOException { + final Field firstBinaryField = new FieldDataGen() + .name("firstBinary") + .velocityVarName("firstBinary") + .type(BinaryField.class) + .next(); + final Field secondBinaryField = new FieldDataGen() + .name("secondBinary") + .velocityVarName("secondBinary") + .type(BinaryField.class) + .next(); + + final ContentType contentType = new ContentTypeDataGen() + .fields(list(firstBinaryField, secondBinaryField)) + .nextPersisted(); + + // Same file name, different folders: both land on the same path in the temp staging folder + final String sharedFileName = "shared_" + System.currentTimeMillis() + ".jpg"; + final File firstFile = createFileNamed("images/test.jpg", sharedFileName); + final File secondFile = createFileNamed("images/test.jpg", sharedFileName); + + final Contentlet contentlet = new ContentletDataGen(contentType.id()) + .setProperty(firstBinaryField.variable(), firstFile) + .setProperty(secondBinaryField.variable(), secondFile) + .nextPersisted(); + + final Contentlet copy = contentletAPI.copyContentlet(contentlet, user, false); + + assertNotEquals(contentlet.getIdentifier(), copy.getIdentifier()); + + final Contentlet copyFromDataBase = APILocator.getContentletAPI() + .find(copy.getInode(), APILocator.systemUser(), false); + + assertNotNull(copyFromDataBase.getBinary(firstBinaryField.variable())); + assertNotNull(copyFromDataBase.getBinary(secondBinaryField.variable())); + } + + /** + * Copies a classpath resource into a fresh temp folder under an explicit file name, so two + * files can deliberately share a name while living in different folders. + */ + private static File createFileNamed(final String path, final String fileName) + throws IOException { + final File originalFile = new File(Thread.currentThread() + .getContextClassLoader().getResource(path).getFile()); + + final File testFile = new File(Files.createTempDirectory("dotcms-test").toFile(), fileName); + FileUtil.copyFile(originalFile, testFile); + + return testFile; + } + /** * Method to test: {@link ESContentletAPIImpl#copyContentlet(Contentlet, User, boolean)} From 8f67fd8ce1275ef55717f912119e4c4c4a6a8242 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 14 Aug 2026 14:32:00 -0600 Subject: [PATCH 2/3] chore(contentlet): add temporary marker log before the binary copy (#37068) Deploy verification aid. The reported "Cannot set the file time." stack was traced to a build that predates the fix, so this logs a greppable marker immediately before FileUtils.copyFile in copyContentlet. If the copy fails and the marker is absent from dotcms.log, the running image does not contain the fix. If the marker is present, the failure is something else and the log carries the exact source and destination paths. To be reverted before merge. Co-Authored-By: Claude Opus 5 (1M context) --- .../content/elasticsearch/business/ESContentletAPIImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index 7834a46f0cef..b838da07f8de 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -9587,6 +9587,10 @@ public Contentlet copyContentlet(final Contentlet sourceContentlet, // would abort the whole copy even though the bytes were copied fine. // REPLACE_EXISTING must be passed explicitly: this overload defaults // to no CopyOption, and Files.copy fails if the destination exists. + Logger.warn(this, "### DOTCMS-37068-MARKER ### copying binary '" + + srcFile.getAbsolutePath() + "' -> '" + + destFile.getAbsolutePath() + + "' with preserveFileDate=false"); FileUtils.copyFile(srcFile, destFile, false, StandardCopyOption.REPLACE_EXISTING); newContentlet.setBinary(tempField.getVelocityVarName(), destFile); From ae3cf1891e2072ccf4c4c3e21f0ae0c62abb8378 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 14 Aug 2026 15:17:51 -0600 Subject: [PATCH 3/3] Revert "chore(contentlet): add temporary marker log before the binary copy (#37068)" This reverts commit 8f67fd8ce1275ef55717f912119e4c4c4a6a8242. --- .../content/elasticsearch/business/ESContentletAPIImpl.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index b838da07f8de..7834a46f0cef 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -9587,10 +9587,6 @@ public Contentlet copyContentlet(final Contentlet sourceContentlet, // would abort the whole copy even though the bytes were copied fine. // REPLACE_EXISTING must be passed explicitly: this overload defaults // to no CopyOption, and Files.copy fails if the destination exists. - Logger.warn(this, "### DOTCMS-37068-MARKER ### copying binary '" - + srcFile.getAbsolutePath() + "' -> '" - + destFile.getAbsolutePath() - + "' with preserveFileDate=false"); FileUtils.copyFile(srcFile, destFile, false, StandardCopyOption.REPLACE_EXISTING); newContentlet.setBinary(tempField.getVelocityVarName(), destFile);