Avoid NPE when request body is empty

dnl50 · Nov 30, 2024 · 820b530 · 820b530
1 parent cc5da42
commit 820b530
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 70 deletions.
diff --git a/app/src/main/java/dev/mieser/tsa/signing/api/exception/InvalidTspRequestException.java b/app/src/main/java/dev/mieser/tsa/signing/api/exception/InvalidTspRequestException.java
@@ -6,4 +6,8 @@ public InvalidTspRequestException(Throwable cause) {
         super(cause);
     }
 
+    public InvalidTspRequestException(String message) {
+        super(message);
+    }
+
 }
diff --git a/app/src/main/java/dev/mieser/tsa/signing/impl/TspParser.java b/app/src/main/java/dev/mieser/tsa/signing/impl/TspParser.java
@@ -1,9 +1,11 @@
 package dev.mieser.tsa.signing.impl;
 
+import java.io.IOException;
 import java.io.InputStream;
+import java.util.Optional;
 
-import org.apache.commons.io.input.CloseShieldInputStream;
-import org.bouncycastle.asn1.ASN1InputStream;
+import org.apache.commons.lang3.ArrayUtils;
+import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.tsp.TimeStampReq;
 import org.bouncycastle.asn1.tsp.TimeStampResp;
 import org.bouncycastle.tsp.TimeStampRequest;
@@ -23,12 +25,14 @@ public class TspParser {
      *     closed.
      * @return The parsed TSP request.
      * @throws InvalidTspRequestException
-     *     When the input stream cannot be parsed to an TSP request.
+     *     When the input stream cannot be parsed as a TSP request.
      */
     public TimeStampRequest parseRequest(InputStream requestInputStream) throws InvalidTspRequestException {
-        try (ASN1InputStream asnInputStream = new ASN1InputStream(CloseShieldInputStream.wrap(requestInputStream))) {
-            TimeStampReq timeStampReq = TimeStampReq.getInstance(asnInputStream.readObject());
+        byte[] asn1EncodedTspRequest = readStream(requestInputStream)
+            .orElseThrow(() -> new InvalidTspRequestException("TSP request data is missing"));
 
+        try {
+            TimeStampReq timeStampReq = TimeStampReq.getInstance(ASN1Sequence.fromByteArray(asn1EncodedTspRequest));
             return new TimeStampRequest(timeStampReq);
         } catch (Exception e) {
             throw new InvalidTspRequestException(e);
@@ -41,16 +45,31 @@ public TimeStampRequest parseRequest(InputStream requestInputStream) throws Inva
      *     closed.
      * @return The parsed TSP response.
      * @throws InvalidTspResponseException
-     *     When the input stream cannot be parsed to an TSP response.
+     *     When the input stream cannot be parsed as a TSP response.
      */
     public TimeStampResponse parseResponse(InputStream inputStream) throws InvalidTspResponseException {
-        try (ASN1InputStream asnInputStream = new ASN1InputStream(CloseShieldInputStream.wrap(inputStream))) {
-            TimeStampResp timeStampResp = TimeStampResp.getInstance(asnInputStream.readObject());
+        byte[] asn1EncodedTspResponse = readStream(inputStream)
+            .orElseThrow(() -> new InvalidTspResponseException("TSP response data is missing"));
 
+        try {
+            TimeStampResp timeStampResp = TimeStampResp.getInstance(ASN1Sequence.fromByteArray(asn1EncodedTspResponse));
             return new TimeStampResponse(timeStampResp);
         } catch (Exception e) {
             throw new InvalidTspResponseException("Could not parse TSP response", e);
         }
     }
 
+    private Optional<byte[]> readStream(InputStream stream) {
+        try {
+            byte[] content = stream.readAllBytes();
+            if (ArrayUtils.isEmpty(content)) {
+                return Optional.empty();
+            }
+
+            return Optional.of(content);
+        } catch (IOException e) {
+            throw new IllegalStateException("Failed to read stream", e);
+        }
+    }
+
 }
diff --git a/app/src/test/java/dev/mieser/tsa/signing/impl/TspParserTest.java b/app/src/test/java/dev/mieser/tsa/signing/impl/TspParserTest.java
@@ -11,6 +11,8 @@
 import java.io.InputStream;
 import java.math.BigInteger;
 
+import lombok.Getter;
+
 import org.bouncycastle.asn1.ASN1Boolean;
 import org.bouncycastle.asn1.ASN1Integer;
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
@@ -19,6 +21,7 @@
 import org.bouncycastle.asn1.x509.AlgorithmIdentifier;
 import org.bouncycastle.tsp.TimeStampRequest;
 import org.bouncycastle.tsp.TimeStampResponse;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
 import dev.mieser.tsa.signing.api.exception.InvalidTspRequestException;
@@ -28,80 +31,112 @@ class TspParserTest {
 
     private final TspParser testSubject = new TspParser();
 
-    @Test
-    void parseRequestThrowsExceptionWhenRequestCannotBeParsed() {
-        // given
-        byte[] invalidTspRequest = "tsp request".getBytes(UTF_8);
-        InputStream tspRequestInputStream = new ByteArrayInputStream(invalidTspRequest);
+    @Nested
+    class RequestParsing {
 
-        // when / then
-        assertThatExceptionOfType(InvalidTspRequestException.class)
-            .isThrownBy(() -> testSubject.parseRequest(tspRequestInputStream));
-    }
+        @Test
+        void throwsExceptionWhenRequestCannotBeParsed() {
+            // given
+            byte[] invalidTspRequest = "tsp request".getBytes(UTF_8);
+            InputStream tspRequestInputStream = new ByteArrayInputStream(invalidTspRequest);
 
-    @Test
-    void parseRequestReturnsExpectedRequest() throws Exception {
-        // given
-        TimeStampReq timeStampRequest = createTimeStampRequest();
-        InputStream tspRequestInputStream = new ByteArrayInputStream(timeStampRequest.getEncoded());
+            // when / then
+            assertThatExceptionOfType(InvalidTspRequestException.class)
+                .isThrownBy(() -> testSubject.parseRequest(tspRequestInputStream));
+        }
 
-        // when
-        TimeStampRequest parsedRequest = testSubject.parseRequest(tspRequestInputStream);
+        @Test
+        void throwsExceptionWhenStreamIsEmpty() {
+            // given
+            InputStream emptyStream = new ByteArrayInputStream(new byte[0]);
 
-        // then
-        assertThat(parsedRequest.getEncoded()).isEqualTo(timeStampRequest.getEncoded());
-    }
+            // when / then
+            assertThatExceptionOfType(InvalidTspRequestException.class)
+                .isThrownBy(() -> testSubject.parseRequest(emptyStream))
+                .withMessage("TSP request data is missing");
+        }
 
-    @Test
-    void parseRequestDoesNotCloseInputStream() throws Exception {
-        // given
-        TimeStampReq timeStampRequest = createTimeStampRequest();
-        CloseAwareInputStream tspRequestInputStream = new CloseAwareInputStream(
-            new ByteArrayInputStream(timeStampRequest.getEncoded()));
+        @Test
+        void returnsExpectedRequest() throws Exception {
+            // given
+            TimeStampReq timeStampRequest = createTimeStampRequest();
+            InputStream tspRequestInputStream = new ByteArrayInputStream(timeStampRequest.getEncoded());
 
-        // when
-        testSubject.parseRequest(tspRequestInputStream);
+            // when
+            TimeStampRequest parsedRequest = testSubject.parseRequest(tspRequestInputStream);
 
-        // then
-        assertThat(tspRequestInputStream.isClosed()).isFalse();
-    }
+            // then
+            assertThat(parsedRequest.getEncoded()).isEqualTo(timeStampRequest.getEncoded());
+        }
+
+        @Test
+        void doesNotCloseInputStream() throws Exception {
+            // given
+            TimeStampReq timeStampRequest = createTimeStampRequest();
+            CloseAwareInputStream tspRequestInputStream = new CloseAwareInputStream(
+                new ByteArrayInputStream(timeStampRequest.getEncoded()));
+
+            // when
+            testSubject.parseRequest(tspRequestInputStream);
 
-    @Test
-    void parseResponseThrowsExceptionWhenRequestCannotBeParsed() {
-        // given
-        byte[] invalidResponse = "tsp response".getBytes(UTF_8);
-        InputStream tspResponseInputStream = new ByteArrayInputStream(invalidResponse);
+            // then
+            assertThat(tspRequestInputStream.isClosed()).isFalse();
+        }
 
-        // when / then
-        assertThatExceptionOfType(InvalidTspResponseException.class)
-            .isThrownBy(() -> testSubject.parseResponse(tspResponseInputStream))
-            .withMessage("Could not parse TSP response");
     }
 
-    @Test
-    void parseResponseReturnsExpectedResponse() throws Exception {
-        // given
-        byte[] timeStampResponse = readAsnEncodedTimeStampResponse();
-        InputStream tspResponseInputStream = new ByteArrayInputStream(timeStampResponse);
+    @Nested
+    class ResponseParsing {
 
-        // when
-        TimeStampResponse parsedResponse = testSubject.parseResponse(tspResponseInputStream);
+        @Test
+        void throwsExceptionWhenRequestCannotBeParsed() {
+            // given
+            byte[] invalidResponse = "tsp response".getBytes(UTF_8);
+            InputStream tspResponseInputStream = new ByteArrayInputStream(invalidResponse);
 
-        // then
-        assertThat(parsedResponse.getEncoded()).isEqualTo(timeStampResponse);
-    }
+            // when / then
+            assertThatExceptionOfType(InvalidTspResponseException.class)
+                .isThrownBy(() -> testSubject.parseResponse(tspResponseInputStream))
+                .withMessage("Could not parse TSP response");
+        }
 
-    @Test
-    void parseResponseDoesNotCloseInputStream() throws Exception {
-        // given
-        byte[] timeStampResponse = readAsnEncodedTimeStampResponse();
-        CloseAwareInputStream tspResponseInputStream = new CloseAwareInputStream(new ByteArrayInputStream(timeStampResponse));
+        @Test
+        void throwsExceptionWhenStreamIsEmpty() {
+            // given
+            InputStream emptyStream = new ByteArrayInputStream(new byte[0]);
 
-        // when
-        testSubject.parseResponse(tspResponseInputStream);
+            // when / then
+            assertThatExceptionOfType(InvalidTspResponseException.class)
+                .isThrownBy(() -> testSubject.parseResponse(emptyStream))
+                .withMessage("TSP response data is missing");
+        }
+
+        @Test
+        void returnsExpectedResponse() throws Exception {
+            // given
+            byte[] timeStampResponse = readAsnEncodedTimeStampResponse();
+            InputStream tspResponseInputStream = new ByteArrayInputStream(timeStampResponse);
+
+            // when
+            TimeStampResponse parsedResponse = testSubject.parseResponse(tspResponseInputStream);
+
+            // then
+            assertThat(parsedResponse.getEncoded()).isEqualTo(timeStampResponse);
+        }
+
+        @Test
+        void doesNotCloseInputStream() throws Exception {
+            // given
+            byte[] timeStampResponse = readAsnEncodedTimeStampResponse();
+            CloseAwareInputStream tspResponseInputStream = new CloseAwareInputStream(new ByteArrayInputStream(timeStampResponse));
+
+            // when
+            testSubject.parseResponse(tspResponseInputStream);
+
+            // then
+            assertThat(tspResponseInputStream.isClosed()).isFalse();
+        }
 
-        // then
-        assertThat(tspResponseInputStream.isClosed()).isFalse();
     }
 
     private TimeStampReq createTimeStampRequest() {
@@ -123,6 +158,7 @@ private byte[] readAsnEncodedTimeStampResponse() throws IOException {
      * @implNote Spying an Input Stream causes the input stream to return -1 when calling the {@code read()} methods (Java
      * 17.0.1, Mockito 4.0.0).
      */
+    @Getter
     private static class CloseAwareInputStream extends FilterInputStream {
 
         private boolean closed;
@@ -137,10 +173,6 @@ public void close() throws IOException {
             this.closed = true;
         }
 
-        public boolean isClosed() {
-            return closed;
-        }
-
     }
 
 }