[DS-3469] virus scan during submission attempts to read uploaded bitstream as anonymous user, which fails (#1632)

* [DS-3469] Add the current session context to the curation task run.

* [DS-3469] Log how I/O failed, not just that it did.

* [DS-3469] Keep reference to Bundle from which we just removed the Bitstream instead of expecting the List of Bundle to be unaltered.

* [DS-3469] Finish switching from e.getMessage() to e

* [DS-3469] Note the side effect of calling curate() with a Context.
This commit is contained in:
Mark H. Wood
2017-02-08 11:00:59 -05:00
committed by Tim Donohue
parent 90cb82922f
commit dd4ca00071
3 changed files with 65 additions and 37 deletions

View File

@@ -19,8 +19,6 @@ import java.sql.SQLException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import org.apache.log4j.Logger;
import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.AuthorizeException;
import org.dspace.content.*; import org.dspace.content.*;
import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.factory.ContentServiceFactory;
@@ -28,6 +26,8 @@ import org.dspace.content.service.BitstreamService;
import org.dspace.curate.AbstractCurationTask; import org.dspace.curate.AbstractCurationTask;
import org.dspace.curate.Curator; import org.dspace.curate.Curator;
import org.dspace.curate.Suspendable; import org.dspace.curate.Suspendable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** ClamScan.java /** ClamScan.java
* *
@@ -55,7 +55,7 @@ public class ClamScan extends AbstractCurationTask
protected final String SCAN_FAIL_MESSAGE = "Error encountered using virus service - check setup"; protected final String SCAN_FAIL_MESSAGE = "Error encountered using virus service - check setup";
protected final String NEW_ITEM_HANDLE = "in workflow"; protected final String NEW_ITEM_HANDLE = "in workflow";
private static Logger log = Logger.getLogger(ClamScan.class); private static final Logger log = LoggerFactory.getLogger(ClamScan.class);
protected String host = null; protected String host = null;
protected int port = 0; protected int port = 0;
@@ -234,18 +234,18 @@ public class ClamScan extends AbstractCurationTask
} }
} }
/** scan /** A buffer to hold chunks of an input stream to be scanned for viruses. */
* final byte[] buffer = new byte[DEFAULT_CHUNK_SIZE];
/**
* Issue the INSTREAM command and return the response to * Issue the INSTREAM command and return the response to
* and from the clamav daemon * and from the clamav daemon.
* *
* @param the bitstream for reporting results * @param bitstream the bitstream for reporting results
* @param the InputStream to read * @param inputstream the InputStream to read
* @param the item handle for reporting results * @param itemHandle the item handle for reporting results
* @return a ScanResult representing the server response * @return a ScanResult representing the server response
* @throws IOException if IO error
*/ */
final byte[] buffer = new byte[DEFAULT_CHUNK_SIZE];;
protected int scan(Bitstream bitstream, InputStream inputstream, String itemHandle) protected int scan(Bitstream bitstream, InputStream inputstream, String itemHandle)
{ {
try try
@@ -254,7 +254,7 @@ public class ClamScan extends AbstractCurationTask
} }
catch (IOException e) catch (IOException e)
{ {
log.error("Error writing INSTREAM command . . ."); log.error("Error writing INSTREAM command", e);
return Curator.CURATE_ERROR; return Curator.CURATE_ERROR;
} }
int read = DEFAULT_CHUNK_SIZE; int read = DEFAULT_CHUNK_SIZE;
@@ -266,7 +266,7 @@ public class ClamScan extends AbstractCurationTask
} }
catch (IOException e) catch (IOException e)
{ {
log.error("Failed attempting to read the InputStream . . . "); log.error("Failed attempting to read the InputStream", e);
return Curator.CURATE_ERROR; return Curator.CURATE_ERROR;
} }
if (read == -1) if (read == -1)
@@ -280,7 +280,7 @@ public class ClamScan extends AbstractCurationTask
} }
catch (IOException e) catch (IOException e)
{ {
log.error("Could not write to the socket . . . "); log.error("Could not write to the socket", e);
return Curator.CURATE_ERROR; return Curator.CURATE_ERROR;
} }
} }
@@ -291,7 +291,7 @@ public class ClamScan extends AbstractCurationTask
} }
catch (IOException e) catch (IOException e)
{ {
log.error("Error writing zero-length chunk to socket") ; log.error("Error writing zero-length chunk to socket", e) ;
return Curator.CURATE_ERROR; return Curator.CURATE_ERROR;
} }
try try
@@ -301,7 +301,7 @@ public class ClamScan extends AbstractCurationTask
} }
catch (IOException e) catch (IOException e)
{ {
log.error( "Error reading result from socket"); log.error( "Error reading result from socket", e);
return Curator.CURATE_ERROR; return Curator.CURATE_ERROR;
} }
@@ -309,7 +309,7 @@ public class ClamScan extends AbstractCurationTask
{ {
String response = new String(buffer, 0, read); String response = new String(buffer, 0, read);
logDebugMessage("Response: " + response); logDebugMessage("Response: " + response);
if (response.indexOf("FOUND") != -1) if (response.contains("FOUND"))
{ {
String itemMsg = "item - " + itemHandle + ": "; String itemMsg = "item - " + itemHandle + ": ";
String bsMsg = "bitstream - " + bitstream.getName() + String bsMsg = "bitstream - " + bitstream.getName() +

View File

@@ -52,12 +52,12 @@ public class Curator
// transaction scopes // transaction scopes
public static enum TxScope { OBJECT, CURATION, OPEN }; public static enum TxScope { OBJECT, CURATION, OPEN };
private static Logger log = Logger.getLogger(Curator.class); private static final Logger log = Logger.getLogger(Curator.class);
protected static final ThreadLocal<Context> curationCtx = new ThreadLocal<Context>(); protected static final ThreadLocal<Context> curationCtx = new ThreadLocal<>();
protected Map<String, TaskRunner> trMap = new HashMap<String, TaskRunner>(); protected Map<String, TaskRunner> trMap = new HashMap<>();
protected List<String> perfList = new ArrayList<String>(); protected List<String> perfList = new ArrayList<>();
protected TaskQueue taskQ = null; protected TaskQueue taskQ = null;
protected String reporter = null; protected String reporter = null;
protected Invoked iMode = null; protected Invoked iMode = null;
@@ -180,8 +180,12 @@ public class Curator
* Performs all configured tasks upon object identified by id. If * Performs all configured tasks upon object identified by id. If
* the object can be resolved as a handle, the DSO will be the * the object can be resolved as a handle, the DSO will be the
* target object. * target object.
* *
* @param c a Dpace context * <p>
* Note: this method has the side-effect of setting this instance's Context
* reference. The setting is retained on return.
*
* @param c a DSpace context
* @param id an object identifier * @param id an object identifier
* @throws IOException if IO error * @throws IOException if IO error
*/ */
@@ -233,9 +237,10 @@ public class Curator
* <P> * <P>
* Note: Site-wide tasks will default to running as * Note: Site-wide tasks will default to running as
* an Anonymous User unless you call the Site-wide task * an Anonymous User unless you call the Site-wide task
* via the 'curate(Context,String)' method with an * via the {@link curate(Context,String)} or
* {@link #curate(Context, DSpaceObject)} method with an
* authenticated Context object. * authenticated Context object.
* *
* @param dso the DSpace object * @param dso the DSpace object
* @throws IOException if IO error * @throws IOException if IO error
*/ */
@@ -268,7 +273,26 @@ public class Curator
} }
} }
} }
/**
* Performs all configured tasks upon DSpace object
* (Community, Collection or Item).
*
* <p>
* Note: this method has the side-effect of setting this instance's Context
* reference. The setting is retained on return.
*
* @param c session context in which curation takes place.
* @param dso the single object to be curated.
* @throws java.io.IOException passed through.
*/
public void curate(Context c, DSpaceObject dso)
throws IOException
{
curationCtx.set(c);
curate(dso);
}
/** /**
* Places a curation request for the object identified by id on a * Places a curation request for the object identified by id on a
* managed queue named by the queueId. * managed queue named by the queueId.

View File

@@ -29,7 +29,6 @@ import org.dspace.authorize.AuthorizeException;
import org.dspace.content.*; import org.dspace.content.*;
import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.factory.ContentServiceFactory;
import org.dspace.content.service.BitstreamFormatService; import org.dspace.content.service.BitstreamFormatService;
import org.dspace.content.service.BitstreamService;
import org.dspace.core.Context; import org.dspace.core.Context;
import org.dspace.curate.Curator; import org.dspace.curate.Curator;
import org.dspace.submit.AbstractProcessingStep; import org.dspace.submit.AbstractProcessingStep;
@@ -47,7 +46,6 @@ import org.dspace.submit.AbstractProcessingStep;
* @see org.dspace.submit.AbstractProcessingStep * @see org.dspace.submit.AbstractProcessingStep
* *
* @author Tim Donohue * @author Tim Donohue
* @version $Revision$
*/ */
public class UploadStep extends AbstractProcessingStep public class UploadStep extends AbstractProcessingStep
{ {
@@ -95,7 +93,7 @@ public class UploadStep extends AbstractProcessingStep
public static final int STATUS_EDIT_COMPLETE = 25; public static final int STATUS_EDIT_COMPLETE = 25;
/** log4j logger */ /** log4j logger */
private static Logger log = Logger.getLogger(UploadStep.class); private static final Logger log = Logger.getLogger(UploadStep.class);
/** is the upload required? */ /** is the upload required? */
protected boolean fileRequired = configurationService.getBooleanProperty("webui.submit.upload.required", true); protected boolean fileRequired = configurationService.getBooleanProperty("webui.submit.upload.required", true);
@@ -614,7 +612,7 @@ public class UploadStep extends AbstractProcessingStep
if (configurationService.getBooleanProperty("submission-curation.virus-scan")) if (configurationService.getBooleanProperty("submission-curation.virus-scan"))
{ {
Curator curator = new Curator(); Curator curator = new Curator();
curator.addTask("vscan").curate(item); curator.addTask("vscan").curate(context, item);
int status = curator.getStatus("vscan"); int status = curator.getStatus("vscan");
if (status == Curator.CURATE_ERROR) if (status == Curator.CURATE_ERROR)
{ {
@@ -652,24 +650,30 @@ public class UploadStep extends AbstractProcessingStep
} }
/* /*
If we created a new Bitstream but now realised there is a problem then remove it. * If we created a new Bitstream but now realise there is a problem then remove it.
*/ */
protected void backoutBitstream(Context context, SubmissionInfo subInfo, Bitstream b, Item item) throws SQLException, AuthorizeException, IOException protected void backoutBitstream(Context context, SubmissionInfo subInfo, Bitstream b, Item item)
throws SQLException, AuthorizeException, IOException
{ {
// remove bitstream from bundle.. // remove bitstream from bundle..
// delete bundle if it's now empty
List<Bundle> bundles = b.getBundles(); List<Bundle> bundles = b.getBundles();
if (bundles.isEmpty())
throw new SQLException("Bitstream is not in any Bundles.");
bundleService.removeBitstream(context, bundles.get(0), b); Bundle firstBundle = bundles.get(0);
List<Bitstream> bitstreams = bundles.get(0).getBitstreams(); bundleService.removeBitstream(context, firstBundle, b);
List<Bitstream> bitstreams = firstBundle.getBitstreams();
// remove bundle if it's now empty // remove bundle if it's now empty
if (bitstreams.size() < 1) if (bitstreams.isEmpty())
{ {
itemService.removeBundle(context, item, bundles.get(0)); itemService.removeBundle(context, item, firstBundle);
itemService.update(context, item); itemService.update(context, item);
} }
else
bundleService.update(context, firstBundle);
subInfo.setBitstream(null); subInfo.setBitstream(null);
} }