[SUREFIRE-2076] BufferOverflowException when encoding message with nu…

…ll runMode (#529)

[SUREFIRE-2076] BufferOverflowException when encoding message with null runMode

* [SUREFIRE-2076] BufferOverflowException when encoding message with null runMode

* [SUREFIRE-2076] Null smartStackTrace not included in buffer length estimation in consoleErrorLog

Can potentially lead to BufferOverflowException with underestimated buffer length

surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamEncoder.java

@@ -181,11 +181,8 @@ public int estimateBufferLength( int opcodeLength, RunMode runMode, CharsetEncod
         // one byte + one delimiter character ':' + <string> + one delimiter character ':'
         int lengthOfMetadata = 1 + getEncodedMagicNumber().length + 1 + 1 + 1 + opcodeLength + 1;
 
-        if ( runMode != null )
-        {
-            // one byte of length + one delimiter character ':' + <string> + one delimiter character ':'
-            lengthOfMetadata += 1 + 1 + runMode.getRunmode().length() + 1;
-        }
+        // one byte of length + one delimiter character ':' + <string> + one delimiter character ':'
+        lengthOfMetadata += 1 + 1 + ( runMode == null ? 0 : runMode.getRunmodeBinary().length ) + 1;
 
         if ( encoder != null )
         {

surefire-api/src/test/java/org/apache/maven/surefire/api/stream/AbstractStreamEncoderTest.java

@@ -134,45 +134,45 @@ public void shouldComputeStreamPreemptiveLength()
             NORMAL_RUN, encoder, 0, 1, "s" ) )
             .isEqualTo( 88 );
 
-        // :maven-surefire-event:16:console-info-log:5:UTF-8:0003:sss:
+        // :maven-surefire-event:16:console-info-log:0::5:UTF-8:0001:s:
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_CONSOLE_INFO.getOpcodeBinary().length,
             null, encoder, 0, 0, "s" ) )
-            .isEqualTo( 58 );
+            .isEqualTo( 61 );
 
-        // :maven-surefire-event:17:console-debug-log:5:UTF-8:0003:sss:
+        // :maven-surefire-event:17:console-debug-log:0::5:UTF-8:0001:s:
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_CONSOLE_DEBUG.getOpcodeBinary().length,
             null, encoder, 0, 0, "s" ) )
-            .isEqualTo( 59 );
+            .isEqualTo( 62 );
 
-        // :maven-surefire-event:19:console-warning-log:5:UTF-8:0003:sss:
+        // :maven-surefire-event:19:console-warning-log:0::5:UTF-8:0001:s:
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_CONSOLE_WARNING.getOpcodeBinary().length,
             null, encoder, 0, 0, "s" ) )
-            .isEqualTo( 61 );
+            .isEqualTo( 64 );
 
-        // :maven-surefire-event:17:console-error-log:5:UTF-8:0003:sss:
+        // :maven-surefire-event:17:console-error-log:0::5:UTF-8:0001:s:
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_CONSOLE_ERROR.getOpcodeBinary().length,
             null, encoder, 0, 0, "s" ) )
-            .isEqualTo( 59 );
+            .isEqualTo( 62 );
 
-        // :maven-surefire-event:3:bye:
+        // :maven-surefire-event:3:bye:0::
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_BYE.getOpcodeBinary().length,
             null, null, 0, 0 ) )
-            .isEqualTo( 28 );
+            .isEqualTo( 31 );
 
-        // :maven-surefire-event:17:stop-on-next-test:
+        // :maven-surefire-event:17:stop-on-next-test:0::
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_STOP_ON_NEXT_TEST.getOpcodeBinary().length,
             null, null, 0, 0 ) )
-            .isEqualTo( 42 );
+            .isEqualTo( 45 );
 
-        // :maven-surefire-event:9:next-test:
+        // :maven-surefire-event:9:next-test:0::
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_NEXT_TEST.getOpcodeBinary().length,
             null, null, 0, 0 ) )
-            .isEqualTo( 34 );
+            .isEqualTo( 37 );
 
-        // :maven-surefire-event:14:jvm-exit-error:
+        // :maven-surefire-event:14:jvm-exit-error:0::
         assertThat( streamEncoder.estimateBufferLength( BOOTERCODE_JVM_EXIT_ERROR.getOpcodeBinary().length,
             null, null, 0, 0 ) )
-            .isEqualTo( 39 );
+            .isEqualTo( 42 );
     }
 
     @Test

surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java

@@ -214,7 +214,7 @@ public void consoleErrorLog( String message, Throwable t )
         CharsetEncoder encoder = newCharsetEncoder();
         String stackTrace = t == null ? null : ConsoleLoggerUtils.toString( t );
         int bufferMaxLength = estimateBufferLength( BOOTERCODE_CONSOLE_ERROR.getOpcode().length(), null, encoder, 0, 0,
-            message, stackTrace );
+            message, null, stackTrace );
         ByteBuffer result = ByteBuffer.allocate( bufferMaxLength );
         encodeHeader( result, BOOTERCODE_CONSOLE_ERROR );
         encodeCharset( result );

surefire-booter/src/test/java/org/apache/maven/surefire/booter/spi/EventChannelEncoderTest.java

@@ -1251,6 +1251,25 @@ public void testStdErrStreamEmptyMessageNullTestId() throws IOException
             .isEqualTo( expected );
     }
 
+    @Test
+    public void testStdErrStreamEmptyMessageNullRunMode() throws IOException
+    {
+        Stream out = Stream.newStream();
+        WritableBufferedByteChannel channel = newBufferedChannel( out );
+        EventChannelEncoder encoder = new EventChannelEncoder( channel );
+
+        // This used to produce a BufferOverflowException; see SUREFIRE-2076.
+        encoder.testOutput( new TestOutputReportEntry( stdErr( "" ), null, 1L ) );
+        channel.close();
+
+        String expected = ":maven-surefire-event:\u000e:std-err-stream:"
+            + (char) 0 + "::" // One byte for length and 1+1 bytes for the 2 delimiters (0 bytes for null runMode)
+            + "\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001:"
+            + "\u0005:UTF-8:\u0000\u0000\u0000\u0000::";
+
+        assertThat( new String( out.toByteArray(), UTF_8 ) )
+            .isEqualTo( expected );
+    }
 
     @Test
     @SuppressWarnings( "checkstyle:innerassignment" )