DS-3700: MediaFilterServiceImpl forgot to close an input stream.

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

@@ -21,6 +21,7 @@ import org.dspace.services.ConfigurationService;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.*;
 
@@ -314,12 +315,10 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
         // get bitstream filename, calculate destination filename
         String newName = formatFilter.getFilteredName(source.getName());
 
-        Bitstream existingBitstream = null; // is there an existing rendition?
-        Bundle targetBundle = null; // bundle we're modifying
-
-        List<Bundle> bundles = itemService.getBundles(item, formatFilter.getBundleName());
-
         // check if destination bitstream exists
+        Bundle existingBundle = null;
+        Bitstream existingBitstream = null;
+        List<Bundle> bundles = itemService.getBundles(item, formatFilter.getBundleName());
         if (bundles.size() > 0)
         {
             // only finds the last match (FIXME?)
@@ -328,13 +327,12 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
 
                 for (Bitstream bitstream : bitstreams) {
                     if (bitstream.getName().equals(newName)) {
-                        targetBundle = bundle;
+                        existingBundle = bundle;
                         existingBitstream = bitstream;
                     }
                 }
             }
         }
-
         // if exists and overwrite = false, exit
         if (!overWrite && (existingBitstream != null))
         {
@@ -352,67 +350,76 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
                 + " (item: " + item.getHandle() + ")");
         }
 
-        InputStream destStream;
+        System.out.println("File: " + newName);
-        try {
+
-            System.out.println("File: " + newName);
+        // start filtering of the bitstream, using try with resource to close all InputStreams properly
-            destStream = formatFilter.getDestinationStream(item, bitstreamService.retrieve(context, source), isVerbose);
+        try (
+                // get the source stream
+                InputStream srcStream = bitstreamService.retrieve(context, source);
+                // filter the source stream to produce the destination stream
+                // this is the hard work, check for OutOfMemoryErrors at the end of the try clause.
+                InputStream destStream = formatFilter.getDestinationStream(item, srcStream, isVerbose);
+        ) {
             if (destStream == null) {
                 if (!isQuiet) {
                     System.out.println("SKIPPED: bitstream " + source.getID()
                             + " (item: " + item.getHandle() + ") because filtering was unsuccessful");
                 }
-
                 return false;
             }
+
+            Bundle targetBundle; // bundle we're modifying
+            if (bundles.size() < 1)
+            {
+                // create new bundle if needed
+                targetBundle = bundleService.create(context, item, formatFilter.getBundleName());
+            }
+            else
+            {
+                // take the first match as we already looked out for the correct bundle name
+                targetBundle = bundles.get(0);
+            }
+
+            // create bitstream to store the filter result
+            Bitstream b = bitstreamService.create(context, targetBundle, destStream);
+            // set the name, source and description of the bitstream
+            b.setName(context, newName);
+            b.setSource(context, "Written by FormatFilter " + formatFilter.getClass().getName() +
+                    " on " + DCDate.getCurrent() + " (GMT).");
+            b.setDescription(context, formatFilter.getDescription());
+            // Set the format of the bitstream
+            BitstreamFormat bf = bitstreamFormatService.findByShortDescription(context,
+                    formatFilter.getFormatString());
+            bitstreamService.setFormat(context, b, bf);
+            bitstreamService.update(context, b);
+
+            //Set permissions on the derivative bitstream
+            //- First remove any existing policies
+            authorizeService.removeAllPolicies(context, b);
+
+            //- Determine if this is a public-derivative format
+            if(publicFiltersClasses.contains(formatFilter.getClass().getSimpleName())) {
+                //- Set derivative bitstream to be publicly accessible
+                Group anonymous = groupService.findByName(context, Group.ANONYMOUS);
+                authorizeService.addPolicy(context, b, Constants.READ, anonymous);
+            } else {
+                //- Inherit policies from the source bitstream
+                authorizeService.inheritPolicies(context, source, b);
+            }
+
+            //do post-processing of the generated bitstream
+            formatFilter.postProcessBitstream(context, item, b);
+
+
         } catch (OutOfMemoryError oome) {
             System.out.println("!!! OutOfMemoryError !!!");
-            return false;
-        }
-
-        // create new bundle if needed
-        if (bundles.size() < 1)
-        {
-            targetBundle = bundleService.create(context, item, formatFilter.getBundleName());
-        }
-        else
-        {
-            // take the first match
-            targetBundle = bundles.get(0);
-        }
-
-        Bitstream b = bitstreamService.create(context, targetBundle, destStream);
-
-        // Now set the format and name of the bitstream
-        b.setName(context, newName);
-        b.setSource(context, "Written by FormatFilter " + formatFilter.getClass().getName() +
-        			" on " + DCDate.getCurrent() + " (GMT)."); 
-        b.setDescription(context, formatFilter.getDescription());
-
-        // Find the proper format
-        BitstreamFormat bf = bitstreamFormatService.findByShortDescription(context,
-                formatFilter.getFormatString());
-        bitstreamService.setFormat(context, b, bf);
-        bitstreamService.update(context, b);
-        
-        //Set permissions on the derivative bitstream
-        //- First remove any existing policies
-        authorizeService.removeAllPolicies(context, b);
-
-        //- Determine if this is a public-derivative format
-        if(publicFiltersClasses.contains(formatFilter.getClass().getSimpleName())) {
-            //- Set derivative bitstream to be publicly accessible
-            Group anonymous = groupService.findByName(context, Group.ANONYMOUS);
-            authorizeService.addPolicy(context, b, Constants.READ, anonymous);
-        } else {
-            //- Inherit policies from the source bitstream
-            authorizeService.inheritPolicies(context, source, b);
         }
 
         // fixme - set date?
         // we are overwriting, so remove old bitstream
         if (existingBitstream != null)
         {
-            bundleService.removeBitstream(context, targetBundle, existingBitstream);
+            bundleService.removeBitstream(context, existingBundle, existingBitstream);
         }
 
         if (!isQuiet)
@@ -421,9 +428,6 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
                     + " (item: " + item.getHandle() + ") and created '" + newName + "'");
         }
 
-        //do post-processing of the generated bitstream
-        formatFilter.postProcessBitstream(context, item, b);
-        
         return true;
     }
     

dspace-api/src/main/java/org/dspace/storage/bitstore/DSBitStoreService.java

@@ -111,37 +111,37 @@ public class DSBitStoreService implements BitStoreService
      */
 	public void put(Bitstream bitstream, InputStream in) throws IOException
 	{
-        try {
+        try
+        {
             File file = getFile(bitstream);
 
             // Make the parent dirs if necessary
             File parent = file.getParentFile();
-            if (!parent.exists()) {
+            if (!parent.exists())
+            {
                 parent.mkdirs();
             }
             //Create the corresponding file and open it
             file.createNewFile();
 
-            FileOutputStream fos = new FileOutputStream(file);
+            try (
+                    FileOutputStream fos = new FileOutputStream(file);
+                    // Read through a digest input stream that will work out the MD5
+                    DigestInputStream dis = new DigestInputStream(in, MessageDigest.getInstance(CSA));
+            )
+            {
+                Utils.bufferedCopy(dis, fos);
+                in.close();
 
-            // Read through a digest input stream that will work out the MD5
+                bitstream.setSizeBytes(file.length());
-            DigestInputStream dis = null;
+                bitstream.setChecksum(Utils.toHex(dis.getMessageDigest().digest()));
-
+                bitstream.setChecksumAlgorithm(CSA);
-            try {
-                dis = new DigestInputStream(in, MessageDigest.getInstance(CSA));
             }
-            // Should never happen
+            catch (NoSuchAlgorithmException nsae)
-            catch (NoSuchAlgorithmException nsae) {
+            {
+                // Should never happen
                 log.warn("Caught NoSuchAlgorithmException", nsae);
             }
-
-            Utils.bufferedCopy(dis, fos);
-            fos.close();
-            in.close();
-
-            bitstream.setSizeBytes(file.length());
-            bitstream.setChecksum(Utils.toHex(dis.getMessageDigest().digest()));
-            bitstream.setChecksumAlgorithm(CSA);
         } catch (Exception e) {
             log.error("put(" + bitstream.getInternalId() + ", inputstream)", e);
             throw new IOException(e);