Handle some IllegalArgument cases in readBytes

src/main/cpp/_nix_based/jssc.cpp

@@ -26,6 +26,7 @@
 #include <limits.h>
 #include <stdio.h>
 #include <fcntl.h>
+#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <string.h>
@@ -657,10 +658,27 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes
   (JNIEnv *env, jobject, jlong portHandle, jint byteCount){
 
     int err;
-    jbyte *lpBuffer = new jbyte[byteCount];
+    jbyte *lpBuffer = NULL;
     jbyteArray returnArray = NULL;
     int byteRemains = byteCount;
 
+    if( byteCount <= 0 ){
+        char emsg[64]; emsg[0] = '\0';
+        snprintf(emsg, sizeof emsg, "byteCount %d. Expected range: 1..2147483647", byteCount);
+        jclass exClz = env->FindClass("java/lang/IllegalArgumentException");
+        if( exClz ) env->ThrowNew(exClz, emsg);
+        returnArray = NULL; goto Finally;
+    }
+
+    lpBuffer = (jbyte*)malloc(byteCount*sizeof*lpBuffer);
+    if( !lpBuffer ){
+        char emsg[32]; emsg[0] = '\0';
+        snprintf(emsg, sizeof emsg, "malloc(%d) failed", byteCount*sizeof*lpBuffer);
+        jclass exClz = env->FindClass("java/lang/RuntimeException");
+        if( exClz ) env->ThrowNew(exClz, emsg);
+        returnArray = NULL; goto Finally;
+    }
+
     while(byteRemains > 0) {
         int result = 0;
 
@@ -707,7 +725,7 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes
     assert(env->ExceptionCheck() == JNI_FALSE);
 
 Finally:
-    delete[] lpBuffer;
+    if( lpBuffer ) free(lpBuffer);
     return returnArray;
 }
 

src/main/cpp/windows/jssc.cpp

@@ -276,17 +276,31 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes
     HANDLE hComm = (HANDLE)portHandle;
     DWORD lpNumberOfBytesTransferred;
     DWORD lpNumberOfBytesRead;
+    jbyteArray returnArray = NULL;
     jbyte *lpBuffer = NULL;
+    OVERLAPPED *overlapped = NULL;
 
-    jbyteArray returnArray = env->NewByteArray(byteCount);
+    if( byteCount <= 0 ){
+        char emsg[64]; emsg[0] = '\0';
+        snprintf(emsg, sizeof emsg, "byteCount %d. Expected range: 1..2147483647", byteCount);
+        jclass exClz = env->FindClass("java/lang/IllegalArgumentException");
+        if( exClz ) env->ThrowNew(exClz, emsg);
+        returnArray = NULL; goto Finally;
+    }
 
-    lpBuffer = (jbyte *)malloc(byteCount * sizeof(jbyte));
-    if(lpBuffer == NULL){
-        // return an empty array
-        return returnArray;
+    returnArray = env->NewByteArray(byteCount);
+    if( returnArray == NULL ) goto Finally;
+
+    lpBuffer = (jbyte*)malloc(byteCount*sizeof*lpBuffer);
+    if( !lpBuffer ){
+        char emsg[32]; emsg[0] = '\0';
+        snprintf(emsg, sizeof emsg, "malloc(%d) failed", byteCount*sizeof*lpBuffer);
+        jclass exClz = env->FindClass("java/lang/RuntimeException");
+        if( exClz ) env->ThrowNew(exClz, emsg);
+        returnArray = NULL; goto Finally;
     }
 
-    OVERLAPPED *overlapped = new OVERLAPPED();
+    overlapped = new OVERLAPPED();
     overlapped->hEvent = CreateEventA(NULL, true, false, NULL);
     if(ReadFile(hComm, lpBuffer, (DWORD)byteCount, &lpNumberOfBytesRead, overlapped)){
         env->SetByteArrayRegion(returnArray, 0, byteCount, lpBuffer);
@@ -302,9 +316,12 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes
         jclass exClz = env->FindClass("java/lang/IllegalArgumentException");
         if( exClz != NULL ) env->ThrowNew(exClz, "EBADF");
     }
-    CloseHandle(overlapped->hEvent);
-    delete overlapped;
-    free(lpBuffer);
+Finally:
+    if( overlapped ){
+        CloseHandle(overlapped->hEvent);
+        delete overlapped;
+    }
+    if( lpBuffer ) free(lpBuffer);
     return returnArray;
 }
 

src/test/java/jssc/SerialNativeInterfaceTest.java

@@ -4,6 +4,7 @@
 import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
+import org.slf4j.Logger;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
@@ -13,9 +14,12 @@
 import static org.junit.Assert.fail;
 import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
+import static org.slf4j.LoggerFactory.getLogger;;
 
 public class SerialNativeInterfaceTest extends DisplayMethodNameRule {
 
+    private static final Logger log = getLogger(SerialNativeInterfaceTest.class);
+
     @Test
     public void testInitNativeInterface() {
         SerialNativeInterface serial = new SerialNativeInterface();
@@ -94,4 +98,48 @@ public void throwsNpeIfPassedBufferIsNull() throws Exception {
         new SerialNativeInterface().writeBytes(1, null);
     }
 
+    @Test
+    public void throwsIfCountNegative() throws Exception {
+        SerialNativeInterface testTarget = new SerialNativeInterface();
+        byte[] ret;
+        try{
+            ret = testTarget.readBytes(0, -42);
+            fail("Where's the exception?");
+        }catch( IllegalArgumentException ex ){
+            assertTrue(ex.getMessage().contains("-42"));
+        }
+    }
+
+    @Test
+    public void throwsIfCountZero() throws Exception {
+        SerialNativeInterface testTarget = new SerialNativeInterface();
+        byte[] ret;
+        try{
+            ret = testTarget.readBytes(0, 0);
+            fail("Where's the exception?");
+        }catch( IllegalArgumentException ex ){
+            assertTrue(ex.getMessage().contains("0"));
+        }
+    }
+
+    @Test
+    @org.junit.Ignore("This test only makes sense if it is run in a situation"
+        +" where large memory allocations WILL fail (for example you could use"
+        +" a virtual machine with low memory available). Because on regular"
+        +" machines allocating 2GiB of RAM is not a problem at all and so the"
+        +" test run will just happily wait infinitely for those 2GiB to arrive"
+        +" at the stdin fd. Feel free to remove this test if you think it"
+        +" doesn't make sense to have it here.")
+    public void throwsIfRequestTooLarge() throws Exception {
+        SerialNativeInterface testTarget = new SerialNativeInterface();
+        int tooLargeSize = Integer.MAX_VALUE;
+        try{
+            byte[] ret = testTarget.readBytes(0, tooLargeSize);
+            fail("Where's the exception?");
+        }catch( RuntimeException ex ){
+            log.debug("Thrown, as expected :)", ex);
+            assertTrue(ex.getMessage().contains(String.valueOf(tooLargeSize)));
+        }
+    }
+
 }

src/test/resources/log4j2.properties

@@ -1,7 +1,7 @@
 # Logging settings for unit tests
 
 # The root logger with appender name
-rootLogger=DEBUG,STDOUT,TESTNAME
+rootLogger=INFO,STDOUT,TESTNAME
 
 # Assign STDOUT a valid appender & define its layout
 appender.console.name=STDOUT
@@ -19,4 +19,8 @@ appender.testName.filter.1.marker=MethodName
 appender.console.filter.1.type=MarkerFilter
 appender.console.filter.1.marker=MethodName
 appender.console.filter.1.onMatch=DENY
-appender.console.filter.1.onMismatch=ACCEPT
+appender.console.filter.1.onMismatch=ACCEPT
+
+# Configures log-level of 'jssc' java package (Handy for debugging tests).
+#logger.jssc.name = jssc
+#logger.jssc.level = DEBUG