From 1e4134c7d1cd73b356aa1914f99aad19c2c8c649 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Sun, 10 Mar 2024 14:00:00 +0000 Subject: [PATCH] Number fxes (#898) * init * user and pass to just pass lang update * session management fixes and avoid demo user locking * fix for UMASK and extract cleanups * fixes for user #889 and #332 * increase session count for demo site * fix * gcc * formatting * number fixes init * || true test * version bump * Hardening suggestions for Stirling-PDF / numberFxes (#899) Switch order of literals to prevent NullPointerException Co-authored-by: pixeebot[bot] <104101892+pixeebot[bot]@users.noreply.github.com> --------- Co-authored-by: pixeebot[bot] <104101892+pixeebot[bot]@users.noreply.github.com> --- build.gradle | 5 +- scripts/download-security-jar.sh | 4 +- scripts/init-without-ocr.sh | 10 +- scripts/init.sh | 10 +- .../api/RearrangePagesPDFController.java | 4 +- .../controller/api/SplitPDFController.java | 12 +- .../controller/api/misc/StampController.java | 2 +- .../SPDF/model/api/PDFWithPageNums.java | 4 +- .../software/SPDF/utils/GeneralUtils.java | 170 ++++++++++-------- .../software/SPDF/utils/GeneralUtilsTest.java | 105 +++++++++++ 10 files changed, 230 insertions(+), 96 deletions(-) create mode 100644 src/test/java/stirling/software/SPDF/utils/GeneralUtilsTest.java diff --git a/build.gradle b/build.gradle index 011ad008..06141dba 100644 --- a/build.gradle +++ b/build.gradle @@ -12,7 +12,8 @@ plugins { import com.github.jk1.license.render.* group = 'stirling.software' -version = '0.22.1' + +version = '0.22.2' sourceCompatibility = '17' repositories { @@ -158,6 +159,8 @@ dependencies { // https://mvnrepository.com/artifact/com.github.vladimir-bukhtoyarov/bucket4j-core implementation 'com.github.vladimir-bukhtoyarov:bucket4j-core:7.6.0' + implementation 'com.fathzer:javaluator:3.0.3' + developmentOnly("org.springframework.boot:spring-boot-devtools:3.2.2") compileOnly 'org.projectlombok:lombok:1.18.30' annotationProcessor 'org.projectlombok:lombok:1.18.28' diff --git a/scripts/download-security-jar.sh b/scripts/download-security-jar.sh index 0b9aa831..42ca2b5a 100644 --- a/scripts/download-security-jar.sh +++ b/scripts/download-security-jar.sh @@ -14,8 +14,8 @@ if [ "$DOCKER_ENABLE_SECURITY" = "true" ] && [ "$VERSION_TAG" != "alpha" ]; then if [ $? -eq 0 ]; then # checks if curl was successful rm -f app.jar ln -s app-security.jar app.jar - chown stirlingpdfuser:stirlingpdfgroup app.jar - chmod 755 app.jar + chown stirlingpdfuser:stirlingpdfgroup app.jar || true + chmod 755 app.jar || true fi fi fi diff --git a/scripts/init-without-ocr.sh b/scripts/init-without-ocr.sh index 7bd129c8..4673b9dd 100644 --- a/scripts/init-without-ocr.sh +++ b/scripts/init-without-ocr.sh @@ -2,17 +2,17 @@ # Update the user and group IDs as per environment variables if [ ! -z "$PUID" ] && [ "$PUID" != "$(id -u stirlingpdfuser)" ]; then - usermod -o -u "$PUID" stirlingpdfuser + usermod -o -u "$PUID" stirlingpdfuser || true fi if [ ! -z "$PGID" ] && [ "$PGID" != "$(getent group stirlingpdfgroup | cut -d: -f3)" ]; then - groupmod -o -g "$PGID" stirlingpdfgroup + groupmod -o -g "$PGID" stirlingpdfgroup || true fi -umask "$UMASK" +umask "$UMASK" || true echo "Setting permissions and ownership for necessary directories..." -chown -R stirlingpdfuser:stirlingpdfgroup $HOME /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar -chmod -R 755 /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar +chown -R stirlingpdfuser:stirlingpdfgroup $HOME /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar || true +chmod -R 755 /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar || true if [[ "$INSTALL_BOOK_AND_ADVANCED_HTML_OPS" == "true" ]]; then apk add --no-cache calibre@testing fi diff --git a/scripts/init.sh b/scripts/init.sh index 58d64e59..1b298e16 100644 --- a/scripts/init.sh +++ b/scripts/init.sh @@ -15,18 +15,18 @@ fi # Update the user and group IDs as per environment variables if [ ! -z "$PUID" ] && [ "$PUID" != "$(id -u stirlingpdfuser)" ]; then - usermod -o -u "$PUID" stirlingpdfuser + usermod -o -u "$PUID" stirlingpdfuser || true fi if [ ! -z "$PGID" ] && [ "$PGID" != "$(getent group stirlingpdfgroup | cut -d: -f3)" ]; then - groupmod -o -g "$PGID" stirlingpdfgroup + groupmod -o -g "$PGID" stirlingpdfgroup || true fi -umask "$UMASK" +umask "$UMASK" || true echo "Setting permissions and ownership for necessary directories..." -chown -R stirlingpdfuser:stirlingpdfgroup $HOME /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar -chmod -R 755 /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar +chown -R stirlingpdfuser:stirlingpdfgroup $HOME /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar || true +chmod -R 755 /logs /scripts /usr/share/fonts/opentype/noto /usr/share/tessdata /configs /customFiles /pipeline /app.jar || true diff --git a/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java b/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java index f6170485..a1e80af3 100644 --- a/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java +++ b/src/main/java/stirling/software/SPDF/controller/api/RearrangePagesPDFController.java @@ -51,7 +51,7 @@ public class RearrangePagesPDFController { String[] pageOrderArr = pagesToDelete.split(","); List pagesToRemove = - GeneralUtils.parsePageList(pageOrderArr, document.getNumberOfPages(), true); + GeneralUtils.parsePageList(pageOrderArr, document.getNumberOfPages(), false); Collections.sort(pagesToRemove); @@ -195,7 +195,7 @@ public class RearrangePagesPDFController { if (sortType != null && sortType.length() > 0) { newPageOrder = processSortTypes(sortType, totalPages); } else { - newPageOrder = GeneralUtils.parsePageList(pageOrderArr, totalPages, true); + newPageOrder = GeneralUtils.parsePageList(pageOrderArr, totalPages, false); } logger.info("newPageOrder = " + newPageOrder); logger.info("totalPages = " + totalPages); diff --git a/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java b/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java index 6979b40b..e415aa9d 100644 --- a/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java +++ b/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java @@ -49,12 +49,14 @@ public class SplitPDFController { // open the pdf document PDDocument document = Loader.loadPDF(file.getBytes()); - - List pageNumbers = request.getPageNumbersList(document, true); - if (!pageNumbers.contains(document.getNumberOfPages() - 1)) { + int totalPages = document.getNumberOfPages(); + List pageNumbers = request.getPageNumbersList(document, false); + System.out.println( + pageNumbers.stream().map(String::valueOf).collect(Collectors.joining(","))); + if (!pageNumbers.contains(totalPages - 1)) { // Create a mutable ArrayList so we can add to it pageNumbers = new ArrayList<>(pageNumbers); - pageNumbers.add(document.getNumberOfPages() - 1); + pageNumbers.add(totalPages - 1); } logger.info( @@ -69,7 +71,7 @@ public class SplitPDFController { for (int i = previousPageNumber; i <= splitPoint; i++) { PDPage page = document.getPage(i); splitDocument.addPage(page); - logger.debug("Adding page {} to split document", i); + logger.info("Adding page {} to split document", i); } previousPageNumber = splitPoint + 1; diff --git a/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java b/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java index 3b541b6c..e5d326a9 100644 --- a/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java +++ b/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java @@ -88,7 +88,7 @@ public class StampController { // Load the input PDF PDDocument document = Loader.loadPDF(pdfFile.getBytes()); - List pageNumbers = request.getPageNumbersList(document, false); + List pageNumbers = request.getPageNumbersList(document, true); for (int pageIndex : pageNumbers) { int zeroBasedIndex = pageIndex - 1; diff --git a/src/main/java/stirling/software/SPDF/model/api/PDFWithPageNums.java b/src/main/java/stirling/software/SPDF/model/api/PDFWithPageNums.java index 5e15d64d..7f94791e 100644 --- a/src/main/java/stirling/software/SPDF/model/api/PDFWithPageNums.java +++ b/src/main/java/stirling/software/SPDF/model/api/PDFWithPageNums.java @@ -33,13 +33,13 @@ public class PDFWithPageNums extends PDFFile { // TODO Auto-generated catch block e.printStackTrace(); } - return GeneralUtils.parsePageString(pageNumbers, pageCount, zeroCount); + return GeneralUtils.parsePageList(pageNumbers, pageCount, zeroCount); } @Hidden public List getPageNumbersList(PDDocument doc, boolean zeroCount) { int pageCount = 0; pageCount = doc.getNumberOfPages(); - return GeneralUtils.parsePageString(pageNumbers, pageCount, zeroCount); + return GeneralUtils.parsePageList(pageNumbers, pageCount, zeroCount); } } diff --git a/src/main/java/stirling/software/SPDF/utils/GeneralUtils.java b/src/main/java/stirling/software/SPDF/utils/GeneralUtils.java index d1a58087..b0bb1aee 100644 --- a/src/main/java/stirling/software/SPDF/utils/GeneralUtils.java +++ b/src/main/java/stirling/software/SPDF/utils/GeneralUtils.java @@ -12,11 +12,12 @@ import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import org.springframework.web.multipart.MultipartFile; +import com.fathzer.soft.javaluator.DoubleEvaluator; + import io.github.pixee.security.HostValidator; import io.github.pixee.security.Urls; @@ -115,91 +116,114 @@ public class GeneralUtils { return null; } - public static List parsePageString(String pageOrder, int totalPages) { - return parsePageString(pageOrder, totalPages, false); - } - - public static List parsePageString( - String pageOrder, int totalPages, boolean isOneBased) { - if (pageOrder == null || pageOrder.isEmpty()) { - return Collections.singletonList(1); + public static List parsePageList(String pages, int totalPages, boolean oneBased) { + if (pages == null) { + return List.of(1); // Default to first page if input is null } - if (pageOrder.matches("\\d+")) { - // Convert the single number string to an integer and return it in a list - return Collections.singletonList(Integer.parseInt(pageOrder)); + try { + return parsePageList(pages.split(","), totalPages, oneBased); + } catch (NumberFormatException e) { + return List.of(1); // Default to first page if input is invalid } - return parsePageList(pageOrder.split(","), totalPages, isOneBased); } - public static List parsePageList(String[] pageOrderArr, int totalPages) { - return parsePageList(pageOrderArr, totalPages, false); + public static List parsePageList(String[] pages, int totalPages) { + return parsePageList(pages, totalPages, false); } - public static List parsePageList( - String[] pageOrderArr, int totalPages, boolean isOneBased) { - List newPageOrder = new ArrayList<>(); - - int adjustmentFactor = isOneBased ? 1 : 0; - - // loop through the page order array - for (String element : pageOrderArr) { - if ("all".equalsIgnoreCase(element)) { + public static List parsePageList(String[] pages, int totalPages, boolean oneBased) { + List result = new ArrayList<>(); + int offset = oneBased ? 1 : 0; + for (String page : pages) { + if ("all".equalsIgnoreCase(page)) { for (int i = 0; i < totalPages; i++) { - newPageOrder.add(i + adjustmentFactor); + result.add(i + offset); } - // As all pages are already added, no need to check further - break; - } else if (element.matches("\\d*n\\+?-?\\d*|\\d*\\+?n")) { - // Handle page order as a function - int coefficient = 0; - int constant = 0; - boolean coefficientExists = false; - boolean constantExists = false; - - if (element.contains("n")) { - String[] parts = element.split("n"); - if (!"".equals(parts[0]) && parts[0] != null) { - coefficient = Integer.parseInt(parts[0]); - coefficientExists = true; - } - if (parts.length > 1 && !"".equals(parts[1]) && parts[1] != null) { - constant = Integer.parseInt(parts[1]); - constantExists = true; - } - } else if (element.contains("+")) { - constant = Integer.parseInt(element.replace("+", "")); - constantExists = true; - } - - for (int i = 1; i <= totalPages; i++) { - int pageNum = coefficientExists ? coefficient * i : i; - pageNum += constantExists ? constant : 0; - - if (pageNum <= totalPages && pageNum > 0) { - newPageOrder.add(pageNum - adjustmentFactor); - } - } - } else if (element.contains("-")) { - // split the range into start and end page - String[] range = element.split("-"); - int start = Integer.parseInt(range[0]); - int end = Integer.parseInt(range[1]); - // check if the end page is greater than total pages - if (end > totalPages) { - end = totalPages; - } - // loop through the range of pages - for (int j = start; j <= end; j++) { - // print the current index - newPageOrder.add(j - adjustmentFactor); + } else if (page.contains(",")) { + // Split the string into parts, could be single pages or ranges + String[] parts = page.split(","); + for (String part : parts) { + result.addAll(handlePart(part, totalPages, offset)); } } else { - // if the element is a single page - newPageOrder.add(Integer.parseInt(element) - adjustmentFactor); + result.addAll(handlePart(page, totalPages, offset)); } } + return new ArrayList<>( + new java.util.LinkedHashSet<>(result)); // Remove duplicates and maintain order + } - return newPageOrder; + public static List evaluateNFunc(String expression, int maxValue) { + List results = new ArrayList<>(); + DoubleEvaluator evaluator = new DoubleEvaluator(); + + // Validate the expression + if (!expression.matches("[0-9n+\\-*/() ]+")) { + throw new IllegalArgumentException("Invalid expression"); + } + + int n = 0; + while (true) { + // Replace 'n' with the current value of n, correctly handling numbers before 'n' + String sanitizedExpression = insertMultiplicationBeforeN(expression, n); + Double result = evaluator.evaluate(sanitizedExpression); + + // Check if the result is null or not within bounds + if (result == null || result <= 0 || result.intValue() > maxValue) { + if (n != 0) break; + } else { + results.add(result.intValue()); + } + n++; + } + + return results; + } + + private static String insertMultiplicationBeforeN(String expression, int nValue) { + // Insert multiplication between a number and 'n' (e.g., "4n" becomes "4*n") + String withMultiplication = expression.replaceAll("(\\d)n", "$1*n"); + // Now replace 'n' with its current value + return withMultiplication.replaceAll("n", String.valueOf(nValue)); + } + + private static List handlePart(String part, int totalPages, int offset) { + List partResult = new ArrayList<>(); + + // First check for n-syntax because it should not be processed as a range + if (part.contains("n")) { + partResult = evaluateNFunc(part, totalPages); + // Adjust the results according to the offset + for (int i = 0; i < partResult.size(); i++) { + int adjustedValue = partResult.get(i) - 1 + offset; + partResult.set(i, adjustedValue); + } + } else if (part.contains("-")) { + // Process ranges only if it's not n-syntax + String[] rangeParts = part.split("-"); + try { + int start = Integer.parseInt(rangeParts[0]); + int end = Integer.parseInt(rangeParts[1]); + for (int i = start; i <= end; i++) { + if (i >= 1 && i <= totalPages) { + partResult.add(i - 1 + offset); + } + } + } catch (NumberFormatException e) { + // Range is invalid, ignore this part + } + } else { + // This is a single page number + try { + int pageNum = Integer.parseInt(part.trim()); + if (pageNum >= 1 && pageNum <= totalPages) { + partResult.add(pageNum - 1 + offset); + } + } catch (NumberFormatException ignored) { + // Ignore invalid numbers + } + } + return partResult; } public static boolean createDir(String path) { diff --git a/src/test/java/stirling/software/SPDF/utils/GeneralUtilsTest.java b/src/test/java/stirling/software/SPDF/utils/GeneralUtilsTest.java new file mode 100644 index 00000000..14f14c82 --- /dev/null +++ b/src/test/java/stirling/software/SPDF/utils/GeneralUtilsTest.java @@ -0,0 +1,105 @@ +package stirling.software.SPDF.utils; + +import static org.junit.jupiter.api.Assertions.*; +import org.junit.jupiter.api.Test; +import java.util.List; + + +public class GeneralUtilsTest { + + + + @Test + void testParsePageListWithAll() { + List result = GeneralUtils.parsePageList(new String[]{"all"}, 5, false); + assertEquals(List.of(0, 1, 2, 3, 4), result, "'All' keyword should return all pages."); + } + + @Test + void testParsePageListWithAllOneBased() { + List result = GeneralUtils.parsePageList(new String[]{"all"}, 5, true); + assertEquals(List.of(1, 2, 3, 4, 5), result, "'All' keyword should return all pages."); + } + + @Test + void nFunc() { + List result = GeneralUtils.parsePageList(new String[]{"n"}, 5, true); + assertEquals(List.of(1, 2, 3, 4, 5), result, "'n' keyword should return all pages."); + } + + @Test + void nFuncAdvanced() { + List result = GeneralUtils.parsePageList(new String[]{"4n"}, 9, true); + //skip 0 as not valid + assertEquals(List.of(4,8), result, "'All' keyword should return all pages."); + } + + @Test + void nFuncAdvancedZero() { + List result = GeneralUtils.parsePageList(new String[]{"4n"}, 9, false); + //skip 0 as not valid + assertEquals(List.of(3,7), result, "'All' keyword should return all pages."); + } + + @Test + void nFuncAdvanced2() { + List result = GeneralUtils.parsePageList(new String[]{"4n-1"}, 9, true); + // skip -1 as not valid + assertEquals(List.of(3,7), result, "4n-1 should do (0-1), (4-1), (8-1)"); + } + + @Test + void nFuncAdvanced3() { + List result = GeneralUtils.parsePageList(new String[]{"4n+1"}, 9, true); + assertEquals(List.of(1,5,9), result, "'All' keyword should return all pages."); + } + + + @Test + void nFuncAdvanced4() { + List result = GeneralUtils.parsePageList(new String[]{"3+2n"}, 9, true); + assertEquals(List.of(3,5,7,9), result, "'All' keyword should return all pages."); + } + + @Test + void nFuncAdvancedZerobased() { + List result = GeneralUtils.parsePageList(new String[]{"4n"}, 9, false); + assertEquals(List.of(3,7), result, "'All' keyword should return all pages."); + } + + @Test + void nFuncAdvanced2Zerobased() { + List result = GeneralUtils.parsePageList(new String[]{"4n-1"}, 9, false); + assertEquals(List.of(2,6), result, "'All' keyword should return all pages."); + } + @Test + void testParsePageListWithRangeOneBasedOutput() { + List result = GeneralUtils.parsePageList(new String[]{"1-3"}, 5, true); + assertEquals(List.of(1, 2, 3), result, "Range should be parsed correctly."); + } + + + @Test + void testParsePageListWithRangeZeroBaseOutput() { + List result = GeneralUtils.parsePageList(new String[]{"1-3"}, 5, false); + assertEquals(List.of(0, 1, 2), result, "Range should be parsed correctly."); + } + + + @Test + void testParsePageListWithRangeOneBasedOutputFull() { + List result = GeneralUtils.parsePageList(new String[]{"1,3,7-8"}, 8, true); + assertEquals(List.of(1, 3, 7,8), result, "Range should be parsed correctly."); + } + + @Test + void testParsePageListWithRangeOneBasedOutputFullOutOfRange() { + List result = GeneralUtils.parsePageList(new String[]{"1,3,7-8"}, 5, true); + assertEquals(List.of(1, 3), result, "Range should be parsed correctly."); + } + @Test + void testParsePageListWithRangeZeroBaseOutputFull() { + List result = GeneralUtils.parsePageList(new String[]{"1,3,7-8"}, 8, false); + assertEquals(List.of(0, 2, 6,7), result, "Range should be parsed correctly."); + } +}