Skip to content

Commit bf59554

Browse files
udawclaude
andcommitted
Fix frame splitting in LengthPrefixedByteArrayProcessor
- Read the length of each frame at the current position instead of always at the start of the read, so several frames of different length in one read are split correctly. - Clear the stored first length byte once it has been used, and do not take the fast path while such a byte is pending, so a read that ends after the first length byte is handled by step(). Adds tests that lock in the existing behaviour (single frame, several frames in one read, a frame split across two reads) plus regression tests for the two cases above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d6d4dce commit bf59554

3 files changed

Lines changed: 129 additions & 2 deletions

File tree

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# HAP-Java 2.0.9
22
* Fix Set Duration and Remaining Duration characteristics to advertise format `uint32` instead of `int` [#202](https://github.com/hap-java/HAP-Java/issues/202)
33
* Stop logging the SRP shared secret, derived key and M5 message data during pairing at DEBUG level
4+
* Fix splitting of encrypted frames when several frames of different length arrive in one read, or when a read ends after the first length byte
45

56
# HAP-Java 2.0.8
67
* Updated bouncy castle to 1.83

‎src/main/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessor.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ public synchronized Collection<byte[]> handle(byte[] data) {
2121
int pos = 0;
2222
LOGGER.trace(
2323
"Received message of length {}. Existing buffer is {}", data.length, buffer.size());
24-
if (buffer.size() == 0) {
24+
if (buffer.size() == 0 && firstLengthByteBuffer == null) {
2525
while (data.length - pos > 18) {
26-
int targetLength = (data[0] & 0xFF) + (data[1] & 0xFF) * 256 + 16 + 2;
26+
int targetLength = (data[pos] & 0xFF) + (data[pos + 1] & 0xFF) * 256 + 16 + 2;
2727
LOGGER.trace("Attempting to read message of length {}", targetLength);
2828
if (data.length >= pos + targetLength) {
2929
byte[] b = new byte[targetLength - 2];
@@ -56,6 +56,7 @@ private void step(byte[] data, int pos, Collection<byte[]> results) {
5656
if (targetLength == 0) {
5757
if (firstLengthByteBuffer != null) {
5858
targetLength = (firstLengthByteBuffer & 0xFF) + (data[pos] & 0xFF) * 256 + 16;
59+
firstLengthByteBuffer = null;
5960
pos += 1;
6061
LOGGER.trace(
6162
"Received the second byte after storing the first byte. New length is {}",
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package io.github.hapjava.server.impl.connections;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.io.ByteArrayOutputStream;
6+
import java.util.ArrayList;
7+
import java.util.Arrays;
8+
import java.util.Collection;
9+
import java.util.List;
10+
import org.junit.Test;
11+
12+
public class LengthPrefixedByteArrayProcessorTest {
13+
14+
private static final int TAG_LENGTH = 16;
15+
16+
/**
17+
* Builds a frame as it arrives on the wire: 2-byte little-endian payload length, payload, 16-byte
18+
* tag. The payload and tag bytes start at 0x80 so that they never look like a short length.
19+
*/
20+
private static byte[] frame(int payloadLength, int seed) {
21+
byte[] frame = new byte[2 + payloadLength + TAG_LENGTH];
22+
frame[0] = (byte) (payloadLength & 0xFF);
23+
frame[1] = (byte) ((payloadLength >> 8) & 0xFF);
24+
for (int i = 2; i < frame.length; i++) {
25+
frame[i] = (byte) (0x80 + seed + i);
26+
}
27+
return frame;
28+
}
29+
30+
/** What the processor is expected to emit for a frame: payload + tag, without the length. */
31+
private static byte[] body(byte[] frame) {
32+
return Arrays.copyOfRange(frame, 2, frame.length);
33+
}
34+
35+
private static byte[] concat(byte[]... parts) {
36+
ByteArrayOutputStream out = new ByteArrayOutputStream();
37+
for (byte[] part : parts) {
38+
out.write(part, 0, part.length);
39+
}
40+
return out.toByteArray();
41+
}
42+
43+
private static List<byte[]> feed(LengthPrefixedByteArrayProcessor processor, byte[]... reads) {
44+
List<byte[]> results = new ArrayList<>();
45+
for (byte[] read : reads) {
46+
results.addAll(processor.handle(read));
47+
}
48+
return results;
49+
}
50+
51+
private static List<Integer> sizes(Collection<byte[]> results) {
52+
List<Integer> sizes = new ArrayList<>();
53+
for (byte[] result : results) {
54+
sizes.add(result.length);
55+
}
56+
return sizes;
57+
}
58+
59+
private static void assertBodies(List<byte[]> results, byte[]... frames) {
60+
List<Integer> expectedSizes = new ArrayList<>();
61+
for (byte[] frame : frames) {
62+
expectedSizes.add(frame.length - 2);
63+
}
64+
assertThat(sizes(results)).isEqualTo(expectedSizes);
65+
for (int i = 0; i < frames.length; i++) {
66+
assertThat(results.get(i)).as("frame %d", i).isEqualTo(body(frames[i]));
67+
}
68+
}
69+
70+
@Test
71+
public void singleFrameInOneRead() {
72+
byte[] a = frame(100, 1);
73+
assertBodies(feed(new LengthPrefixedByteArrayProcessor(), a), a);
74+
}
75+
76+
@Test
77+
public void threeFramesInOneRead() {
78+
byte[] a = frame(1024, 1);
79+
byte[] b = frame(1024, 2);
80+
byte[] c = frame(200, 3);
81+
assertBodies(feed(new LengthPrefixedByteArrayProcessor(), concat(a, b, c)), a, b, c);
82+
}
83+
84+
@Test
85+
public void frameSplitAcrossTwoReads() {
86+
byte[] a = frame(1024, 1);
87+
int[] offsets = {1, 2, 3, 10, 18, 19, 20, 500, a.length - 1};
88+
for (int offset : offsets) {
89+
byte[] first = Arrays.copyOfRange(a, 0, offset);
90+
byte[] second = Arrays.copyOfRange(a, offset, a.length);
91+
List<byte[]> results = feed(new LengthPrefixedByteArrayProcessor(), first, second);
92+
assertThat(sizes(results)).as("split at %d", offset).containsExactly(a.length - 2);
93+
assertThat(results.get(0)).as("split at %d", offset).isEqualTo(body(a));
94+
}
95+
}
96+
97+
@Test
98+
public void frameSplitWithNextFrameInSecondRead() {
99+
byte[] a = frame(1024, 1);
100+
byte[] b = frame(200, 2);
101+
byte[] first = Arrays.copyOfRange(a, 0, 500);
102+
byte[] second = concat(Arrays.copyOfRange(a, 500, a.length), b);
103+
assertBodies(feed(new LengthPrefixedByteArrayProcessor(), first, second), a, b);
104+
}
105+
106+
@Test
107+
public void shortFrameFollowedByLongerFrameInOneRead() {
108+
byte[] a = frame(5, 1);
109+
byte[] b = frame(100, 2);
110+
List<byte[]> results = feed(new LengthPrefixedByteArrayProcessor(), concat(a, b));
111+
assertThat(sizes(results)).containsExactly(21, 116);
112+
assertBodies(results, a, b);
113+
}
114+
115+
@Test
116+
public void singleLengthByteThenRestThenAnotherFrame() {
117+
byte[] a = frame(5, 1);
118+
byte[] b = frame(40, 2);
119+
byte[] first = Arrays.copyOfRange(a, 0, 1);
120+
byte[] second = concat(Arrays.copyOfRange(a, 1, a.length), b);
121+
List<byte[]> results = feed(new LengthPrefixedByteArrayProcessor(), first, second);
122+
assertThat(sizes(results)).containsExactly(21, 56);
123+
assertBodies(results, a, b);
124+
}
125+
}

0 commit comments

Comments
 (0)