Skip to content

4.1.3: Fix DataReader.findNewLine with multiple lone EOL character #9391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
barchetta merged 1 commit into from
Oct 14, 2024
Merged

4.1.3: Fix DataReader.findNewLine with multiple lone EOL character #9391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 18 additions & 25 deletions common/buffers/src/main/java/io/helidon/common/buffers/DataReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,69 +395,62 @@ public String debugDataHex() {
*/
public int findNewLine(int max) throws IncorrectNewLineException {
ensureAvailable();
int idx = 0;
Node n = head;
int indexWithinNode = n.position;
int idx = 0;
int fromIndexNode = n.position;

while (true) {
byte[] barr = n.bytes;
int maxLength = Math.min(max - idx, barr.length - indexWithinNode);
int crIndex = Bytes.firstIndexOf(barr, indexWithinNode, indexWithinNode + maxLength, Bytes.CR_BYTE);
int maxLength = Math.min(max - idx, barr.length - fromIndexNode);
int crIndexNode = Bytes.firstIndexOf(barr, fromIndexNode, fromIndexNode + maxLength, Bytes.CR_BYTE);

if (crIndex == -1) {
int lfIndex = Bytes.firstIndexOf(barr, indexWithinNode, indexWithinNode + maxLength, Bytes.LF_BYTE);
if (lfIndex != -1) {
if (crIndexNode == -1) {
int lfIndexNode = Bytes.firstIndexOf(barr, fromIndexNode, fromIndexNode + maxLength, Bytes.LF_BYTE);
if (lfIndexNode != -1) {
if (!ignoreLoneEol) {
throw new IncorrectNewLineException("Found LF (" + (idx + lfIndex - n.position)
throw new IncorrectNewLineException("Found LF (" + (idx + lfIndexNode - n.position)
+ ") without preceding CR. :\n" + this.debugDataHex());
}
}
// not found, continue with next buffer
idx += maxLength;
if (idx >= max) {
// not found and reached the limit
return max;
}
n = n.next();
indexWithinNode = n.position;
continue;
} else {
// found, next byte should be LF
if (crIndex == barr.length - 1) {
if (crIndexNode == barr.length - 1) {
// found CR as the last byte of the current node, peek next node
byte nextByte = n.next().peek();
if (nextByte == Bytes.LF_BYTE) {
return idx + crIndex - n.position;
return idx + crIndexNode - fromIndexNode;
}
if (!ignoreLoneEol) {
throw new IncorrectNewLineException("Found CR (" + (idx + crIndex - n.position)
throw new IncorrectNewLineException("Found CR (" + (idx + crIndexNode - n.position)
+ ") without following LF. :\n" + this.debugDataHex());
}
} else {
// found CR within the current array
byte nextByte = barr[crIndex + 1];
byte nextByte = barr[crIndexNode + 1];
if (nextByte == Bytes.LF_BYTE) {
return idx + crIndex - n.position;
return idx + crIndexNode - fromIndexNode;
}
if (!ignoreLoneEol) {
throw new IncorrectNewLineException("Found CR (" + idx
+ ") without following LF. :\n" + this.debugDataHex());
}
indexWithinNode = crIndex + 1;
idx += indexWithinNode;
idx += (crIndexNode - fromIndexNode + 1);
fromIndexNode = crIndexNode + 1;
if (idx >= max) {
return max;
}
continue;
}
}

// not found, continue with next buffer
idx += maxLength;
if (idx >= max) {
// not found and reached the limit
return max;
}
n = n.next();
indexWithinNode = n.position;
fromIndexNode = n.position;
}
}

Expand Down
31 changes: 30 additions & 1 deletion common/buffers/src/test/java/io/helidon/common/buffers/DataReaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package io.helidon.common.buffers;

import java.nio.charset.StandardCharsets;
import java.util.concurrent.atomic.AtomicReference;

import org.junit.jupiter.api.Test;
Expand All @@ -30,7 +31,7 @@ void testFindNewLineWithLoneCR() {
// reading N bytes at a time until a new line is found
// with data containing a lone CR

byte[] data = new byte[] {0, 0, (byte) '\r', 0, (byte) '\r', (byte) '\n'};
byte[] data = "00\r0\r\n".getBytes(StandardCharsets.US_ASCII);
AtomicReference<byte[]> ref = new AtomicReference<>(data);
DataReader dataReader = new DataReader(() -> ref.getAndSet(null), true);

Expand All @@ -41,4 +42,32 @@ void testFindNewLineWithLoneCR() {
dataReader.skip(n);
assertThat(dataReader.findNewLine(n), is(0));
}

@Test
void testFindNewLineWithMultipleLoneCR() {
// if the stream index is accumulated with the node index for each lone CR
// it may exceed max and the new line is ignored

byte[] data = "00\r\r\r\n".getBytes(StandardCharsets.US_ASCII);
AtomicReference<byte[]> ref = new AtomicReference<>(data);
DataReader dataReader = new DataReader(() -> ref.getAndSet(null), true);

int n = 5;
assertThat(dataReader.findNewLine(n), is(4));
}

@Test
void testFindNewLineWithMultipleLoneWithinMax() {
// if the stream index is not updated for each lone CR
// the computed search range is too big and a value greater than max is returned

byte[] data = "00\r00\r\n00".getBytes(StandardCharsets.US_ASCII);
AtomicReference<byte[]> ref = new AtomicReference<>(data);
DataReader dataReader = new DataReader(() -> ref.getAndSet(null), true);

int n = 4;
assertThat(dataReader.findNewLine(n), is(n));
dataReader.skip(n);
assertThat(dataReader.findNewLine(n), is(1));
}
}