refactor(ssrf): default enum MEDIUM prevents OFF=false (#4280)

# Description of Changes - **What was changed** - **URL to PDF flow** - Changed `ConvertWebsiteToPDF#urlToPdf` to return `ResponseEntity<?>` and perform a redirect (`303 SEE_OTHER`) back to `/url-to-pdf` with an `error` query param instead of throwing exceptions. - Added alert rendering in `url-to-pdf.html` using `param.error` for localized error display. - Introduced new translation key `error.invalidUrlFormat` in `messages_en_GB.properties`. - **Security / SSRF** - Migrated `ApplicationProperties.System.UrlSecurity.level` from `String` to `SsrfProtectionLevel` enum. - Default now set to `SsrfProtectionLevel.MEDIUM` (`// MAX, MEDIUM, OFF`). - This avoids the issue where setting `OFF` returned `false` in configuration parsing. - Updated `SsrfProtectionService#parseProtectionLevel` accordingly (using `level.name()`). - **Repo hygiene** - Added `**/LOCAL_APPDATA_FONTCONFIG_CACHE/**` to `.gitignore`. - **Why the change was made** - Provide user-friendly, localized error messages instead of exposing internal exceptions on URL-to-PDF conversions. - Ensure SSRF protection level parsing is type-safe and consistent—`OFF` can now be set without yielding a misleading `false` state. - Prevent unwanted fontconfig cache files from being tracked in Git. --- ## Checklist ### General - [x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details.
2025-09-08 17:51:20 +02:00 · 2025-09-04 13:39:37 +02:00 · 2025-09-04 13:39:37 +02:00 · 963b4ee69d
commit 963b4ee69d
parent cd76f5e50a
7 changed files with 271 additions and 49 deletions
--- a/.gitignore
+++ b/.gitignore
@ -200,3 +200,6 @@ id_ed25519.pub

 # node_modules
 node_modules/
+
+# weasyPrint
+**/LOCAL_APPDATA_FONTCONFIG_CACHE/**
--- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java
+++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java
@ -41,6 +41,7 @@ import stirling.software.common.model.oauth2.GitHubProvider;
 import stirling.software.common.model.oauth2.GoogleProvider;
 import stirling.software.common.model.oauth2.KeycloakProvider;
 import stirling.software.common.model.oauth2.Provider;
+import stirling.software.common.service.SsrfProtectionService.SsrfProtectionLevel;
 import stirling.software.common.util.ValidationUtils;

@Data
@ -390,7 +391,7 @@ public class ApplicationProperties {
        @Data
        public static class UrlSecurity {
            private boolean enabled = true;
-            private String level = "MEDIUM"; // MAX, MEDIUM, OFF
+            private SsrfProtectionLevel level = SsrfProtectionLevel.MEDIUM; // MAX, MEDIUM, OFF
            private List<String> allowedDomains = new ArrayList<>();
            private List<String> blockedDomains = new ArrayList<>();
            private List<String> internalTlds =
--- a/app/common/src/main/java/stirling/software/common/service/SsrfProtectionService.java
+++ b/app/common/src/main/java/stirling/software/common/service/SsrfProtectionService.java
@ -61,9 +61,9 @@ public class SsrfProtectionService {
        };
    }

-    private SsrfProtectionLevel parseProtectionLevel(String level) {
+    private SsrfProtectionLevel parseProtectionLevel(SsrfProtectionLevel level) {
        try {
-            return SsrfProtectionLevel.valueOf(level.toUpperCase());
+            return SsrfProtectionLevel.valueOf(level.name());
        } catch (IllegalArgumentException e) {
            log.warn("Invalid SSRF protection level '{}', defaulting to MEDIUM", level);
            return SsrfProtectionLevel.MEDIUM;
@ -215,7 +215,8 @@ public class SsrfProtectionService {
                return false;
            }
        }
-        // For IPv4-mapped IPv6 addresses, bytes 10 and 11 must be 0xff (i.e., address is ::ffff:w.x.y.z)
+        // For IPv4-mapped IPv6 addresses, bytes 10 and 11 must be 0xff (i.e., address is
+        // ::ffff:w.x.y.z)
        return addr[10] == (byte) 0xff && addr[11] == (byte) 0xff;
    }

--- a/app/core/src/main/java/stirling/software/SPDF/controller/api/converters/ConvertWebsiteToPDF.java
+++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/converters/ConvertWebsiteToPDF.java
@ -1,17 +1,21 @@
 package stirling.software.SPDF.controller.api.converters;

 import java.io.IOException;
+import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;

 import org.apache.pdfbox.pdmodel.PDDocument;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.servlet.support.ServletUriComponentsBuilder;
+import org.springframework.web.util.UriComponentsBuilder;

 import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.tags.Tag;
@ -23,7 +27,6 @@ import stirling.software.SPDF.model.api.converters.UrlToPdfRequest;
 import stirling.software.common.configuration.RuntimePathConfig;
 import stirling.software.common.model.ApplicationProperties;
 import stirling.software.common.service.CustomPDFDocumentFactory;
-import stirling.software.common.util.ExceptionUtils;
 import stirling.software.common.util.GeneralUtils;
 import stirling.software.common.util.ProcessExecutor;
 import stirling.software.common.util.ProcessExecutor.ProcessExecutorResult;
@ -46,24 +49,43 @@ public class ConvertWebsiteToPDF {
            description =
                    "This endpoint fetches content from a URL and converts it to a PDF format."
                            + " Input:N/A Output:PDF Type:SISO")
-    public ResponseEntity<byte[]> urlToPdf(@ModelAttribute UrlToPdfRequest request)
+    public ResponseEntity<?> urlToPdf(@ModelAttribute UrlToPdfRequest request)
            throws IOException, InterruptedException {
        String URL = request.getUrlInput();
+        UriComponentsBuilder uriComponentsBuilder =
+                ServletUriComponentsBuilder.fromCurrentContextPath().path("/url-to-pdf");
+        URI location = null;
+        HttpStatus status = HttpStatus.SEE_OTHER;

        if (!applicationProperties.getSystem().getEnableUrlToPDF()) {
-            throw ExceptionUtils.createIllegalArgumentException(
-                    "error.endpointDisabled", "This endpoint has been disabled by the admin");
-        }
+            location =
+                    uriComponentsBuilder
+                            .queryParam("error", "error.endpointDisabled")
+                            .build()
+                            .toUri();
+        } else
+
        // Validate the URL format
        if (!URL.matches("^https?://.*") || !GeneralUtils.isValidURL(URL)) {
-            throw ExceptionUtils.createInvalidArgumentException(
-                    "URL", "provided format is invalid");
-        }
+            location =
+                    uriComponentsBuilder
+                            .queryParam("error", "error.invalidUrlFormat")
+                            .build()
+                            .toUri();
+        } else

        // validate the URL is reachable
        if (!GeneralUtils.isURLReachable(URL)) {
-            throw ExceptionUtils.createIllegalArgumentException(
-                    "error.urlNotReachable", "URL is not reachable, please provide a valid URL");
+            location =
+                    uriComponentsBuilder
+                            .queryParam("error", "error.urlNotReachable")
+                            .build()
+                            .toUri();
+        }
+
+        if (location != null) {
+            log.info("Redirecting to: {}", location.toString());
+            return ResponseEntity.status(status).location(location).build();
        }

        Path tempOutputFile = null;
--- a/app/core/src/main/resources/messages_en_GB.properties
+++ b/app/core/src/main/resources/messages_en_GB.properties
@ -193,6 +193,7 @@ error.fileFormatRequired=File must be in {0} format
 error.invalidFormat=Invalid {0} format: {1}
 error.endpointDisabled=This endpoint has been disabled by the admin
 error.urlNotReachable=URL is not reachable, please provide a valid URL
+error.invalidUrlFormat=Invalid URL format provided. The provided format is invalid.

 # DPI and image rendering messages - used by frontend for dynamic translation
 # Backend sends: [TRANSLATE:messageKey:arg1,arg2] English message
--- a/app/core/src/main/resources/templates/convert/url-to-pdf.html
+++ b/app/core/src/main/resources/templates/convert/url-to-pdf.html
@ -19,6 +19,7 @@
              <span class="material-symbols-rounded tool-header-icon convertto">link</span>
              <span class="tool-header-text" th:text="#{URLToPDF.header}"></span>
            </div>
+            <p th:if="${not #lists.isEmpty(param.error)}" th:text="#{${param.error[0]}}" class="alert alert-danger text-center"></p>
            <form method="post" enctype="multipart/form-data" th:action="@{'/api/v1/convert/url/pdf'}">
              <input type="text" class="form-control" id="urlInput" name="urlInput" placeholder="http://">
              <br>
--- a/app/core/src/test/java/stirling/software/SPDF/controller/api/converters/ConvertWebsiteToPdfTest.java
+++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/converters/ConvertWebsiteToPdfTest.java
@ -1,71 +1,264 @@
 package stirling.software.SPDF.controller.api.converters;

-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.when;

+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Method;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+import org.apache.pdfbox.pdmodel.PDDocument;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.web.context.request.RequestContextHolder;
+import org.springframework.web.context.request.ServletRequestAttributes;

 import stirling.software.SPDF.model.api.converters.UrlToPdfRequest;
 import stirling.software.common.configuration.RuntimePathConfig;
 import stirling.software.common.model.ApplicationProperties;
 import stirling.software.common.service.CustomPDFDocumentFactory;
+import stirling.software.common.util.GeneralUtils;
+import stirling.software.common.util.ProcessExecutor;
+import stirling.software.common.util.ProcessExecutor.ProcessExecutorResult;
+import stirling.software.common.util.ProcessExecutor.Processes;
+import stirling.software.common.util.WebResponseUtils;

 public class ConvertWebsiteToPdfTest {

-    @Mock private CustomPDFDocumentFactory mockPdfDocumentFactory;
-
+    @Mock private CustomPDFDocumentFactory pdfDocumentFactory;
    @Mock private RuntimePathConfig runtimePathConfig;

    private ApplicationProperties applicationProperties;
-
-    private ConvertWebsiteToPDF convertWebsiteToPDF;
+    private ConvertWebsiteToPDF sut;
+    private AutoCloseable mocks;

    @BeforeEach
-    void setUp() {
-        MockitoAnnotations.openMocks(this);
+    void setUp() throws Exception {
+        mocks = MockitoAnnotations.openMocks(this);
+
+        // Feature einschalten (ggf. Struktur an dein Projekt anpassen)
        applicationProperties = new ApplicationProperties();
        applicationProperties.getSystem().setEnableUrlToPDF(true);
-        convertWebsiteToPDF =
-                new ConvertWebsiteToPDF(
-                        mockPdfDocumentFactory, runtimePathConfig, applicationProperties);
+
+        // Stubs, falls der Code weiterlaufen sollte
+        when(runtimePathConfig.getWeasyPrintPath()).thenReturn("/usr/bin/weasyprint");
+        when(pdfDocumentFactory.load(any(File.class))).thenReturn(new PDDocument());
+
+        // SUT bauen
+        sut = new ConvertWebsiteToPDF(pdfDocumentFactory, runtimePathConfig, applicationProperties);
+
+        // RequestContext für ServletUriComponentsBuilder bereitstellen
+        MockHttpServletRequest req = new MockHttpServletRequest();
+        req.setScheme("http");
+        req.setServerName("localhost");
+        req.setServerPort(8080);
+        RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(req));
+    }
+
+    @AfterEach
+    void tearDown() throws Exception {
+        RequestContextHolder.resetRequestAttributes();
+        if (mocks != null) mocks.close();
    }

    @Test
-    public void test_exemption_is_thrown_when_invalid_url_format_provided() {
+    void redirect_with_error_when_invalid_url_format_provided() throws Exception {
+        UrlToPdfRequest request = new UrlToPdfRequest();
+        request.setUrlInput("not-a-url");

-        String invalid_format_Url = "invalid-url";
+        ResponseEntity<?> resp = sut.urlToPdf(request);
+
+        assertEquals(HttpStatus.SEE_OTHER, resp.getStatusCode());
+        URI location = resp.getHeaders().getLocation();
+        assertNotNull(location, "Location header expected");
+        assertTrue(
+                location.getQuery() != null
+                        && location.getQuery().contains("error=error.invalidUrlFormat"));
+    }
+
+    @Test
+    void redirect_with_error_when_url_is_not_reachable() throws Exception {
+        UrlToPdfRequest request = new UrlToPdfRequest();
+        // .invalid ist per RFC reserviert und nicht auflösbar
+        request.setUrlInput("https://nonexistent.invalid/");
+
+        ResponseEntity<?> resp = sut.urlToPdf(request);
+
+        assertEquals(HttpStatus.SEE_OTHER, resp.getStatusCode());
+        URI location = resp.getHeaders().getLocation();
+        assertNotNull(location, "Location header expected");
+        assertTrue(
+                location.getQuery() != null
+                        && location.getQuery().contains("error=error.urlNotReachable"));
+    }
+
+    @Test
+    void redirect_with_error_when_endpoint_disabled() throws Exception {
+        // Feature deaktivieren
+        applicationProperties.getSystem().setEnableUrlToPDF(false);

        UrlToPdfRequest request = new UrlToPdfRequest();
-        request.setUrlInput(invalid_format_Url);
-        // Act
-        IllegalArgumentException thrown =
-                assertThrows(
-                        IllegalArgumentException.class,
-                        () -> {
-                            convertWebsiteToPDF.urlToPdf(request);
-                        });
-        // Assert
-        assertEquals("Invalid URL format: provided format is invalid", thrown.getMessage());
+        request.setUrlInput("https://example.com/");
+
+        ResponseEntity<?> resp = sut.urlToPdf(request);
+
+        assertEquals(HttpStatus.SEE_OTHER, resp.getStatusCode());
+        URI location = resp.getHeaders().getLocation();
+        assertNotNull(location, "Location header expected");
+        assertTrue(
+                location.getQuery() != null
+                        && location.getQuery().contains("error=error.endpointDisabled"));
    }

    @Test
-    public void test_exemption_is_thrown_when_url_is_not_reachable() {
+    void convertURLToFileName_sanitizes_and_appends_pdf() throws Exception {
+        Method m =
+                ConvertWebsiteToPDF.class.getDeclaredMethod("convertURLToFileName", String.class);
+        m.setAccessible(true);

-        String unreachable_Url = "https://www.googleeeexyz.com";
+        String in = "https://ex-ample.com/path?q=1&x=y#frag";
+        String out = (String) m.invoke(sut, in);
+
+        assertTrue(out.endsWith(".pdf"));
+        // Nur A–Z, a–z, 0–9, Unterstrich und Punkt erlaubt
+        assertTrue(out.matches("[A-Za-z0-9_]+\\.pdf"));
+        // keine Truncation hier (Quelle ist nicht so lang)
+        assertTrue(out.length() <= 54);
+    }
+
+    @Test
+    void convertURLToFileName_truncates_to_50_chars_before_pdf_suffix() throws Exception {
+        Method m =
+                ConvertWebsiteToPDF.class.getDeclaredMethod("convertURLToFileName", String.class);
+        m.setAccessible(true);
+
+        // Sehr lange URL → löst Truncation aus
+        String longUrl =
+                "https://very-very-long-domain.example.com/some/really/long/path/with?many=params&and=chars";
+        String out = (String) m.invoke(sut, longUrl);
+
+        assertTrue(out.endsWith(".pdf"));
+        assertTrue(out.matches("[A-Za-z0-9_]+\\.pdf"));
+        // safeName ist auf 50 begrenzt → total max 54 inkl. ".pdf"
+        assertTrue(out.length() <= 54, "Filename should be truncated to 50 + '.pdf'");
+    }
+
+    @Test
+    void happy_path_executes_weasyprint_loads_pdf_and_returns_response() throws Exception {
+        UrlToPdfRequest request = new UrlToPdfRequest();
+        request.setUrlInput("https://example.com");
+
+        try (MockedStatic<ProcessExecutor> pe = Mockito.mockStatic(ProcessExecutor.class);
+                MockedStatic<WebResponseUtils> wr = Mockito.mockStatic(WebResponseUtils.class);
+                MockedStatic<GeneralUtils> gu = Mockito.mockStatic(GeneralUtils.class)) {
+
+            // URL-Checks positiv erzwingen
+            gu.when(() -> GeneralUtils.isValidURL("https://example.com")).thenReturn(true);
+            gu.when(() -> GeneralUtils.isURLReachable("https://example.com")).thenReturn(true);
+
+            // richtiger ProcessExecutor!
+            ProcessExecutor mockExec = Mockito.mock(ProcessExecutor.class);
+            pe.when(() -> ProcessExecutor.getInstance(Processes.WEASYPRINT)).thenReturn(mockExec);
+
+            @SuppressWarnings("unchecked")
+            ArgumentCaptor<List<String>> cmdCaptor = ArgumentCaptor.forClass(List.class);
+
+            // Rückgabewert typgerecht
+            ProcessExecutorResult dummyResult = Mockito.mock(ProcessExecutorResult.class);
+            when(mockExec.runCommandWithOutputHandling(cmdCaptor.capture()))
+                    .thenReturn(dummyResult);
+
+            // WebResponseUtils mocken
+            ResponseEntity<byte[]> fakeResponse = ResponseEntity.ok(new byte[0]);
+            wr.when(() -> WebResponseUtils.pdfDocToWebResponse(any(PDDocument.class), anyString()))
+                    .thenReturn(fakeResponse);
+
+            // Act
+            ResponseEntity<?> resp = sut.urlToPdf(request);
+
+            // Assert – Response OK
+            assertEquals(HttpStatus.OK, resp.getStatusCode());
+
+            // Assert – WeasyPrint-Kommando korrekt
+            List<String> cmd = cmdCaptor.getValue();
+            assertNotNull(cmd);
+            assertEquals("/usr/bin/weasyprint", cmd.get(0));
+            assertEquals("https://example.com", cmd.get(1));
+            assertEquals("--pdf-forms", cmd.get(2));
+            assertTrue(cmd.size() >= 4, "WeasyPrint sollte einen Output-Pfad erhalten");
+            String outPathStr = cmd.get(3);
+            assertNotNull(outPathStr);
+
+            // Temp-Datei muss im finally gelöscht sein
+            Path outPath = Path.of(outPathStr);
+            assertFalse(
+                    Files.exists(outPath), "Temp-Output-Datei sollte nach dem Call gelöscht sein");
+        }
+    }
+
+    @Test
+    void finally_block_logs_and_swallows_ioexception_on_delete() throws Exception {
        // Arrange
        UrlToPdfRequest request = new UrlToPdfRequest();
-        request.setUrlInput(unreachable_Url);
-        // Act
-        IllegalArgumentException thrown =
-                assertThrows(
-                        IllegalArgumentException.class,
-                        () -> {
-                            convertWebsiteToPDF.urlToPdf(request);
-                        });
-        // Assert
-        assertEquals("URL is not reachable, please provide a valid URL", thrown.getMessage());
+        request.setUrlInput("https://example.com");
+
+        Path preCreatedTemp = java.nio.file.Files.createTempFile("test_output_", ".pdf");
+
+        try (MockedStatic<GeneralUtils> gu = Mockito.mockStatic(GeneralUtils.class);
+                MockedStatic<ProcessExecutor> pe = Mockito.mockStatic(ProcessExecutor.class);
+                MockedStatic<WebResponseUtils> wr = Mockito.mockStatic(WebResponseUtils.class);
+                MockedStatic<Files> files = Mockito.mockStatic(Files.class)) {
+
+            // URL-Checks positiv
+            gu.when(() -> GeneralUtils.isValidURL("https://example.com")).thenReturn(true);
+            gu.when(() -> GeneralUtils.isURLReachable("https://example.com")).thenReturn(true);
+
+            // Temp-Datei erzwingen + Delete-Fehler provozieren
+            files.when(() -> Files.createTempFile("output_", ".pdf")).thenReturn(preCreatedTemp);
+            files.when(() -> Files.deleteIfExists(preCreatedTemp))
+                    .thenThrow(new IOException("fail delete"));
+            files.when(() -> Files.exists(preCreatedTemp)).thenReturn(true); // für den Assert
+
+            // ProcessExecutor
+            ProcessExecutor mockExec = Mockito.mock(ProcessExecutor.class);
+            pe.when(() -> ProcessExecutor.getInstance(Processes.WEASYPRINT)).thenReturn(mockExec);
+            ProcessExecutorResult dummy = Mockito.mock(ProcessExecutorResult.class);
+            when(mockExec.runCommandWithOutputHandling(Mockito.<List>any())).thenReturn(dummy);
+
+            // WebResponseUtils
+            ResponseEntity<byte[]> fakeResponse = ResponseEntity.ok(new byte[0]);
+            wr.when(() -> WebResponseUtils.pdfDocToWebResponse(any(PDDocument.class), anyString()))
+                    .thenReturn(fakeResponse);
+
+            // Act: darf keine Exception werfen und soll eine Response liefern
+            ResponseEntity<?> resp = assertDoesNotThrow(() -> sut.urlToPdf(request));
+
+            // Assert
+            assertNotNull(resp, "Response should not be null");
+            assertEquals(HttpStatus.OK, resp.getStatusCode());
+            assertTrue(
+                    java.nio.file.Files.exists(preCreatedTemp),
+                    "Temp-Datei sollte trotz Lösch-IOException noch existieren");
+        } finally {
+            try {
+                java.nio.file.Files.deleteIfExists(preCreatedTemp);
+            } catch (IOException ignore) {
+            }
+        }
    }
 }