Skip to content

Commit

Permalink
[SUREFIRE-2058] Corrupted STDOUT by directly writing to native stream…
Browse files Browse the repository at this point in the history 
… in forked JVM 1 with UTF-8 console logging (#518)

[SUREFIRE-2058] Corrupted STDOUT by directly writing to native stream in forked JVM 1 with UTF-8 console logging
* [SUREFIRE-2058] Add readString unit test covering cases with overflowing output buffer

- shouldReadStringOverflowOnNewLine - ends up with 1 single byte (LF) remaining on input buffer
- shouldReadStringOverflowOn4BytesEncodedSymbol - causing an infinite loop with 4 bytes left on input buffer

* [SUREFIRE-2058] Flip and clear output char buffer after each chunk read

Overflow can happen even when output buffer has still some remaining space left

* [SUREFIRE-2058] Add static import for emptyMap and remove explicit type arguments
  • Loading branch information
@zoltanmeze
zoltanmeze authored Apr 26, 2022
1 parent 2657a32 commit 754dd9c
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 43 deletions.
7 changes: 2 additions & 5 deletions surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,8 @@ private String readString( @Nonnull final Memento memento, @Nonnegative final in
}
while ( isLastChunk && bytesToDecode > 0 && output.hasRemaining() );

if ( isLastChunk || !output.hasRemaining() )
{
strings.add( ( (Buffer) output ).flip().toString() );
( (Buffer) output ).clear();
}
strings.add( ( (Buffer) output ).flip().toString() );
( (Buffer) output ).clear();
}

memento.getDecoder().reset();
Expand Down
106 changes: 68 additions & 38 deletions ...ire-api/src/test/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.nio.CharBuffer;
import java.nio.channels.ReadableByteChannel;
import java.nio.charset.CharsetDecoder;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
Expand All @@ -51,6 +50,7 @@
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.nio.charset.StandardCharsets.US_ASCII;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static org.apache.maven.surefire.api.booter.Constants.DEFAULT_STREAM_ENCODING;
import static org.apache.maven.surefire.api.booter.ForkedProcessEventType.BOOTERCODE_STDOUT;
Expand Down Expand Up @@ -196,8 +196,7 @@ public void shouldReadInt() throws Exception
{
Channel channel = new Channel( new byte[] {0x01, 0x02, 0x03, 0x04, ':'}, 1 );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();

Expand All @@ -210,8 +209,7 @@ public void shouldReadInteger() throws Exception
{
Channel channel = new Channel( new byte[] {(byte) 0xff, 0x01, 0x02, 0x03, 0x04, ':'}, 1 );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
assertThat( thread.readInteger( memento ) )
Expand All @@ -223,8 +221,7 @@ public void shouldReadNullInteger() throws Exception
{
Channel channel = new Channel( new byte[] {(byte) 0x00, ':'}, 1 );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
assertThat( thread.readInteger( memento ) )
Expand All @@ -237,8 +234,7 @@ public void shouldNotReadString() throws Exception
Channel channel = new Channel( PATTERN1.getBytes(), PATTERN1.length() );
channel.read( ByteBuffer.allocate( 100 ) );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
invokeMethod( thread, "readString", memento, 10 );
Expand All @@ -249,15 +245,62 @@ public void shouldReadString() throws Exception
{
Channel channel = new Channel( PATTERN1.getBytes(), PATTERN1.length() );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
String s = invokeMethod( thread, "readString", memento, 10 );
assertThat( s )
.isEqualTo( "0123456789" );
}

@Test
public void shouldReadStringOverflowOnNewLine() throws Exception
{
StringBuilder s = new StringBuilder( 1025 );
for ( int i = 0; i < 10; i++ )
{
s.append( PATTERN1 );
}
s.append( PATTERN1, 0, 23 );
s.append( "\u00FA\n" ); // 2-bytes encoded character + LF

Channel channel = new Channel( s.toString().getBytes( UTF_8 ), s.length() );

Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();

assertThat( (String) invokeMethod( thread, "readString", memento, 1026 ) )
.isEqualTo( s.toString() );

assertThat ( memento.getByteBuffer().remaining() )
.isEqualTo( 0 );
}

@Test
public void shouldReadStringOverflowOn4BytesEncodedSymbol() throws Exception
{
StringBuilder s = new StringBuilder( 1025 );
for ( int i = 0; i < 10; i++ )
{
s.append( PATTERN1 );
}
s.append( PATTERN1, 0, 23 );
s.append( "\uD83D\uDE35" ); // 4-bytes encoded character

Channel channel = new Channel( s.toString().getBytes( UTF_8 ), s.length() );

Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();

assertThat( (String) invokeMethod( thread, "readString", memento, 1027 ) )
.isEqualTo( s.toString() );

assertThat ( memento.getByteBuffer().remaining() )
.isEqualTo( 0 );
}

@Test
public void shouldReadStringShiftedBuffer() throws Exception
{
Expand All @@ -269,8 +312,7 @@ public void shouldReadStringShiftedBuffer() throws Exception

Channel channel = new Channel( s.toString().getBytes( UTF_8 ), s.length() );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
// whatever position will be compacted to 0
Expand All @@ -291,8 +333,7 @@ public void shouldReadStringShiftedInput() throws Exception
Channel channel = new Channel( s.toString().getBytes( UTF_8 ), s.length() );
channel.read( ByteBuffer.allocate( 997 ) );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
assertThat( (String) invokeMethod( thread, "readString", memento, PATTERN1.length() ) )
Expand All @@ -312,8 +353,7 @@ public void shouldReadMultipleStringsAndShiftedInput() throws Exception
Channel channel = new Channel( s.toString().getBytes( UTF_8 ), s.length() );
channel.read( ByteBuffer.allocate( 1997 ) );

Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
// whatever position will be compacted to 0
Expand Down Expand Up @@ -341,8 +381,7 @@ public void shouldDecode3BytesEncodedSymbol() throws Exception
}

Channel channel = new Channel( input, 64 * 1024 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );
Memento memento = thread.new Memento();
String decodedOutput = invokeMethod( thread, "readString", memento, input.length );

Expand Down Expand Up @@ -416,8 +455,7 @@ public void shouldEventTypeReachedMalformedHeader() throws Exception
{
byte[] stream = ":xxxxx-xxxxxxxx-xxxxx:\u000E:xxx".getBytes( UTF_8 );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -429,8 +467,7 @@ public void shouldReadEmptyString() throws Exception
{
byte[] stream = "\u0000\u0000\u0000\u0000::".getBytes( UTF_8 );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -444,8 +481,7 @@ public void shouldReadNullString() throws Exception
{
byte[] stream = "\u0000\u0000\u0000\u0001:\u0000:".getBytes( UTF_8 );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -459,8 +495,7 @@ public void shouldReadSingleCharString() throws Exception
{
byte[] stream = "\u0000\u0000\u0000\u0001:A:".getBytes( UTF_8 );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -474,8 +509,7 @@ public void shouldReadThreeCharactersString() throws Exception
{
byte[] stream = "\u0000\u0000\u0000\u0003:ABC:".getBytes( UTF_8 );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -489,8 +523,7 @@ public void shouldReadDefaultCharset() throws Exception
{
byte[] stream = "\u0005:UTF-8:".getBytes( US_ASCII );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -505,8 +538,7 @@ public void shouldReadNonDefaultCharset() throws Exception
{
byte[] stream = ( (char) 10 + ":ISO_8859_1:" ).getBytes( US_ASCII );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand All @@ -521,10 +553,9 @@ public void shouldSetNonDefaultCharset()
{
byte[] stream = {};
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Memento memento = thread.new Memento();
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( ISO_8859_1 );
assertThat( memento.getDecoder().charset() ).isEqualTo( ISO_8859_1 );

Expand All @@ -541,8 +572,7 @@ public void malformedCharset() throws Exception
{
byte[] stream = ( (char) 8 + ":ISO_8859:" ).getBytes( US_ASCII );
Channel channel = new Channel( stream, 1 );
Mock thread = new Mock( channel, new MockForkNodeArguments(),
Collections.<Segment, ForkedProcessEventType>emptyMap() );
Mock thread = new Mock( channel, new MockForkNodeArguments(), emptyMap() );

Memento memento = thread.new Memento();
memento.setCharset( UTF_8 );
Expand Down

0 comments on commit 754dd9c

Please sign in to comment.