fix(content-import): snapshot servlet request before async handoff to prevent recycling crash

[dsolistorres]

Summary

• Fixes Content Import Fails Silently When Importing Large CSV Files (Struts Request Recycling Issue Causes Backend Exception) #33908 — Large CSV imports fail silently because Tomcat recycles the RequestFacade before the async import thread finishes processing.
• Moved MockAttributeRequest/MockSessionRequest/MockHeaderRequest construction from _processFile() (runs on the background thread) to processAction() (runs on the HTTP thread while the request is still alive).
• Replaced _handleException(ae, req) inside the async Runnable catch block with Logger.error() to avoid accessing the recycled request on error paths.

Test plan

• Import a CSV with 500+ rows using a content type with many fields (18+) — verify all rows are imported successfully
• Import a small CSV (~50 rows) — verify it still works as before
• Check backend logs for absence of IllegalStateException: The request object has been recycled
• Verify IMPORT_CONTENTLETS_ASYNC=true (default) triggers the async path

Closes #33908

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes the async content import execution path and its transaction/audit updates; mistakes could leave imports incorrectly marked or hide/over-report failures, especially under load.

Overview
Fixes async content imports by snapshotting the HttpServletRequest (headers/session/attributes via Mock*Request wrappers) in processAction() before handing work to the background thread, and passing the safe request into _processFile().

Adds an import_audit FAILED status (STATUS_FAILED) plus setAuditRecordAsFailed(), and updates the async import error/finally handling to log exceptions, attempt HibernateUtil.rollbackTransaction(), and mark audit records as failed (instead of always completed) when the import throws.

^{Reviewed by Cursor Bugbot for commit 8395b5a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[claude]

Claude finished @dsolistorres's task in 4m 39s —— View job

---

🔍 dotCMS Backend Review

• Load dotCMS conventions
• Get Java diff
• Run security review
• Run database review
• Run Java standards review
• Run REST API review
• Consolidate findings and post results

Review posted. Found 2 High and 2 Medium issues across the 4 specialized reviewers. See the backend review comment for details.

[claude]

Rollback safety analysis complete. Label AI: Safe To Rollback applied. No unsafe categories matched — this is a pure Java thread-safety fix with no DB/ES/API/OSGi changes.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2c3f0b8. Configure here.}

[claude]

Rollback safety analysis complete (re-run for new commits). Label AI: Safe To Rollback confirmed.

No unsafe categories matched for the 2 new commits (2c3f0b8d, f3a6691b):

• No schema changes — setAuditRecordAsFailed() uses existing import_audit columns (status INT, errors TEXT) already defined in the schema; STATUS_FAILED = 40 is just a new constant in an existing integer column
• No ES mapping changes
• No API contract changes — changes are internal Struts action classes only
• No runonce tasks or data migrations

This is a pure Java thread-safety fix (moving request snapshot to HTTP thread) plus improved error handling in the async import path.

[claude]

🔍 dotCMS Backend Review

[🟡 Medium] ImportContentletsAction.java:296 / ImportAuditUtil.java:311

ae.getMessage() is stored verbatim into import_audit.errors and surfaced in the audit UI. Exception messages can contain SQL constraint-violation text (embedding table/column names), internal class names, server file paths, or user-supplied CSV cell values that triggered a constraint error. This column is readable by any admin with import access.

importErrorMessage = ae.getMessage();   // ImportContentletsAction.java:296
// ...
dc.addParam(UtilMethods.isSet(errorMessage) && errorMessage.length() > 500  // ImportAuditUtil.java:311
        ? errorMessage.substring(0, 500) : errorMessage);

💡 Use ae.toString() (includes class name without stack) or a generic fallback: ae.getMessage() != null ? ae.getMessage() : ae.getClass().getSimpleName() + ": (no message)". The full stack trace is already logged via Logger.error so no diagnostic info is lost.

---

[🟡 Medium] ImportContentletsAction.java:296

ae.getMessage() returns null for bare exceptions like NullPointerException. When null is passed to setAuditRecordAsFailed, UtilMethods.isSet(null) is false so dc.addParam(null) stores NULL in the errors column. The admin UI shows no error message, making silent failures impossible to diagnose without checking server logs.

importErrorMessage = ae.getMessage();  // null for NullPointerException
// ...
ImportAuditUtil.setAuditRecordAsFailed(importId, importErrorMessage);  // passes null

💡 Fallback: importErrorMessage = ae.getMessage() != null ? ae.getMessage() : ae.getClass().getSimpleName();

---

[🟡 Medium] ImportContentletsAction.java:301–307

When an import is both cancelled and failed simultaneously, the finally block removes the cancelledImports entry and skips setAuditRecordAsFailed. The import status stays at STATUS_USERSTOPPED and the failure reason is silently discarded — only logged. This is a new edge-case introduced by this PR's branched finally logic.

if(ImportAuditUtil.cancelledImports.containsKey(importId)){
    ImportAuditUtil.cancelledImports.remove(importId);
} else if(importFailed){          // never reached if cancelled+failed
    ImportAuditUtil.setAuditRecordAsFailed(importId, importErrorMessage);

💡 Separate the concerns: always remove the cancelled entry, then independently check importFailed:

ImportAuditUtil.cancelledImports.remove(importId);
if (importFailed) {
    ImportAuditUtil.setAuditRecordAsFailed(importId, importErrorMessage);
}

---

Next steps

• 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in <File.java>
• Every new push triggers a fresh review automatically

[cursor]

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[claude]

🔍 dotCMS Backend Review

[🟠 High] ImportContentletsAction.java — finally block (~line 299)

cancelledImports.remove(importId) is now called unconditionally before checking importFailed. The old code used containsKey to detect user-cancelled imports and skipped setAuditRecordCompleted for them (leaving STATUS_USERSTOPPED = 30 intact). The new code removes the key first, then always calls either setAuditRecordCompleted or setAuditRecordAsFailed, overwriting STATUS_USERSTOPPED with STATUS_COMPLETED for every successfully-finishing cancelled import. Users who stopped an import will see it as "Completed" in the UI.

ImportAuditUtil.cancelledImports.remove(importId);   // key gone before check
if(importFailed){
    ImportAuditUtil.setAuditRecordAsFailed(importId, importErrorMessage);
} else {
    ImportAuditUtil.setAuditRecordCompleted(importId); // overwrites USERSTOPPED
}

💡 Snapshot the flag before removing, then branch on it:

boolean wasCancelled = ImportAuditUtil.cancelledImports.containsKey(importId);
ImportAuditUtil.cancelledImports.remove(importId);
if (importFailed) {
    ImportAuditUtil.setAuditRecordAsFailed(importId, importErrorMessage);
} else if (!wasCancelled) {
    ImportAuditUtil.setAuditRecordCompleted(importId);
}
// if wasCancelled && !importFailed: STATUS_USERSTOPPED already written, leave it

---

[🟠 High] ImportContentletsAction.java:296 — raw exception message exposed to UI

ae.getMessage() is stored verbatim into import_audit.errors, which is then loaded and surfaced to the UI via loadImportResults. Arbitrary exceptions (JDBC errors, file I/O failures, class-cast errors) can contain SQL fragments, table/column names, internal file paths, or server internals. The full exception is already logged server-side via Logger.error, so nothing diagnostic is lost.

importErrorMessage = ae.getMessage() != null
        ? ae.getMessage() : ae.getClass().getSimpleName();

💡 Store a user-safe summary; keep detailed logging for the server:

Logger.error(this, "Import failed for ID " + importId + ": " + ae.getMessage(), ae);
importErrorMessage = "Import failed due to an internal error. Check server logs for details.";

---

[🟡 Medium] ImportAuditUtil.java:316 — setAuditRecordAsFailed swallows DB exception silently

The @WrapInTransaction method catches all exceptions internally and logs them, returning normally even when the UPDATE fails. The caller in the finally block has no indication the audit record was not updated, leaving the import's status unchanged (no terminal state written to DB). This pattern exists in sibling methods (setAuditRecordCompleted, cancelImport), but those are best-effort operations; marking as FAILED is a critical error-path operation where silent failure is more harmful.

} catch (Exception e) {
    Logger.error(ImportAuditUtil.class, e.getMessage(), e);
    // caller sees normal return; audit record stays in prior state
}

💡 Either propagate the exception (letting the outer catch in ImportContentletsAction handle it) or add an explicit log line noting the audit record could not be marked as failed.

---

[🟡 Medium] ImportAuditUtil.java:309 — magic number 500 in error truncation

The truncation limit is inlined without explanation. It is inconsistent with the named constants (STATUS_PENDING, STATUS_COMPLETED, etc.) used elsewhere in the same class, and makes the limit invisible to callers.

dc.addParam(UtilMethods.isSet(errorMessage) && errorMessage.length() > 500
        ? errorMessage.substring(0, 500) : errorMessage);

💡 Add a named constant: private static final int MAX_ERROR_LENGTH = 500;

---

Next steps

• 🟠 Fix locally and push — these need your judgment
• 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in <File.java>
• Every new push triggers a fresh review automatically