Browse Source

fix: The "window-status" is causing the "process.waitFor()" to block indefinitely. (#131)

* When the window-status parameter is used in PDF generation, add a windowStatusTimeout to prevent the thread from being blocked indefinitely in case of a SyntaxError: Parse error in JavaScript, which may cause the failure of setting the window-status.

* close process

* fix: Remove the windowStatusTimeout parameter and add unit tests for windowStatusTimeout.

* Modification: Update the test URL for the WithWindowStatusTimeout unit test.

* Modification: Remove excessive comments according to the code style.
weiba 1 year ago
parent
commit
bac1394389

+ 7 - 11
src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/Pdf.java

@@ -3,11 +3,7 @@ package com.github.jhonnymertz.wkhtmltopdf.wrapper;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.configurations.FilenameFilterConfig;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.configurations.WrapperConfig;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.exceptions.PDFExportException;
-import com.github.jhonnymertz.wkhtmltopdf.wrapper.objects.BaseObject;
-import com.github.jhonnymertz.wkhtmltopdf.wrapper.objects.Cover;
-import com.github.jhonnymertz.wkhtmltopdf.wrapper.objects.Page;
-import com.github.jhonnymertz.wkhtmltopdf.wrapper.objects.SourceType;
-import com.github.jhonnymertz.wkhtmltopdf.wrapper.objects.TableOfContents;
+import com.github.jhonnymertz.wkhtmltopdf.wrapper.objects.*;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.params.Param;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.params.Params;
 import org.apache.commons.io.FileUtils;
@@ -26,11 +22,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;
 import java.util.stream.Collectors;
 
 /**
@@ -320,7 +312,11 @@ public class Pdf {
             Future<byte[]> inputStreamToByteArray = executor.submit(streamToByteArrayTask(process.getInputStream()));
             Future<byte[]> outputStreamToByteArray = executor.submit(streamToByteArrayTask(process.getErrorStream()));
 
-            process.waitFor();
+            if (!process.waitFor(this.timeout, TimeUnit.SECONDS)) {
+                process.destroy();
+                logger.error("PDF generation failed by defined timeout of {}s, command: {}", timeout, command);
+                throw new RuntimeException("PDF generation timeout by user. Try to increase the timeout.");
+            }
 
             if (!successValues.contains(process.exitValue())) {
                 byte[] errorStream = getFuture(outputStreamToByteArray);

+ 22 - 0
src/test/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/integration/PdfIntegrationTests.java

@@ -247,4 +247,26 @@ class PdfIntegrationTests {
 
         assertThat("document should be generated", pdfText, containsString("Google"));
     }
+
+    @Test
+    void testPdfWithWindowStatusTimeoutParameters() throws Exception {
+        final String executable = WrapperConfig.findExecutable();
+        Pdf pdf = new Pdf(new WrapperConfig(executable));
+        pdf.addPageFromUrl("http://www.google.com");
+
+        pdf.addParam(new Param("--window-status", "complete"));
+
+        String exceptionMessage = "";
+        try {
+            /*
+             * Due to the absence of the `window.status` assignment on the `www.google.com` page, a timeout exception is certain to occur.
+             * This simulates a scenario where the `window.status` assignment fails.
+             * */
+            pdf.getPDF();
+        } catch (RuntimeException e) {
+            exceptionMessage = e.getMessage();
+        }
+        assertThat("it should throw a PDF generation timeout exception", exceptionMessage, containsString("PDF generation timeout by user"));
+
+    }
 }