8218280: LineNumberReader throws "Mark invalid" exception if CRLF straddles buffer.

Daniel Fuchs daniel.fuchs at oracle.com
Fri Apr 26 18:45:40 UTC 2019


Looks good Brian!

nit:

 > +            reader.reset(); // reset to position after '\n'

actually it's 'after '\r'' now.

No need for new review!

best regards,

-- daniel

On 26/04/19 19:27, Brian Burkhalter wrote:
> Daniel,
> 
> I modified the test as shown below. It passes both before and after the 
> implementation change.
> 
> Thanks,
> 
> Brian
> 
> --- a/test/jdk/java/io/LineNumberReader/MarkSplitCRLF.java
> +++ b/test/jdk/java/io/LineNumberReader/MarkSplitCRLF.java
> @@ -56,6 +56,24 @@
>       }
> 
> 
>       @Test
> +    public static void testCRNotFollowedByLF() throws IOException {
> +        final String string = "foo\rbar";
> +        try (Reader reader =
> +            new LineNumberReader(new StringReader(string), 5)) {
> +            reader.read();  // 'f'
> +            reader.read();  // 'o'
> +            reader.read();  // 'o'
> +            reader.read();  // '\r'
> +            reader.mark(1); // mark position of next character
> +            reader.read();  // 'b'
> +            reader.reset(); // reset to position after '\n'
> +            assertEquals(reader.read(), 'b');
> +            assertEquals(reader.read(), 'a');
> +            assertEquals(reader.read(), 'r');
> +        }
> +    }
> +
> +    @Test
> 
>> On Apr 26, 2019, at 4:23 AM, Daniel Fuchs <daniel.fuchs at oracle.com 
>> <mailto:daniel.fuchs at oracle.com>> wrote:
>>
>> I had to convince myself that there was no issue when
>> '\r' is not followed by '\n' but I couldn't fault  the
>> logic.
>>
>> I wonder if adding a third test case with '\r' not
>> followed by '\n' would be a good idea?
> 



More information about the core-libs-dev mailing list