CLARIN-DSpace v9/Port #1409 + #1407 + #1408 (filter-media WARN, SWORDv2 double-delete guard, LDAP hardening) to the v9 base (#1430)

* Port #1409 to dtq-dev-9-base: fix(mediafilter): log unparsable-PDF filter-media errors as WARN, not ERROR (#1409)

Source: b0c4850 (dtq-dev PR #1409)

A corrupt or malformed PDF makes PDFBoxThumbnail and TikaTextExtractionFilter
throw a parse IOException that MediaFilterServiceImpl re-logs at ERROR, flooding
the nightly filter-media job and tripping log-based alerting even though the job
already skips the file and continues. Both filters now catch the IOException,
log at WARN and return null, so the bitstream is skipped cleanly. The encrypted
-PDF branch (InvalidPasswordException) stays at ERROR.

Applied verbatim -- both files are byte-identical with vanilla 9.3 on this
branch, and the v9 `Loader.loadPDF(new RandomAccessReadBuffer(source))` rewrite
did not disturb the catch chain the new block attaches to. No test on either
branch covers this path (the source PR has none either).

Co-authored-by: MatusBeke <matus.beke7@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Port #1407 to dtq-dev-9-base: [Port to dtq-dev] fix: SWORDv2 item double-delete with WorkflowManagerDefault (#1407)

Source: f241c47 (dtq-dev PR #1407)

ContainerManagerDSpace.doContainerDelete() now checks, before the final
itemService.delete(), whether a DELETE event for the same item UUID is already
queued on the context -- i.e. the item was deleted earlier in this transaction --
and skips the second delete. Historically that second delete threw when the item
came in through WorkflowManagerDefault.

Both delete paths keep deleteWrapper(); the guard is the explicit safety net the
source PR settled on, in its NPE-hardened form
(itemUUID.equals(event.getSubjectID()), constrained to Event.DELETE on
Constants.ITEM subjects). As the source PR states, the current base does not
double-delete on any path, so this is behaviour-neutral today.

Applied verbatim: the file is byte-identical with vanilla 9.3 on this branch and
Context.getEvents() / Event.getEventType / getSubjectType / getSubjectID are
unchanged in v9. The new java.util.UUID import lands after java.util.TreeMap and
org.dspace.event.Event after org.dspace.core.LogHelper, so checkstyle import
order (com < jakarta < org) is preserved.

Known gap carried over from the source PR: the negative branch of the guard
(delete skipped because a DELETE event is already queued) has no test on any
branch. The two existing Swordv2IT delete tests only cover the positive path.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Port #1408 to dtq-dev-9-base: [Port to dtq-dev] fix: harden LDAP auth (groupmap guard, DN logging) (#1408)

Source: 6615b01 (dtq-dev PR #1408)

Two of the source commit's three hunks are ported; the third is already here.

Ported (both in assignGroups()):
- A groupmap entry without a ':' separator, or with a blank left or right part,
  no longer reaches `t[1]` (ArrayIndexOutOfBoundsException) and no longer leaves
  an empty ldapSearchString -- which made containsIgnoreCase(dn, "" + ",") match
  essentially every DN and assign the mapped group to all LDAP users. The entry
  is now logged at ERROR with its index and skipped, and scanning continues at
  the next index. Parsing uses split(":", 2) so a colon inside a DSpace group
  name is preserved instead of truncated.
- System.out.println("dn:" + dn) becomes
  log.debug(LogHelper.getHeader(context, "assignGroups", "dn=" + dn)) -- a
  semi-sensitive DN off the default log level and out of stdout.

NOT ported -- VANILLA-COVERED: the e-mail fallback hunk (the 5-arg
setEpersonAttributes overload plus its two call sites). That fix went upstream as
23b999e (PRs DSpace#11293/DSpace#11331, authored by DataQuest) and is in dspace-9.3:
  git merge-base --is-ancestor 23b999e dspace-9.3   -> 0
  git diff dspace-9.3 origin/dtq-dev-9-base -- .../LDAPAuthentication.java  -> empty
On this branch setEpersonAttributes already has both overloads, both call sites
already pass `email`, and `StringUtils.isNotEmpty(email)` occurs exactly twice --
the same count as on dtq-dev. Re-applying the hunk would have duplicated it.

Added beyond the source commit: LDAPAuthenticationTest, 5 unit tests over the
groupmap parsing (no LDAP server; ConfigurationService and GroupService mocked,
assignGroups reached by reflection because it is private). The source PR lists
the missing test under its own "Open points"; card BE-06's AC-3 needs exactly
this behaviour proven, and with no LDAP server anywhere in the estate a unit test
is the only way to prove it. Worth backporting to dtq-dev.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: jurinecko <juraj.roka@dataquest.sk>
Co-authored-by: MatusBeke <matus.beke7@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: 4 people

dspace-api/src/main/java/org/dspace/app/mediafilter/PDFBoxThumbnail.java

@@ -8,6 +8,7 @@
 package org.dspace.app.mediafilter;
 
 import java.awt.image.BufferedImage;
+import java.io.IOException;
 import java.io.InputStream;
 
 import org.apache.logging.log4j.Logger;
@@ -79,6 +80,13 @@ public InputStream getDestinationStream(Item currentItem, InputStream source, bo
         } catch (InvalidPasswordException ex) {
             log.error("PDF is encrypted. Cannot create thumbnail (item: {})", currentItem::getHandle);
             return null;
+        } catch (IOException ex) {
+            // A malformed/non-standard PDF (bad %PDF- header, missing xref, truncated file, etc.)
+            // is a data-quality issue in the source bitstream, not a DSpace fault. Skip the
+            // thumbnail instead of failing the whole filter-media run.
+            log.warn("PDF could not be parsed by PDFBox. Cannot create thumbnail (item: {}): {}",
+                    currentItem::getHandle, ex::getMessage);
+            return null;
         }
 
         // Generate thumbnail derivative and return as IO stream.

dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java

@@ -85,10 +85,13 @@ public InputStream getDestinationStream(Item currentItem, InputStream source, bo
             tika.setMaxStringLength(maxChars); // Tell Tika the maximum number of characters to extract
             extractedText = tika.parseToString(source);
         } catch (IOException e) {
-            System.err.format("Unable to extract text from bitstream in Item %s%n", currentItem.getID().toString());
-            e.printStackTrace(System.err);
-            log.error("Unable to extract text from bitstream in Item {}", currentItem.getID().toString(), e);
-            throw e;
+            // A malformed/non-standard source file (e.g. a PDF with a corrupt header, missing
+            // xref, or truncated content) is a data-quality issue in the bitstream, not a
+            // DSpace fault. Skip text extraction for it instead of failing the whole
+            // filter-media run.
+            log.warn("Unable to extract text from bitstream in Item {}: {}",
+                    currentItem.getHandle(), e.getMessage());
+            return null;
         } catch (OutOfMemoryError oe) {
             System.err.format("OutOfMemoryError occurred when extracting text from bitstream in Item %s. " +
                 "You may wish to enable 'textextractor.use-temp-file'.%n", currentItem.getID().toString());

dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java

@@ -610,7 +610,7 @@ public String getName() {
      */
     private void assignGroups(String dn, ArrayList<String> group, Context context) {
         if (StringUtils.isNotBlank(dn)) {
-            System.out.println("dn:" + dn);
+            log.debug(LogHelper.getHeader(context, "assignGroups", "dn=" + dn));
             int groupmapIndex = 1;
             String groupMap = configurationService.getProperty("authentication-ldap.login.groupmap." + groupmapIndex);
             boolean cmp;
@@ -619,7 +619,16 @@ private void assignGroups(String dn, ArrayList<String> group, Context context) {
             // groupmap contains the mapping of LDAP groups to DSpace groups
             // outer loop with the DSpace groups
             while (groupMap != null) {
-                String t[] = groupMap.split(":");
+                String t[] = groupMap.split(":", 2);
+                if (t.length < 2 || StringUtils.isBlank(t[0]) || StringUtils.isBlank(t[1])) {
+                    log.error(LogHelper.getHeader(context, "assignGroups",
+                        "malformed groupmap entry at index " + groupmapIndex + ": " + groupMap +
+                        " - expected '<ldapSearchFragment>:<dspaceGroupName>' with both parts non-empty"));
+                    groupMap = configurationService.getProperty(
+                            "authentication-ldap.login.groupmap." + ++groupmapIndex);
+                    continue;
+                }
+
                 String ldapSearchString = t[0];
                 String dspaceGroupName = t[1];
 

dspace-api/src/test/java/org/dspace/authenticate/LDAPAuthenticationTest.java

@@ -0,0 +1,108 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.authenticate;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+
+import org.dspace.AbstractUnitTest;
+import org.dspace.core.Context;
+import org.dspace.eperson.Group;
+import org.dspace.eperson.service.GroupService;
+import org.dspace.services.ConfigurationService;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Unit tests for the {@code authentication-ldap.login.groupmap.N} parsing in
+ * {@link LDAPAuthentication#assignGroups(String, ArrayList, Context)}. No LDAP server is
+ * involved: the configuration and the group service are mocked and only the mapping logic runs.
+ */
+public class LDAPAuthenticationTest extends AbstractUnitTest {
+
+    private static final String GROUPMAP = "authentication-ldap.login.groupmap.";
+    private static final String DN = "uid=jdoe,ou=staff,dc=example,dc=org";
+
+    private LDAPAuthentication ldapAuthentication;
+    private ConfigurationService configurationService;
+    private GroupService groupService;
+
+    @Before
+    public void setUp() {
+        ldapAuthentication = new LDAPAuthentication();
+        configurationService = mock(ConfigurationService.class);
+        groupService = mock(GroupService.class);
+        ldapAuthentication.configurationService = configurationService;
+        ldapAuthentication.groupService = groupService;
+    }
+
+    /**
+     * assignGroups is private, and the branch under test is the one taken when the caller has no
+     * list of LDAP groups, so the second argument has to be a real null.
+     */
+    private void assignGroups() throws Exception {
+        Method method = LDAPAuthentication.class
+                .getDeclaredMethod("assignGroups", String.class, ArrayList.class, Context.class);
+        method.setAccessible(true);
+        method.invoke(ldapAuthentication, DN, null, context);
+    }
+
+    @Test
+    public void assignGroupsSkipsEntryWithoutSeparator() throws Exception {
+        when(configurationService.getProperty(GROUPMAP + 1)).thenReturn("ou=staff");
+
+        assignGroups();
+
+        verify(groupService, never()).findByName(any(), any());
+    }
+
+    @Test
+    public void assignGroupsSkipsEntryWithBlankSearchPart() throws Exception {
+        when(configurationService.getProperty(GROUPMAP + 1)).thenReturn(":Admins");
+
+        assignGroups();
+
+        verify(groupService, never()).findByName(any(), any());
+    }
+
+    @Test
+    public void assignGroupsSkipsEntryWithBlankGroupPart() throws Exception {
+        when(configurationService.getProperty(GROUPMAP + 1)).thenReturn("ou=staff:");
+
+        assignGroups();
+
+        verify(groupService, never()).findByName(any(), any());
+    }
+
+    @Test
+    public void assignGroupsKeepsScanningAfterAMalformedEntry() throws Exception {
+        when(configurationService.getProperty(GROUPMAP + 1)).thenReturn(":Admins");
+        when(configurationService.getProperty(GROUPMAP + 2)).thenReturn("ou=staff:Staff");
+        when(groupService.findByName(context, "Staff")).thenReturn(mock(Group.class));
+
+        assignGroups();
+
+        verify(groupService).findByName(context, "Staff");
+    }
+
+    @Test
+    public void assignGroupsKeepsAColonInsideTheDSpaceGroupName() throws Exception {
+        when(configurationService.getProperty(GROUPMAP + 1)).thenReturn("ou=staff:Staff:Local");
+        when(groupService.findByName(context, "Staff:Local")).thenReturn(mock(Group.class));
+
+        assignGroups();
+
+        verify(groupService).findByName(context, "Staff:Local");
+    }
+}

dspace-swordv2/src/main/java/org/dspace/sword2/ContainerManagerDSpace.java

@@ -13,6 +13,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
+import java.util.UUID;
 
 import org.apache.logging.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
@@ -25,6 +26,7 @@
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.core.LogHelper;
+import org.dspace.event.Event;
 import org.dspace.workflow.WorkflowItem;
 import org.dspace.workflow.WorkflowItemService;
 import org.dspace.workflow.factory.WorkflowServiceFactory;
@@ -755,14 +757,18 @@ protected void doContainerDelete(SwordContext swordContext, Item item,
             WorkflowTools wft = new WorkflowTools();
             if (wft.isItemInWorkspace(swordContext.getContext(), item)) {
                 WorkspaceItem wsi = wft.getWorkspaceItem(context, item);
+                // remove only the workspace wrapper row; the item itself is deleted below.
                 workspaceItemService.deleteWrapper(context, wsi);
             } else if (wft.isItemInWorkflow(context, item)) {
                 WorkflowItem wfi = wft.getWorkflowItem(context, item);
                 workflowItemService.deleteWrapper(context, wfi);
             }
 
-            // then delete the item
-            itemService.delete(context, item);
+            // then delete the item, unless an upstream method already queued its deletion
+            // in this transaction (safety net against a double itemService.delete()).
+            if (!isItemAlreadyDeleted(context, item.getID())) {
+                itemService.delete(context, item);
+            }
         } catch (SQLException | IOException e) {
             throw new DSpaceSwordException(e);
         } catch (AuthorizeException e) {
@@ -788,4 +794,24 @@ private Item getDSpaceTarget(Context context, String editUrl,
 
         return item;
     }
+
+    /**
+     * Returns true if a DELETE event for this item is already queued on the context
+     * (i.e. the item was deleted earlier in this transaction), so the caller can skip
+     * a second {@code itemService.delete()} that would otherwise fail.
+     */
+    private boolean isItemAlreadyDeleted(Context context, UUID itemUUID) {
+        if (context.getEvents() == null) {
+            return false;
+        }
+
+        for (Event event : context.getEvents()) {
+            if (event.getEventType() == Event.DELETE
+                    && event.getSubjectType() == Constants.ITEM
+                    && itemUUID.equals(event.getSubjectID())) {
+                return true;
+            }
+        }
+        return false;
+    }
 }