tballison commented on a change in pull request #382:
URL: https://github.com/apache/tika/pull/382#discussion_r526860010



##########
File path: 
tika-parser-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/detect/microsoft/ooxml/OPCPackageDetector.java
##########
@@ -159,32 +159,32 @@
 
     @Override
     public MediaType detect(ZipFile zipFile, TikaInputStream stream) throws 
IOException {
-        //as of 4.x, POI throws an exception for non-POI OPC file types
-        //unless we change POI, we can't rely on POI for non-POI files
+        // as of 4.x, POI throws an exception for non-POI OPC file types
+        // unless we change POI, we can't rely on POI for non-POI files
         ZipEntrySource zipEntrySource = new ZipFileZipEntrySource(zipFile);
 
         // Use POI to open and investigate it for us
-        //Unfortunately, POI can throw a RuntimeException...so we
-        //have to catch that.
-        OPCPackage pkg = null;
-        MediaType type = null;
+        // Unfortunately, POI can throw a RuntimeException...so we have to 
catch that.
         try {
-            pkg = OPCPackage.open(zipEntrySource);
-            type = detectOfficeOpenXML(pkg);
+            OPCPackage pkg = OPCPackage.open(zipEntrySource);
+            MediaType type = detectOfficeOpenXML(pkg);
+
+            // only set the open container if we made it here
+            stream.setOpenContainer(pkg);
+            return type;
 
-        } catch (SecurityException e) {
-            closeQuietly(zipEntrySource);
-            closeQuietly(zipFile);
-            //TIKA-2571
-            throw e;
         } catch (InvalidFormatException | RuntimeException e) {
             closeQuietly(zipEntrySource);
             closeQuietly(zipFile);
-            return null;
+
+            if (e instanceof SecurityException) {

Review comment:
       I have a personal preference for catching/handling specific exceptions 
according to behavior of the response.  Is there a performance reason to check 
`instanceof` SecurityException?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to