diff --git a/app/common/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java b/app/common/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java index 81e70a6319..858227a50a 100644 --- a/app/common/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java +++ b/app/common/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java @@ -648,6 +648,10 @@ public class EndpointConfiguration { disableEndpoint("accessibility-report"); } + // Only FormDetectionModelManager (proprietary) can enable this; default it off so a core + // build does not advertise a tool whose controller is not on the classpath. + disableEndpoint("form-detection", DisableReason.DEPENDENCY); + if (!applicationProperties.getSystem().isEnableUrlToPDF()) { disableEndpoint("url-to-pdf"); } diff --git a/app/common/src/test/java/stirling/software/SPDF/config/EndpointConfigurationGapTest.java b/app/common/src/test/java/stirling/software/SPDF/config/EndpointConfigurationGapTest.java index afc6fa7020..7064c7695d 100644 --- a/app/common/src/test/java/stirling/software/SPDF/config/EndpointConfigurationGapTest.java +++ b/app/common/src/test/java/stirling/software/SPDF/config/EndpointConfigurationGapTest.java @@ -463,6 +463,17 @@ class EndpointConfigurationGapTest { assertFalse(config.isEndpointEnabled("remove-pages")); } + @Test + @DisplayName( + "form-detection defaults to disabled with DEPENDENCY when the proprietary module is absent") + void formDetectionDisabledWithoutProprietaryModule() { + EndpointConfiguration config = buildDefault(); + assertFalse(config.isEndpointEnabled("form-detection")); + assertEquals( + DisableReason.DEPENDENCY, + config.getEndpointAvailability("form-detection").getReason()); + } + @Test @DisplayName("non-pro build disables the enterprise group") void nonProDisablesEnterprise() { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java b/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java index 9ecfba5cc0..bad1e0382d 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java @@ -67,6 +67,12 @@ public class FormDetectionModelManager { private static final int MAX_REDIRECTS = 5; + /** Slack over the catalogue size; a larger body could never match the checksum anyway. */ + private static final long DOWNLOAD_SLACK_BYTES = 8L * 1024 * 1024; + + /** Ceiling when neither the catalogue nor Content-Length declares a size. */ + private static final long MAX_DOWNLOAD_BYTES = 512L * 1024 * 1024; + /** * Whether the ONNX engine is bundled in this build; the jar only ships with {@code * -PbundleOnnxRuntime=true}. Without it the tool cannot run at all. @@ -125,13 +131,20 @@ public class FormDetectionModelManager { private void applyEndpointState() { if (!isFeatureEnabled()) { endpointConfiguration.disableEndpoint(ENDPOINT_KEY, DisableReason.CONFIG); - } else if (state == FormDetectionStatus.READY && getActiveModelFile().isPresent()) { + } else if (isServerEngineAvailable() + && state == FormDetectionStatus.READY + && getActiveModelFile().isPresent()) { endpointConfiguration.enableEndpoint(ENDPOINT_KEY); } else { endpointConfiguration.disableEndpoint(ENDPOINT_KEY, DisableReason.DEPENDENCY); } } + /** Package-private so a test can simulate a build packaged without the ONNX runtime. */ + boolean isServerEngineAvailable() { + return SERVER_ENGINE_AVAILABLE; + } + public boolean isFeatureEnabled() { return applicationProperties.getFormDetection().isEnabled(); } @@ -195,6 +208,7 @@ public class FormDetectionModelManager { } catch (Exception e) { log.error("Auto Form Detection install failed for {}", modelId, e); error = e.getMessage(); + progress = 0; // Keep a previously-installed model usable if the new one failed. state = getActiveModelFile().isPresent() @@ -244,6 +258,7 @@ public class FormDetectionModelManager { conn = openModelDownload(url); long total = entry.getSizeBytes() > 0 ? entry.getSizeBytes() : conn.getContentLengthLong(); + long ceiling = total > 0 ? total + DOWNLOAD_SLACK_BYTES : MAX_DOWNLOAD_BYTES; try (InputStream in = conn.getInputStream(); OutputStream out = Files.newOutputStream(tmp, CREATE, TRUNCATE_EXISTING, WRITE)) { @@ -251,14 +266,28 @@ public class FormDetectionModelManager { long read = 0; int n; while ((n = in.read(buf)) >= 0) { + read += n; + if (read > ceiling) { + throw new IOException( + "Model download exceeded the expected size of " + + ceiling + + " bytes"); + } out.write(buf, 0, n); digest.update(buf, 0, n); - read += n; if (total > 0) { progress = (int) Math.min(99, (read * 100) / total); } } } + } catch (IOException | RuntimeException e) { + // The partial file can never verify, so do not leave it on the configs volume. + try { + Files.deleteIfExists(tmp); + } catch (IOException suppressed) { + e.addSuppressed(suppressed); + } + throw e; } finally { if (conn != null) { conn.disconnect(); @@ -331,8 +360,11 @@ public class FormDetectionModelManager { || status == 308; } - /** Resolve a redirect target and re-apply the https + host allowlist to it. */ - private static String requireAllowedRedirect(String from, String location) throws IOException { + /** + * Resolve a redirect target and re-apply the https + host allowlist to it. Package-private so + * the allowlist can be tested directly. + */ + static String requireAllowedRedirect(String from, String location) throws IOException { URI next; try { next = URI.create(from).resolve(location); @@ -484,7 +516,7 @@ public class FormDetectionModelManager { isWritable(dir), catalog.getAll(), isFeatureEnabled(), - SERVER_ENGINE_AVAILABLE, + isServerEngineAvailable(), downloadingModelId); } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java index 960ad3c656..3af9ab5ff7 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java @@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import java.io.IOException; +import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.InetSocketAddress; import java.net.URI; @@ -30,6 +31,7 @@ import org.springframework.beans.factory.ObjectProvider; import com.sun.net.httpserver.HttpServer; import stirling.software.SPDF.config.EndpointConfiguration; +import stirling.software.SPDF.config.EndpointConfiguration.DisableReason; import stirling.software.common.configuration.RuntimePathConfig; import stirling.software.common.model.ApplicationProperties; import stirling.software.proprietary.formdetection.catalog.ModelCatalogService; @@ -95,6 +97,21 @@ class FormDetectionModelManagerTest { ModelCatalogEntry entry, EndpointConfiguration ep, ApplicationProperties props) { + return manager(dir, entry, ep, props, true); + } + + /** Stands in for a build packaged without the ONNX runtime, where the tool cannot run. */ + private FormDetectionModelManager managerWithoutEngine( + Path dir, ModelCatalogEntry entry, EndpointConfiguration ep) { + return manager(dir, entry, ep, new ApplicationProperties(), false); + } + + private FormDetectionModelManager manager( + Path dir, + ModelCatalogEntry entry, + EndpointConfiguration ep, + ApplicationProperties props, + boolean serverEngineAvailable) { RuntimePathConfig paths = Mockito.mock(RuntimePathConfig.class); Mockito.when(paths.getFormDetectionModelPath()).thenReturn(dir.toString()); ModelCatalogService catalog = Mockito.mock(ModelCatalogService.class); @@ -113,6 +130,11 @@ class FormDetectionModelManagerTest { conn.setReadTimeout(10000); return conn; } + + @Override + boolean isServerEngineAvailable() { + return serverEngineAvailable; + } }; } @@ -160,6 +182,94 @@ class FormDetectionModelManagerTest { Mockito.verify(ep, Mockito.never()).enableEndpoint("form-detection"); } + @Test + void followsRedirectsOnlyWithinTheAllowedDomains() throws Exception { + String from = "https://huggingface.co/a/b.onnx"; + // Hugging Face hands the file off to its own CDN, which is a subdomain of hf.co. + assertEquals( + "https://cdn-lfs.hf.co/x", + FormDetectionModelManager.requireAllowedRedirect(from, "https://cdn-lfs.hf.co/x")); + assertEquals( + "https://huggingface.co/c/d.onnx", + FormDetectionModelManager.requireAllowedRedirect(from, "/c/d.onnx")); + } + + @Test + void rejectsRedirectsOffTheAllowlist() { + String from = "https://huggingface.co/a/b.onnx"; + assertThrows( + IOException.class, + () -> + FormDetectionModelManager.requireAllowedRedirect( + from, "https://evil.example/x")); + assertThrows( + IOException.class, + () -> + FormDetectionModelManager.requireAllowedRedirect( + from, "https://huggingface.co.evil.example/x")); + assertThrows( + IOException.class, + () -> + FormDetectionModelManager.requireAllowedRedirect( + from, "http://huggingface.co/x")); + assertThrows( + IOException.class, + () -> + FormDetectionModelManager.requireAllowedRedirect( + from, "https://user@huggingface.co/x")); + } + + @Test + void abortsAnOversizedDownloadAndLeavesNoPartialFile(@TempDir Path dir) throws Exception { + // Well past the 8MB slack the manager allows over the catalogue size. + long size = modelBytes.length + 9L * 1024 * 1024; + server.createContext( + "/huge.onnx", + ex -> { + ex.sendResponseHeaders(200, size); + byte[] chunk = new byte[1 << 16]; + try (OutputStream body = ex.getResponseBody()) { + for (long sent = 0; sent < size; sent += chunk.length) { + body.write(chunk, 0, (int) Math.min(chunk.length, size - sent)); + } + } catch (IOException ignored) { + // Expected: the client aborts as soon as the ceiling trips. + } + ex.close(); + }); + FormDetectionModelManager m = + manager( + dir, + entry(ALLOWED_URL + "/huge.onnx", modelSha), + Mockito.mock(EndpointConfiguration.class)); + + m.startInstall("test-model"); + awaitState(m, "failed", 15000); + + assertFalse( + Files.exists(dir.resolve("test-model.onnx")), "no model on an aborted download"); + assertFalse( + Files.exists(dir.resolve("test-model.onnx.tmp")), "partial download is deleted"); + assertEquals(0, m.status().getProgress(), "progress resets on failure"); + assertTrue( + m.status().getError().contains("exceeded the expected size"), + "the size ceiling, not the checksum, is what stopped it"); + } + + @Test + void doesNotEnableTheEndpointWhenTheOnnxEngineIsAbsent(@TempDir Path dir) throws Exception { + EndpointConfiguration ep = Mockito.mock(EndpointConfiguration.class); + FormDetectionModelManager m = + managerWithoutEngine(dir, entry(ALLOWED_URL + "/model.onnx", modelSha), ep); + + m.startInstall("test-model"); + awaitState(m, "ready", 5000); + + Mockito.verify(ep, Mockito.never()).enableEndpoint("form-detection"); + Mockito.verify(ep, Mockito.atLeastOnce()) + .disableEndpoint("form-detection", DisableReason.DEPENDENCY); + } + @Test void secondConcurrentInstallIsRejected(@TempDir Path dir) throws Exception { CountDownLatch gate = new CountDownLatch(1); diff --git a/frontend/editor/src/core/hooks/useFormDetectionModelStatus.test.tsx b/frontend/editor/src/core/hooks/useFormDetectionModelStatus.test.tsx new file mode 100644 index 0000000000..54570d6f30 --- /dev/null +++ b/frontend/editor/src/core/hooks/useFormDetectionModelStatus.test.tsx @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, vi, type Mock } from "vitest"; +import { act, render, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("@app/services/apiClient", () => ({ + default: { get: vi.fn(), post: vi.fn(), delete: vi.fn() }, +})); + +import apiClient from "@app/services/apiClient"; +import { qk } from "@app/query/keys"; +import { + useFormDetectionModelStatus, + type FormDetectionModelStatus, +} from "@app/hooks/useFormDetectionModelStatus"; + +function statusPayload(enabled: boolean): FormDetectionModelStatus { + return { + status: "ready", + progress: 100, + activeModelId: "test-model", + installed: ["test-model"], + error: null, + writable: true, + catalog: [], + enabled, + serverEngineAvailable: true, + }; +} + +let hook: ReturnType | null = null; + +function Probe() { + hook = useFormDetectionModelStatus(); + return ( + + {hook.status ? String(hook.status.enabled) : "loading"} + + ); +} + +describe("useFormDetectionModelStatus", () => { + beforeEach(() => { + hook = null; + (apiClient.get as Mock).mockReset(); + (apiClient.post as Mock).mockReset().mockResolvedValue({ data: {} }); + }); + + it("refreshes the tool availability cache when the master switch flips", async () => { + // The endpoint is re-gated server-side on toggle without the wire status changing, + // so an effect keyed only on status would leave the tile clickable until a reload. + (apiClient.get as Mock).mockResolvedValue({ data: statusPayload(true) }); + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const invalidate = vi.spyOn(client, "invalidateQueries"); + + const el = render( + + + , + ).getByTestId("enabled"); + await waitFor(() => expect(el.textContent).toBe("true")); + + invalidate.mockClear(); + (apiClient.get as Mock).mockResolvedValue({ data: statusPayload(false) }); + await act(async () => { + await hook!.setConfig({ enabled: false }); + }); + + await waitFor(() => expect(el.textContent).toBe("false")); + expect(invalidate).toHaveBeenCalledWith({ + queryKey: qk.endpointsAvailability(), + }); + expect(invalidate).toHaveBeenCalledWith({ + queryKey: qk.endpointEnabled("form-detection"), + }); + }); +}); diff --git a/frontend/editor/src/core/hooks/useFormDetectionModelStatus.ts b/frontend/editor/src/core/hooks/useFormDetectionModelStatus.ts index 563d525045..8817d6143b 100644 --- a/frontend/editor/src/core/hooks/useFormDetectionModelStatus.ts +++ b/frontend/editor/src/core/hooks/useFormDetectionModelStatus.ts @@ -65,6 +65,7 @@ export function useFormDetectionModelStatus() { }, [fetchStatus]); const active = status?.status; + const featureEnabled = status?.enabled; // Poll only while an install is in flight. useEffect(() => { @@ -75,7 +76,8 @@ export function useFormDetectionModelStatus() { return undefined; }, [active, fetchStatus]); - // When readiness flips, the tool availability cache must be refreshed. + // Readiness and the master switch both gate the endpoint, so either flipping must refresh + // the tool availability cache. useEffect(() => { if (active === "ready" || active === "not_installed") { void queryClient.invalidateQueries({ @@ -85,7 +87,7 @@ export function useFormDetectionModelStatus() { queryKey: qk.endpointEnabled("form-detection"), }); } - }, [active, queryClient]); + }, [active, featureEnabled, queryClient]); const install = useCallback( async (modelId: string) => { diff --git a/frontend/editor/src/desktop/hooks/useEndpointConfig.ts b/frontend/editor/src/desktop/hooks/useEndpointConfig.ts index 8b4d6d6e03..490a793df0 100644 --- a/frontend/editor/src/desktop/hooks/useEndpointConfig.ts +++ b/frontend/editor/src/desktop/hooks/useEndpointConfig.ts @@ -483,6 +483,3 @@ export function useEndpointConfig(): EndpointConfig { return { backendUrl }; } - -// Desktop endpoint config holds no module-level cache; nothing to invalidate. -export function invalidateEndpointCache() {} diff --git a/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx b/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx index adb89d2db5..ed14f93b64 100644 --- a/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx +++ b/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx @@ -271,7 +271,12 @@ export default function AdminFormDetectionSection() { size="sm" variant={isInstalled ? "secondary" : "primary"} loading={isBusy || isDownloading} - disabled={!installable || inFlight || !enabled} + disabled={ + !installable || + inFlight || + !enabled || + !serverEngineAvailable + } onClick={() => doInstall(entry.id)} > {isInstalled @@ -365,7 +370,7 @@ export default function AdminFormDetectionSection() {