Skip to content

Commit c6565f4

Browse files
Fix JSON-RPC read failures with large payloads (>64KB)
- Add _read_exact() method to handle short reads from pipes - Update _read_message() to use _read_exact() for reliable large payload handling - Add comprehensive unit tests for short read scenarios and large payloads Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
1 parent d36017e commit c6565f4

File tree

2 files changed

+292
-2
lines changed

2 files changed

+292
-2
lines changed

python/copilot/jsonrpc.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,29 @@ def _read_loop(self):
160160
if self._running:
161161
print(f"JSON-RPC read loop error: {e}")
162162

163+
def _read_exact(self, num_bytes: int) -> bytes:
164+
"""
165+
Read exactly num_bytes, handling partial/short reads from pipes.
166+
167+
Args:
168+
num_bytes: Number of bytes to read
169+
170+
Returns:
171+
Bytes read from stream
172+
173+
Raises:
174+
EOFError: If stream ends before reading all bytes
175+
"""
176+
chunks = []
177+
remaining = num_bytes
178+
while remaining > 0:
179+
chunk = self.process.stdout.read(remaining)
180+
if not chunk:
181+
raise EOFError("Unexpected end of stream while reading JSON-RPC message")
182+
chunks.append(chunk)
183+
remaining -= len(chunk)
184+
return b"".join(chunks)
185+
163186
def _read_message(self) -> Optional[dict]:
164187
"""
165188
Read a single JSON-RPC message with Content-Length header (blocking)
@@ -182,8 +205,8 @@ def _read_message(self) -> Optional[dict]:
182205
# Read empty line
183206
self.process.stdout.readline()
184207

185-
# Read exact content
186-
content_bytes = self.process.stdout.read(content_length)
208+
# Read exact content using loop to handle short reads
209+
content_bytes = self._read_exact(content_length)
187210
content = content_bytes.decode("utf-8")
188211

189212
return json.loads(content)

python/test_jsonrpc.py

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
"""
2+
JsonRpcClient Unit Tests
3+
4+
Tests for the JSON-RPC client implementation, focusing on proper handling
5+
of large payloads and short reads from pipes.
6+
"""
7+
8+
import io
9+
import json
10+
11+
import pytest
12+
13+
from copilot.jsonrpc import JsonRpcClient
14+
15+
16+
class MockProcess:
17+
"""Mock subprocess.Popen for testing JSON-RPC client"""
18+
19+
def __init__(self):
20+
self.stdin = io.BytesIO()
21+
self.stdout = None # Will be set per test
22+
self.returncode = None
23+
24+
def poll(self):
25+
return self.returncode
26+
27+
28+
class ShortReadStream:
29+
"""
30+
Mock stream that simulates short reads from a pipe.
31+
32+
This simulates the behavior of Unix pipes when reading data larger than
33+
the pipe buffer (typically 64KB). The read() method will return fewer
34+
bytes than requested, requiring multiple read calls.
35+
"""
36+
37+
def __init__(self, data: bytes, chunk_size: int = 32768):
38+
"""
39+
Args:
40+
data: Complete data to be read
41+
chunk_size: Maximum bytes to return per read() call (simulates pipe buffer)
42+
"""
43+
self.data = data
44+
self.chunk_size = chunk_size
45+
self.pos = 0
46+
47+
def readline(self):
48+
"""Read until newline"""
49+
end = self.data.find(b"\n", self.pos) + 1
50+
if end == 0: # Not found
51+
result = self.data[self.pos :]
52+
self.pos = len(self.data)
53+
else:
54+
result = self.data[self.pos : end]
55+
self.pos = end
56+
return result
57+
58+
def read(self, n: int) -> bytes:
59+
"""
60+
Read at most n bytes, but may return fewer (short read).
61+
62+
This simulates the behavior of pipes when data exceeds buffer size.
63+
"""
64+
# Calculate how much we can return (limited by chunk_size)
65+
available = len(self.data) - self.pos
66+
to_read = min(n, available, self.chunk_size)
67+
68+
result = self.data[self.pos : self.pos + to_read]
69+
self.pos += to_read
70+
return result
71+
72+
73+
class TestReadExact:
74+
"""Tests for the _read_exact() method that handles short reads"""
75+
76+
def test_read_exact_single_chunk(self):
77+
"""Test reading data that fits in a single chunk"""
78+
content = b"Hello, World!"
79+
mock_stream = ShortReadStream(content, chunk_size=1024)
80+
81+
process = MockProcess()
82+
process.stdout = mock_stream
83+
84+
client = JsonRpcClient(process)
85+
result = client._read_exact(len(content))
86+
87+
assert result == content
88+
89+
def test_read_exact_multiple_chunks(self):
90+
"""Test reading data that requires multiple chunks (short reads)"""
91+
# Create 100KB of data
92+
content = b"x" * 100000
93+
# Simulate 32KB chunks (typical pipe behavior)
94+
mock_stream = ShortReadStream(content, chunk_size=32768)
95+
96+
process = MockProcess()
97+
process.stdout = mock_stream
98+
99+
client = JsonRpcClient(process)
100+
result = client._read_exact(len(content))
101+
102+
assert result == content
103+
assert len(result) == 100000
104+
105+
def test_read_exact_at_64kb_boundary(self):
106+
"""Test reading exactly 64KB (common pipe buffer size)"""
107+
content = b"y" * 65536 # Exactly 64KB
108+
mock_stream = ShortReadStream(content, chunk_size=65536)
109+
110+
process = MockProcess()
111+
process.stdout = mock_stream
112+
113+
client = JsonRpcClient(process)
114+
result = client._read_exact(len(content))
115+
116+
assert result == content
117+
assert len(result) == 65536
118+
119+
def test_read_exact_exceeds_64kb(self):
120+
"""Test reading data that exceeds 64KB (triggers the bug without fix)"""
121+
# 80KB - larger than typical pipe buffer
122+
content = b"z" * 81920
123+
# Simulate reading with 64KB limit (macOS pipe buffer)
124+
mock_stream = ShortReadStream(content, chunk_size=65536)
125+
126+
process = MockProcess()
127+
process.stdout = mock_stream
128+
129+
client = JsonRpcClient(process)
130+
result = client._read_exact(len(content))
131+
132+
assert result == content
133+
assert len(result) == 81920
134+
135+
def test_read_exact_empty_stream_raises_eof(self):
136+
"""Test that reading from closed stream raises EOFError"""
137+
mock_stream = ShortReadStream(b"", chunk_size=1024)
138+
139+
process = MockProcess()
140+
process.stdout = mock_stream
141+
142+
client = JsonRpcClient(process)
143+
144+
with pytest.raises(EOFError, match="Unexpected end of stream"):
145+
client._read_exact(10)
146+
147+
def test_read_exact_partial_data_raises_eof(self):
148+
"""Test that stream ending mid-message raises EOFError"""
149+
# Only 50 bytes available, but we request 100
150+
content = b"a" * 50
151+
mock_stream = ShortReadStream(content, chunk_size=1024)
152+
153+
process = MockProcess()
154+
process.stdout = mock_stream
155+
156+
client = JsonRpcClient(process)
157+
158+
with pytest.raises(EOFError, match="Unexpected end of stream"):
159+
client._read_exact(100)
160+
161+
162+
class TestReadMessageWithLargePayloads:
163+
"""Tests for _read_message() with large JSON-RPC messages"""
164+
165+
def create_jsonrpc_message(self, content_dict: dict) -> bytes:
166+
"""Create a complete JSON-RPC message with Content-Length header"""
167+
content = json.dumps(content_dict, separators=(",", ":"))
168+
content_bytes = content.encode("utf-8")
169+
header = f"Content-Length: {len(content_bytes)}\r\n\r\n"
170+
return header.encode("utf-8") + content_bytes
171+
172+
def test_read_message_small_payload(self):
173+
"""Test reading a small JSON-RPC message"""
174+
message = {"jsonrpc": "2.0", "id": "1", "result": {"status": "ok"}}
175+
full_data = self.create_jsonrpc_message(message)
176+
177+
mock_stream = ShortReadStream(full_data, chunk_size=1024)
178+
process = MockProcess()
179+
process.stdout = mock_stream
180+
181+
client = JsonRpcClient(process)
182+
result = client._read_message()
183+
184+
assert result == message
185+
186+
def test_read_message_large_payload_70kb(self):
187+
"""Test reading a 70KB JSON-RPC message (exceeds typical pipe buffer)"""
188+
# Simulate a large response with context echo (common pattern)
189+
large_content = "x" * 70000 # 70KB of data
190+
message = {
191+
"jsonrpc": "2.0",
192+
"id": "1",
193+
"result": {"content": large_content, "status": "complete"},
194+
}
195+
196+
full_data = self.create_jsonrpc_message(message)
197+
# Simulate 64KB pipe buffer limit
198+
mock_stream = ShortReadStream(full_data, chunk_size=65536)
199+
200+
process = MockProcess()
201+
process.stdout = mock_stream
202+
203+
client = JsonRpcClient(process)
204+
result = client._read_message()
205+
206+
assert result == message
207+
assert len(result["result"]["content"]) == 70000
208+
209+
def test_read_message_large_payload_100kb(self):
210+
"""Test reading a 100KB JSON-RPC message"""
211+
large_content = "y" * 100000 # 100KB
212+
message = {
213+
"jsonrpc": "2.0",
214+
"id": "2",
215+
"result": {"data": large_content, "metadata": {"size": 100000}},
216+
}
217+
218+
full_data = self.create_jsonrpc_message(message)
219+
# Simulate short reads with 32KB chunks
220+
mock_stream = ShortReadStream(full_data, chunk_size=32768)
221+
222+
process = MockProcess()
223+
process.stdout = mock_stream
224+
225+
client = JsonRpcClient(process)
226+
result = client._read_message()
227+
228+
assert result == message
229+
assert len(result["result"]["data"]) == 100000
230+
231+
def test_read_message_exactly_64kb_content(self):
232+
"""Test reading message with exactly 64KB of content"""
233+
content_64kb = "z" * 65536 # Exactly 64KB
234+
message = {"jsonrpc": "2.0", "id": "3", "result": {"content": content_64kb}}
235+
236+
full_data = self.create_jsonrpc_message(message)
237+
mock_stream = ShortReadStream(full_data, chunk_size=65536)
238+
239+
process = MockProcess()
240+
process.stdout = mock_stream
241+
242+
client = JsonRpcClient(process)
243+
result = client._read_message()
244+
245+
assert result == message
246+
assert len(result["result"]["content"]) == 65536
247+
248+
def test_read_message_multiple_messages_in_sequence(self):
249+
"""Test reading multiple large messages in sequence"""
250+
message1 = {"jsonrpc": "2.0", "id": "1", "result": {"data": "a" * 50000}}
251+
message2 = {"jsonrpc": "2.0", "id": "2", "result": {"data": "b" * 80000}}
252+
253+
data1 = self.create_jsonrpc_message(message1)
254+
data2 = self.create_jsonrpc_message(message2)
255+
full_data = data1 + data2
256+
257+
mock_stream = ShortReadStream(full_data, chunk_size=32768)
258+
process = MockProcess()
259+
process.stdout = mock_stream
260+
261+
client = JsonRpcClient(process)
262+
263+
result1 = client._read_message()
264+
assert result1 == message1
265+
266+
result2 = client._read_message()
267+
assert result2 == message2

0 commit comments

Comments
 (0)