fix(page): exclude archived content from page render and content save (#35993) (#36215)

dsolistorres · claude · web-flow · commit 527c553dedd4 · 2026-06-20T09:29:04.000Z
## Problem When a contentlet placed in a container on a page is **archived**, it continues to render in the page's **Edit** and **Preview** modes (UVE). It is correctly hidden in **Live** mode. Archived content should not appear on a page in any mode. A secondary symptom: while an archived contentlet remains in a container, removing a different (non-archived) contentlet from the page hangs ("spinning forever"). The browser POST to `/api/v1/page/{id}/content` returns **400 "Can't find contentlet: \<archived-id\>"**. ## Root cause Archiving sets `deleted = true` on the version info but keeps the working inode. In Edit/Preview `PageMode.showLive = false`, so `PageRenderUtil.getSpecificContentlet()` resolves `find(workingInode)` and returns the archived working version — and `populateContainers()` only guarded against `null`. Live mode has no live version, so it was already excluded. Because the archived content kept rendering, it stayed in the editor's page model. On save (a **full replacement**), `PageResource.validateContainerEntries()` looked the id up via `findContentletByIdentifierAnyLanguageAnyVariant()`, which filters out deleted versions, found nothing, and threw `DotContentletStateException` — a `RuntimeException` not caught by the method's `catch (DotDataException)` — surfacing as HTTP 400 and hanging the remove. ## Changes - **`PageRenderUtil.populateContainers()`** — skip `contentlet.isArchived()` in all modes, consistent with Live-mode behavior. (Also corrected a stray `DotStateException` import.) - **`PageResource.validateContainerEntries()`** — skip archived content instead of throwing the 400, so a page that still references archived content remains saveable (the content gets removed on save). - **`PageResourceTest`** — new self-validating integration test `testArchivedContentNotRenderedInEditAndPreviewMode`: renders the page **before** archiving (asserts the content shows, count == 1), archives, then asserts count == 0 in PREVIEW and EDIT modes. ## Testing - Bug reproduced on unfixed core: the new test **failed** `expected:<0> but was:<1>`. - With the fix: the test **passes**. - Full `PageResourceTest` (31 tests) passes against the fixed core with the build cache disabled. ## ⚠️ Reviewer note — intentional behavior change in page-save semantics After this fix, archived content no longer renders on a page in any mode. Because the page-save endpoint (`POST /api/v1/page/{id}/content`) is a **full replacement** of each container's content, the next time a user saves the page the archived content — no longer in the editor's model — is **removed from the page's `multi_tree` association**. **Implication:** if that content is later **unarchived**, it will **not** automatically reappear on the page; it must be re-added manually. We consider this acceptable (archived content shouldn't be bound to live pages), but flagging it explicitly. An alternative — preserving the `multi_tree` association so unarchive restores placement — was considered and rejected as more complex and less intuitive. **Please confirm you're comfortable with the chosen semantics.** Refs: #35993 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/PageRenderUtil.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/services/PageRenderUtil.java
@@ -296,6 +296,26 @@ private List<ContainerRaw> populateContainers() throws DotDataException, DotSecu
                         continue;
                     }
 
+                    // Archived (deleted) content keeps its working version, so a showLive=false
+                    // lookup (EDIT/PREVIEW modes) still resolves it. Skip it in every mode so that
+                    // archived content never renders on the page, consistent with LIVE-mode behavior.
+                    // isArchived() declares DotSecurityException, but VersionableAPI.isDeleted() does
+                    // not throw it via this path (it is declared for forward-compatibility). Should a
+                    // DotSecurityException ever surface, it is a genuine access-control failure and
+                    // must NOT be swallowed as "probably archived" -- so it is intentionally left
+                    // uncaught and propagates to the caller.
+                    try {
+                        if (nonHydratedContentlet.isArchived()) {
+                            Logger.debug(this, () -> "Skipping archived contentlet: "
+                                    + nonHydratedContentlet.getIdentifier());
+                            continue;
+                        }
+                    } catch (final DotStateException | DotDataException e) {
+                        Logger.warn(this, "Could not determine archived state for contentlet '"
+                                + nonHydratedContentlet.getIdentifier() + "'; skipping it", e);
+                        continue;
+                    }
+
                     final DotContentletTransformer transformer = new DotTransformerBuilder()
                             .defaultOptions().content(nonHydratedContentlet).build();
                     final Contentlet contentlet = transformer.hydrate().get(0);
diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/page/PageResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/page/PageResource.java
@@ -86,7 +86,10 @@
 import com.dotmarketing.exception.DotSecurityException;
 import com.dotmarketing.exception.StalePageSaveException;
 import com.dotmarketing.portlets.containers.model.Container;
+import com.dotcms.contenttype.exception.NotFoundInDbException;
+import com.dotcms.exception.ExceptionUtil;
 import com.dotmarketing.portlets.contentlet.business.ContentletAPI;
+import com.dotmarketing.portlets.contentlet.business.DotContentletStateException;
 import com.dotmarketing.portlets.contentlet.model.Contentlet;
 import com.dotmarketing.portlets.contentlet.transform.DotTransformerBuilder;
 import com.dotmarketing.portlets.contentlet.util.ContentletUtil;
@@ -1033,24 +1036,69 @@ protected void validateContainerEntries(final List<ContainerEntry> containerEntr
         for (final ContainerEntry containerEntry : containerEntries) {
 
             final String containerId = containerEntry.getContainerId();
+
+            if (!UtilMethods.isSet(containerId)) {
+                throw new BadRequestException("A container identifier is required");
+            }
+
             final Set<String> contentTypeSet    = containerContentTypesMap.computeIfAbsent(containerId,  key -> this.getContainerContentTypes(containerId));
             final List<String> contentletIdList = containerEntry.getContentIds();
             for (final String contentletId : contentletIdList) {
+
+                if (!UtilMethods.isSet(contentletId)) {
+                    throw new BadRequestException("A contentlet identifier is required for the container");
+                }
+
                 final Contentlet contentlet;
                 try {
+                    // This lookup excludes deleted (archived) versions. It wraps everything it
+                    // catches in a DotContentletStateException, so we must inspect the cause:
+                    // a NotFoundInDbException means the content is archived or does not exist --
+                    // both are skipped silently (treated identically to avoid leaking whether an
+                    // identifier exists, and so a page still referencing archived content stays
+                    // saveable). Any other cause is a genuine failure and must be surfaced rather
+                    // than silently dropping the contentlet. See issue #35993.
                     contentlet = APILocator.getContentletAPI().findContentletByIdentifierAnyLanguageAnyVariant(contentletId);
-                    if (null == contentlet) {
-
-                        throw new BadRequestException("The contentlet: " + contentletId + " does not exists!");
+                } catch (final DotContentletStateException e) {
+                    if (ExceptionUtil.causedBy(e, NotFoundInDbException.class)) {
+                        // Expected/benign case (archived or non-existent). Checks the full cause
+                        // chain (not just the direct cause) so an added intermediate wrapper does
+                        // not silently turn this into a client error. Logged without the exception
+                        // so we don't emit a stack trace -- or the wrapped lookup message -- on
+                        // every page save that references archived content.
+                        Logger.warn(this, "Skipping contentlet '" + contentletId
+                                + "' on page content save: archived or not found");
+                        continue;
                     }
+                    // Genuine lookup/DB failure wrapped as DotContentletStateException: log full
+                    // detail server-side and return a generic message so internal identifiers /
+                    // SQL fragments are not leaked in the response.
+                    Logger.error(this, "Error validating contentlet '" + contentletId
+                            + "' on page content save", e);
+                    throw new BadRequestException("Error validating one or more contentlets for the page");
+                } catch (final DotDataException e) {
+                    // findContentletByIdentifierAnyLanguageAnyVariant declares this checked
+                    // exception (in practice it wraps failures in DotContentletStateException,
+                    // handled above). Handle it the same way so this method throws only unchecked
+                    // exceptions and never forwards a raw message to the client.
+                    Logger.error(this, "Error validating contentlet '" + contentletId
+                            + "' on page content save", e);
+                    throw new BadRequestException("Error validating one or more contentlets for the page");
+                }
 
-                    if (contentlet.getBaseType().get().equals(BaseContentType.CONTENT) && !contentTypeSet.contains(contentlet.getContentType().variable())) {
-
-                        throw new BadRequestException("The content type: " + contentlet.getContentType().variable() + " is not valid for the container");
-                    }
-                } catch (DotDataException e) {
+                if (null == contentlet) {
+                    Logger.warn(this, "Skipping contentlet '" + contentletId
+                            + "' on page content save: working version not found");
+                    continue;
+                }
 
-                    throw new BadRequestException(e, e.getMessage());
+                if (contentlet.getBaseType().get().equals(BaseContentType.CONTENT)
+                        && !contentTypeSet.contains(contentlet.getContentType().variable())) {
+                    // The content-type variable is public schema metadata (already exposed via the
+                    // Content Types API), so it is safe to include and helps the caller identify the
+                    // offending content.
+                    throw new BadRequestException("Content type '" + contentlet.getContentType().variable()
+                            + "' is not allowed in this container");
                 }
             }
         }
@@ -1063,9 +1111,22 @@ private Set<String> getContainerContentTypes (final String containerId) {
                     .orElseThrow(() -> new DoesNotExistException("Container with ID :" + containerId + " not found"));
             final List<ContentType> contentTypes = APILocator.getContainerAPI().getContentTypesInContainer(container);
             return null != contentTypes? contentTypes.stream().map(ContentType::variable).collect(Collectors.toSet()) : Collections.emptySet();
-        } catch (DotDataException | DotSecurityException e) {
-
-            throw new BadRequestException(e, e.getMessage());
+        } catch (final DotSecurityException e) {
+            // The system user lacking read permission on the container is a server-side
+            // misconfiguration, not a client bad request -- surface it as 403 (generic message;
+            // full detail logged server-side) rather than 400.
+            Logger.error(this, "No permission to read container '" + containerId + "'", e);
+            throw new ForbiddenException(e, "Not allowed to read the container");
+        } catch (final DotDataException e) {
+            // Log full detail server-side; return a generic message so internal identifiers /
+            // SQL fragments are not leaked in the response.
+            //
+            // DoesNotExistException is intentionally NOT caught here: a missing container must
+            // surface as a 404 carrying its message. The id in that message is the caller's own
+            // input (not sensitive), and the 404 + message is an established API contract verified
+            // by the Define_Contentlets_StyleProperties postman test.
+            Logger.error(this, "Error retrieving content types for container '" + containerId + "'", e);
+            throw new BadRequestException("Error retrieving content types for the container");
         }
     }
 
diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/page/PageResourceTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/page/PageResourceTest.java
@@ -749,6 +749,87 @@ public void testRenderWithContent() throws DotDataException, DotSecurityExceptio
         assertEquals(pageView.getNumberContents(), 1);
     }
 
+    /**
+     * Method to test: {@link PageResource#loadJson(HttpServletRequest, HttpServletResponse, String, String, String, String, String, String)}
+     * Given Scenario: A page has a container with a single contentlet, and that contentlet is then
+     *                 archived. Archiving keeps the working version (it only sets deleted=true on the
+     *                 version info), so a showLive=false lookup still resolves it in EDIT/PREVIEW mode.
+     * Expected Result: The archived contentlet must NOT be rendered on the page in EDIT or PREVIEW mode,
+     *                 consistent with LIVE-mode behavior. See issue #35993.
+     */
+    @Test
+    public void testArchivedContentNotRenderedInEditAndPreviewMode()
+            throws DotDataException, DotSecurityException {
+
+        final User systemUser = APILocator.getUserAPI().getSystemUser();
+        final long languageId = 1L;
+
+        final ContentType containerContentType = new ContentTypeDataGen().nextPersisted();
+        final Container localContainer = new ContainerDataGen().withContentType(containerContentType, "")
+                .friendlyName("container-archived-friendly-name").title("container-archived-title")
+                .nextPersisted();
+
+        final TemplateLayout templateLayout = TemplateLayoutDataGen.get()
+                .withContainer(localContainer.getIdentifier())
+                .next();
+
+        final Template newTemplate = new TemplateDataGen()
+                .drawedBody(templateLayout)
+                .withContainer(localContainer.getIdentifier())
+                .nextPersisted();
+        APILocator.getVersionableAPI().setWorking(newTemplate);
+        APILocator.getVersionableAPI().setLive(newTemplate);
+
+        final Contentlet checkout = APILocator.getContentletAPI().checkout(pageAsset.getInode(), systemUser, false);
+        checkout.setStringProperty(HTMLPageAssetAPI.TEMPLATE_FIELD, newTemplate.getIdentifier());
+        APILocator.getContentletAPI().checkin(checkout, systemUser, false);
+
+        final ContentTypeAPI contentTypeAPI = APILocator.getContentTypeAPI(systemUser);
+        final ContentType contentGenericType = contentTypeAPI.find("webPageContent");
+
+        final ContentletDataGen contentletDataGen = new ContentletDataGen(contentGenericType.id());
+        final Contentlet contentlet = contentletDataGen.setProperty("title", "title")
+                .setProperty("body", TestDataUtils.BLOCK_EDITOR_DUMMY_CONTENT).languageId(languageId).nextPersisted();
+
+        final MultiTreeAPI multiTreeAPI = APILocator.getMultiTreeAPI();
+        final MultiTree multiTree = new MultiTree(pageAsset.getIdentifier(), localContainer.getIdentifier(),
+                contentlet.getIdentifier(), "1", 1);
+        multiTreeAPI.saveMultiTree(multiTree);
+
+        when(request.getAttribute(WebKeys.HTMLPAGE_LANGUAGE)).thenReturn(String.valueOf(languageId));
+
+        // Baseline: while the contentlet is live/working it must render in PREVIEW mode.
+        // This proves the test setup actually places the content on the page.
+        final int previewCountBeforeArchive = renderAndCountContents(PageMode.PREVIEW_MODE);
+        assertEquals("Content should render in PREVIEW mode before archiving", 1,
+                previewCountBeforeArchive);
+
+        // Archive the contentlet placed in the container
+        APILocator.getContentletAPI().archive(contentlet, systemUser, false);
+        assertTrue("Contentlet should be archived", contentlet.isArchived());
+
+        // PREVIEW_MODE: archived content must not render
+        assertEquals("Archived content must not render in PREVIEW mode", 0,
+                renderAndCountContents(PageMode.PREVIEW_MODE));
+
+        // EDIT_MODE: archived content must not render
+        assertEquals("Archived content must not render in EDIT mode", 0,
+                renderAndCountContents(PageMode.EDIT_MODE));
+    }
+
+    /**
+     * Renders {@code pagePath} in the given {@link PageMode} via {@link PageResource#loadJson} and
+     * returns the number of contentlets placed in the page's containers.
+     */
+    private int renderAndCountContents(final PageMode mode)
+            throws DotDataException, DotSecurityException {
+        final Response response = pageResource
+                .loadJson(request, this.response, pagePath, mode.name(), null, "1", null, null);
+        RestUtilTest.verifySuccessResponse(response);
+        final PageView pageView = (PageView) ((ResponseEntityView) response.getEntity()).getEntity();
+        return pageView.getNumberContents();
+    }
+
     @Test
     public void shouldReturnPageByURLPattern()
             throws DotDataException, DotSecurityException, InterruptedException, SystemException, PortalException {
