You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tika.apache.org by GitBox <gi...@apache.org> on 2020/11/12 01:42:15 UTC
[GitHub] [tika] PeterAlfredLee opened a new pull request #382: Simplify some code in OPCPackageDetector#detect
PeterAlfredLee opened a new pull request #382:
URL: https://github.com/apache/tika/pull/382
1. Simpify catch statement in method `detect`.
2. Adjust some comment in method `detect`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tika] tballison commented on a change in pull request #382: Simplify some code in OPCPackageDetector#detect
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [tika] PeterAlfredLee commented on a change in pull request #382: Simplify some code in OPCPackageDetector#detect
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on a change in pull request #382:
URL: https://github.com/apache/tika/pull/382#discussion_r527336143
##########
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:
Ah, I was just thinking to simplify the two catch clause as they look similar. Haven't think about the performance here. :)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tika] PeterAlfredLee closed pull request #382: Simplify some code in OPCPackageDetector#detect
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee closed pull request #382:
URL: https://github.com/apache/tika/pull/382
----------------------------------------------------------------
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:
users@infra.apache.org