fix(signing): stream the upload instead of buffering it whole

This commit is contained in:
Anthony Stirling
2026-08-21 09:14:06 +01:00
parent 639f669b73
commit 950a6111bc
2 changed files with 78 additions and 10 deletions
@@ -2,7 +2,9 @@ package stirling.software.SPDF.controller.api.security;
import java.beans.PropertyEditorSupport;
import java.io.ByteArrayInputStream;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.PKIXCertPathBuilderResult;
@@ -56,6 +58,8 @@ import stirling.software.common.util.ExceptionUtils;
@RequiredArgsConstructor
public class ValidateSignatureController {
private static final int SKIP_BUFFER_SIZE = 8192;
private final CustomPDFDocumentFactory pdfDocumentFactory;
private final CertificateValidationService certValidationService;
@@ -107,12 +111,7 @@ public class ValidateSignatureController {
}
}
// Read the upload once. PDFBox walks the /ByteRange by skipping the signature hole, and
// InputStream.skip() is allowed to return 0 - servlet part streams do, which failed
// validation on documents other verifiers accept.
byte[] pdfBytes = file.getBytes();
try (PDDocument document = pdfDocumentFactory.load(new ByteArrayInputStream(pdfBytes))) {
try (PDDocument document = pdfDocumentFactory.load(file.getInputStream())) {
List<PDSignature> signatures = document.getSignatureDictionaries();
// Detect content appended outside every signature's ByteRange (added after signing). A
@@ -120,7 +119,7 @@ public class ValidateSignatureController {
// furthest any signature reaches stops short of the file length, the tail is unsigned.
// Taking the max across all signatures avoids false positives on legitimately
// multi-signed PDFs, where an earlier signature intentionally omits later revisions.
long fileLength = pdfBytes.length;
long fileLength = file.getSize();
long maxCovered = 0;
for (PDSignature sig : signatures) {
int[] byteRange = sig.getByteRange();
@@ -138,8 +137,13 @@ public class ValidateSignatureController {
result.setCoversEntireDocument(documentCovered);
try {
byte[] signedContent = sig.getSignedContent(pdfBytes);
byte[] signatureBytes = sig.getContents(pdfBytes);
byte[] signedContent;
byte[] signatureBytes;
try (InputStream contentStream = skipByReading(file.getInputStream());
InputStream signatureStream = skipByReading(file.getInputStream())) {
signedContent = sig.getSignedContent(contentStream);
signatureBytes = sig.getContents(signatureStream);
}
// An RFC 3161 document timestamp (PAdES-LTV) carries its signed content
// *inside* the CMS - a TSTInfo - rather than being detached over the document.
@@ -357,6 +361,33 @@ public class ValidateSignatureController {
return ResponseEntity.ok(results);
}
/**
* Wrap a stream so {@code skip()} always makes progress.
*
* <p>PDFBox walks a signature's /ByteRange by skipping the hole where /Contents sits, and
* treats a {@code skip()} of 0 as a hard failure. Returning 0 is legal - servlet container part
* streams do it - so signatures on perfectly good documents were rejected with
* "FilterInputStream.skip() returns 0". Reading and discarding keeps this streaming: the upload
* is never held in memory in full.
*/
private static InputStream skipByReading(InputStream source) {
return new FilterInputStream(source) {
@Override
public long skip(long n) throws IOException {
long remaining = n;
byte[] scratch = new byte[SKIP_BUFFER_SIZE];
while (remaining > 0) {
int read = in.read(scratch, 0, (int) Math.min(scratch.length, remaining));
if (read < 0) {
break;
}
remaining -= read;
}
return n - remaining;
}
};
}
/**
* True when the timestamp token was issued over exactly these bytes.
*
@@ -24,7 +24,8 @@ import stirling.software.common.model.ApplicationProperties;
import stirling.software.common.service.CustomPDFDocumentFactory;
/**
* Signature validation must not depend on the upload's stream supporting skip().
* Signature validation must not depend on the upload's stream supporting skip(), and must not buy
* that independence by loading the whole upload into memory.
*
* <p>InputStream.skip() is allowed to return 0, and servlet container part streams do. PDFBox walks
* the /ByteRange by skipping the signature hole, so reading the signed content straight off the
@@ -87,6 +88,18 @@ class ValidateSignatureStreamHandlingTest {
assertThat(actual.getSubjectDN()).isEqualTo(expected.getSubjectDN());
}
@Test
@DisplayName("Never materialises the whole upload in memory")
void streamsTheUploadRatherThanBufferingIt() throws Exception {
SignatureValidationRequest request = new SignatureValidationRequest();
request.setFileInput(new StreamOnlyMultipartFile(signedPdf));
List<SignatureValidationResult> results = controller.validateSignature(request).getBody();
assertThat(results).hasSize(1);
assertThat(results.get(0).isValid()).isTrue();
}
/** Upload whose stream honours the InputStream contract that skip() may return 0. */
private static final class NonSkippingMultipartFile extends MockMultipartFile {
@@ -107,4 +120,28 @@ class ValidateSignatureStreamHandlingTest {
};
}
}
/**
* Upload that refuses to hand over its bytes in one piece. PDFBox already materialises the
* signed content, so buffering the upload on top of that doubles peak memory on large files.
*/
private static final class StreamOnlyMultipartFile extends MockMultipartFile {
private final byte[] content;
StreamOnlyMultipartFile(byte[] content) {
super("fileInput", "doc.pdf", "application/pdf", content);
this.content = content;
}
@Override
public byte[] getBytes() {
throw new AssertionError("validation must stream the upload, not buffer it whole");
}
@Override
public InputStream getInputStream() {
return new ByteArrayInputStream(content);
}
}
}