Improved error handling.

Cancel piped operation if intermediate operations yield errors. Better handle invalid extensions.
dscalzi · May 8, 2019 · 9025186 · 9025186
1 parent 5c6987d
commit 9025186
Show file tree

Hide file tree

Showing 10 changed files with 125 additions and 31 deletions.
diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/ZCompressor.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/ZCompressor.java
@@ -24,6 +24,7 @@
 import java.util.ArrayList;
 import java.util.Deque;
 import java.util.List;
+import java.util.function.BooleanSupplier;
 
 import com.dscalzi.zipextractor.core.managers.MessageManager;
 import com.dscalzi.zipextractor.core.provider.TypeProvider;
@@ -72,8 +73,7 @@ public static void asyncCompress(ICommandSender sender, File src, File dest, boo
             String pth = destNorm.toString();
 
             for(int i=destExts.length-1; i>=0; i--) {
-                if(supportedExtensions().contains(destExts[i].toLowerCase()) && 
-                        (srcExts.length > 0 ? !destExts[i].equalsIgnoreCase(srcExts[srcExts.length-1]) : true)) {
+                if(srcExts.length > 0 ? !destExts[i].equalsIgnoreCase(srcExts[srcExts.length-1]) : true) {
                     pth = pth.substring(0, pth.length()-destExts[i].length()-1);
                     sTemp = new File(pth);
 
@@ -99,7 +99,7 @@ public static void asyncCompress(ICommandSender sender, File src, File dest, boo
         Runnable task = null;
         int c = 0;
         boolean piped = false;
-        final Runnable[] pipes = new Runnable[pDeque.size()];
+        final BooleanSupplier[] pipes = new BooleanSupplier[pDeque.size()];
         for (final OpTuple e : pDeque) {
             final boolean interOp = c != pDeque.size()-1;
 
@@ -111,21 +111,24 @@ public static void asyncCompress(ICommandSender sender, File src, File dest, boo
 
             if(piped) {
                 pipes[c] = () -> {
-                    e.getProvider().compress(sender, e.getSrc(), e.getDest(), log, interOp);
+                    boolean res = e.getProvider().compress(sender, e.getSrc(), e.getDest(), log, interOp);
                     e.getSrc().delete();
+                    return res;
                 };
             } else {
                 pipes[c] = () -> {
-                    e.getProvider().compress(sender, e.getSrc(), e.getDest(), log, interOp);
+                    return e.getProvider().compress(sender, e.getSrc(), e.getDest(), log, interOp);
                 };
             }
             piped = true;
             c++;
         }
 
         task = () -> {
-            for(Runnable r : pipes) {
-                r.run();
+            for(BooleanSupplier r : pipes) {
+                if(!r.getAsBoolean()) {
+                    break;
+                }
             }
         };
 

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/ZExtractor.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/ZExtractor.java
@@ -89,13 +89,21 @@ public static void asyncExtract(ICommandSender sender, File src, File dest, bool
                 String queue = srcNorm.toString();
 
                 for(int i=srcExts.length-1; i>=0; i--) {
-                    if((until != null && srcExts[i].equalsIgnoreCase(until)) || !supportedExtensions().contains(srcExts[i].toLowerCase())) {
+                    if((until != null && srcExts[i].equalsIgnoreCase(until))) {
+                        if(i == srcExts.length-1) {
+                            mm.nothingToDo(sender);
+                            return;
+                        }
                         break;
                     }
                     TypeProvider p = getApplicableProvider(tSrc);
                     if(p == null) {
-                        mm.invalidExtractionExtension(sender);
-                        return;
+                        if(i == srcExts.length-1) {
+                            mm.invalidExtractionExtension(sender);
+                            return;
+                        } else {
+                            break;
+                        }
                     }
                     pDeque.add(new OpTuple(tSrc, dest, p));
                     queue = queue.substring(0, queue.lastIndexOf('.'));
@@ -163,9 +171,9 @@ public static void asyncExtract(ICommandSender sender, File src, File dest, bool
                         atRisk = op.getProvider().scanForExtractionConflicts(sender, op.getSrc(), op.getDest(), false);
                     }
                     if (atRisk.size() == 0 || override) {
-                        op.getProvider().extract(sender, op.getSrc(), op.getDest(), log, interOp);
+                        boolean res = op.getProvider().extract(sender, op.getSrc(), op.getDest(), log, interOp);
                         op.getSrc().delete();
-                        return true;
+                        return res;
                     } else {
                         WARNED.put(sender.getName(), new WarnData(op.getSrc(), op.getDest(), new PageList<String>(4, atRisk)));
                         mm.warnOfConflicts(sender, atRisk.size());
@@ -179,8 +187,7 @@ public static void asyncExtract(ICommandSender sender, File src, File dest, bool
                         atRisk = op.getProvider().scanForExtractionConflicts(sender, op.getSrc(), op.getDest(), false);
                     }
                     if (atRisk.size() == 0 || override) {
-                        op.getProvider().extract(sender, op.getSrc(), op.getDest(), log, interOp);
-                        return true;
+                        return op.getProvider().extract(sender, op.getSrc(), op.getDest(), log, interOp);
                     } else {
                         WARNED.put(sender.getName(), new WarnData(op.getSrc(), op.getDest(), new PageList<String>(4, atRisk)));
                         mm.warnOfConflicts(sender, atRisk.size());
@@ -196,7 +203,7 @@ public static void asyncExtract(ICommandSender sender, File src, File dest, bool
         task = () -> {
             for(BooleanSupplier r : pipes) {
                 if(!r.getAsBoolean()) {
-                    // Conflicts
+                    // Conflicts or errors
                     break;
                 }
             }

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/managers/MessageManager.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/managers/MessageManager.java
@@ -294,6 +294,10 @@ public void taskInterruption(ICommandSender sender, ZTask task) {
         plugin.warn("Channel closed during " + task.getProcessName()
                 + ", unable to continue. This is most likely due to a forced termination of the execution servicer.");
     }
+
+    public void nothingToDo(ICommandSender sender) {
+        sendError(sender, "No operation can be performed for your request.");
+    }
 
     public void startingProcess(ICommandSender sender, ZTask task, String fileName) {
         if (!sender.isConsole()) {
@@ -322,7 +326,17 @@ public void compressionComplete(ICommandSender sender, File dest) {
         plugin.info("The folder's contents have been compressed to\n" + dest.toPath().toAbsolutePath().normalize().toString());
         plugin.info("---------------------------------------------------");
     }
+
+    public void genericOperationError(ICommandSender sender, File src, ZTask task) {
+        sendError(sender, "Error during " + task.getProcessName() + " of " + src.toPath().toAbsolutePath().normalize().toString());
+        sendError(sender, "Is the file corrupt?");
+    }
 
+    public void extractionFormatError(ICommandSender sender, File src, String format) {
+        sendError(sender, "Error while extracting, source file must be in " + format + " format.");
+        sendError(sender, src.toPath().toAbsolutePath().normalize().toString());
+    }
+
     public void denyCommandBlock(ICommandSender sender) {
         sendError(sender, "Command blocks are blocked from accessing this command for security purposes.");
     }

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/GZProvider.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/GZProvider.java
@@ -28,6 +28,7 @@
 import java.util.regex.Pattern;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
+import java.util.zip.ZipException;
 
 import com.dscalzi.zipextractor.core.TaskInterruptedException;
 import com.dscalzi.zipextractor.core.ZTask;
@@ -60,7 +61,7 @@ public boolean canDetectPipedConflicts() {
     }
 
     @Override
-    public void extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         final MessageManager mm = MessageManager.inst();
         mm.startingProcess(sender, ZTask.EXTRACT, src.getName());
         File realDest = new File(dest.getAbsolutePath(), PATH_END.matcher(src.getName()).replaceAll(""));
@@ -77,15 +78,22 @@ public void extract(ICommandSender sender, File src, File dest, boolean log, boo
             }
             if(!pipe)
                 mm.extractionComplete(sender, realDest);
+            return true;
         } catch (TaskInterruptedException e) {
             mm.taskInterruption(sender, ZTask.EXTRACT);
+            return false;
+        } catch (ZipException e) {
+            mm.extractionFormatError(sender, src, "GZip");
+            return false;
         } catch (IOException e) {
             e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.EXTRACT);
+            return false;
         }
     }
 
     @Override
-    public void compress(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public boolean compress(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         final MessageManager mm = MessageManager.inst();
         mm.startingProcess(sender, ZTask.COMPRESS, src.getName());
         try (GZIPOutputStream xzos = new GZIPOutputStream(new FileOutputStream(dest));
@@ -101,10 +109,14 @@ public void compress(ICommandSender sender, File src, File dest, boolean log, bo
             }
             if(!pipe)
                 mm.compressionComplete(sender, dest);
+            return true;
         } catch (TaskInterruptedException e) {
             mm.taskInterruption(sender, ZTask.COMPRESS);
+            return false;
         } catch (IOException e) {
             e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.COMPRESS);
+            return false;
         }
     }
 

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/JarProvider.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/JarProvider.java
@@ -29,6 +29,7 @@
 import java.util.jar.JarEntry;
 import java.util.jar.JarInputStream;
 import java.util.regex.Pattern;
+import java.util.zip.ZipException;
 
 import com.dscalzi.zipextractor.core.TaskInterruptedException;
 import com.dscalzi.zipextractor.core.ZTask;
@@ -78,7 +79,7 @@ public boolean canDetectPipedConflicts() {
     }
 
     @Override
-    public void extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         final MessageManager mm = MessageManager.inst();
         byte[] buffer = new byte[1024];
         mm.startingProcess(sender, ZTask.EXTRACT, src.getName());
@@ -112,12 +113,20 @@ public void extract(ICommandSender sender, File src, File dest, boolean log, boo
             jis.closeEntry();
             if(!pipe)
                 mm.extractionComplete(sender, dest);
+            return true;
         } catch (AccessDeniedException e) {
             mm.fileAccessDenied(sender, ZTask.EXTRACT, e.getMessage());
+            return false;
+        } catch (ZipException e) {
+            mm.extractionFormatError(sender, src, "Jar");
+            return false;
         } catch (TaskInterruptedException e) {
             mm.taskInterruption(sender, ZTask.EXTRACT);
+            return false;
         } catch (IOException e) {
             e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.EXTRACT);
+            return false;
         }
     }
 

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/PackProvider.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/PackProvider.java
@@ -61,7 +61,7 @@ public boolean canDetectPipedConflicts() {
     }
 
     @Override
-    public void extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         final MessageManager mm = MessageManager.inst();
         mm.startingProcess(sender, ZTask.EXTRACT, src.getName());
         File realDest = new File(dest.getAbsolutePath(), PATH_END_EXTRACT.matcher(src.getName()).replaceAll(""));
@@ -71,13 +71,16 @@ public void extract(ICommandSender sender, File src, File dest, boolean log, boo
             Pack200.newUnpacker().unpack(src, jarStream);
             if(!pipe)
                 mm.extractionComplete(sender, realDest);
+            return true;
         } catch (IOException e) {
             e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.EXTRACT);
+            return false;
         }
     }
 
     @Override
-    public void compress(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public boolean compress(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         final MessageManager mm = MessageManager.inst();
         mm.startingProcess(sender, ZTask.COMPRESS, src.getName());
         try (JarFile in = new JarFile(src); OutputStream out = Files.newOutputStream(dest.toPath())) {
@@ -86,8 +89,11 @@ public void compress(ICommandSender sender, File src, File dest, boolean log, bo
             Pack200.newPacker().pack(in, out);
             if(!pipe)
                 mm.compressionComplete(sender, dest);
+            return true;
         } catch (IOException e) {
             e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.COMPRESS);
+            return false;
         }
     }
 

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/RarProvider.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/RarProvider.java
@@ -39,6 +39,7 @@
 import com.dscalzi.zipextractor.core.util.ICommandSender;
 import com.github.junrar.Archive;
 import com.github.junrar.exception.RarException;
+import com.github.junrar.exception.RarException.RarExceptionType;
 import com.github.junrar.impl.FileVolumeManager;
 import com.github.junrar.rarfile.FileHeader;
 
@@ -85,7 +86,7 @@ public boolean canDetectPipedConflicts() {
     }
 
     @Override
-    public void extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         final MessageManager mm = MessageManager.inst();
         try (Archive a = new Archive(new FileVolumeManager(src))) {
             if (a != null) {
@@ -118,14 +119,28 @@ public void extract(ICommandSender sender, File src, File dest, boolean log, boo
                     fh = a.nextFileHeader();
                 }
             }
+            if(!pipe)
+                mm.extractionComplete(sender, dest);
+            return true;
         } catch (TaskInterruptedException e) {
             mm.taskInterruption(sender, ZTask.EXTRACT);
-            return;
-        } catch (RarException | IOException e) {
+            return false;
+        } catch(RarException e) {
+            if(e.getType() == RarExceptionType.notRarArchive) {
+                mm.extractionFormatError(sender, src, "Rar");
+            } else {
+                mm.warn("RarException of type " + e.getType().toString() + " thrown.");
+            }
+            return false;
+        } catch (IOException e) {
+            e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.EXTRACT);
+            return false;
+        } catch (Exception e) {
             e.printStackTrace();
+            mm.genericOperationError(sender, src, ZTask.EXTRACT);
+            return false;
         }
-        if(!pipe)
-            mm.extractionComplete(sender, dest);
     }
 
     @Override

diff --git a/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/TypeProvider.java b/ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/TypeProvider.java
@@ -89,8 +89,9 @@ public default List<String> scanForExtractionConflicts(ICommandSender sender, Fi
      *            Whether or not to log the progress.
      * @param pipe
      *            Whether this output will be piped.
+     * @return True if successful, false otherwise.
      */
-    public default void extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public default boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         throw new UnsupportedOperationException();
     }
 
@@ -107,8 +108,9 @@ public default void extract(ICommandSender sender, File src, File dest, boolean
      *            Whether or not to log the progress.
      * @param pipe
      *            Whether this output will be piped.
+     * @return True if successful, false otherwise.
      */
-    public default void compress(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
+    public default boolean compress(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
         throw new UnsupportedOperationException();
     }