Skip to content

Commit cacf112

Browse files
ChuxelCopilot
andauthored
fix(rust): recover JSON-RPC lone surrogates (#2283)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 0cb965b commit cacf112

2 files changed

Lines changed: 218 additions & 3 deletions

File tree

rust/src/jsonrpc.rs

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,68 @@ impl JsonRpcResponse {
169169

170170
const CONTENT_LENGTH_HEADER: &str = "Content-Length: ";
171171

172+
/// Rewrites unpaired UTF-16 surrogate escapes to `\uFFFD`.
173+
///
174+
/// Returns `None` when the body contains no unpaired surrogate, so valid
175+
/// frames do not incur a repair allocation.
176+
fn repair_lone_surrogates(body: &[u8]) -> Option<Vec<u8>> {
177+
fn hex_escape_at(body: &[u8], index: usize) -> Option<u16> {
178+
let digits = body.get(index + 2..index + 6)?;
179+
let text = std::str::from_utf8(digits).ok()?;
180+
u16::from_str_radix(text, 16).ok()
181+
}
182+
183+
let mut repaired = None;
184+
let mut in_string = false;
185+
let mut index = 0;
186+
187+
while index < body.len() {
188+
let byte = body[index];
189+
190+
if !in_string {
191+
in_string = byte == b'"';
192+
index += 1;
193+
continue;
194+
}
195+
196+
match byte {
197+
b'"' => {
198+
in_string = false;
199+
index += 1;
200+
}
201+
// Consume non-Unicode escapes whole so an escaped backslash cannot
202+
// be mistaken for the start of a surrogate escape.
203+
b'\\' if body.get(index + 1) != Some(&b'u') => index += 2,
204+
b'\\' => {
205+
let Some(unit) = hex_escape_at(body, index) else {
206+
index += 2;
207+
continue;
208+
};
209+
210+
let is_pair = (0xD800..0xDC00).contains(&unit)
211+
&& body.get(index + 6) == Some(&b'\\')
212+
&& body.get(index + 7) == Some(&b'u')
213+
&& hex_escape_at(body, index + 6)
214+
.is_some_and(|low| (0xDC00..0xE000).contains(&low));
215+
216+
if is_pair {
217+
index += 12;
218+
continue;
219+
}
220+
221+
if (0xD800..0xE000).contains(&unit) {
222+
let output = repaired.get_or_insert_with(|| body.to_vec());
223+
output[index..index + 6].copy_from_slice(br"\ufffd");
224+
}
225+
index += 6;
226+
}
227+
_ => index += 1,
228+
}
229+
}
230+
231+
repaired
232+
}
233+
172234
/// One framed JSON-RPC message handed to the writer actor.
173235
///
174236
/// `frame` is the fully serialized bytes (header + body); the caller pays
@@ -428,8 +490,26 @@ impl JsonRpcClient {
428490
let mut body = vec![0u8; length];
429491
reader.read_exact(&mut body).await?;
430492

431-
let message: JsonRpcMessage = serde_json::from_slice(&body)?;
432-
Ok(Some(message))
493+
match serde_json::from_slice::<JsonRpcMessage>(&body) {
494+
Ok(message) => Ok(Some(message)),
495+
Err(error) => {
496+
// Dropping an undecodable frame could leave its pending
497+
// request waiting forever because this layer has no timeout.
498+
match repair_lone_surrogates(&body)
499+
.and_then(|repaired| serde_json::from_slice::<JsonRpcMessage>(&repaired).ok())
500+
{
501+
Some(message) => {
502+
warn!(
503+
error = %error,
504+
length,
505+
"recovered JSON-RPC frame containing unpaired UTF-16 surrogates"
506+
);
507+
Ok(Some(message))
508+
}
509+
None => Err(error.into()),
510+
}
511+
}
512+
}
433513
}
434514

435515
/// Send a JSON-RPC request and wait for the matching response.

rust/tests/jsonrpc_test.rs

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#![allow(clippy::unwrap_used)]
33

44
use github_copilot_sdk::test_support::{JsonRpcClient, JsonRpcNotification, JsonRpcRequest};
5-
use tokio::io::{AsyncWrite, AsyncWriteExt, duplex};
5+
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt, duplex};
66
use tokio::sync::{broadcast, mpsc};
77

88
/// Write a Content-Length framed JSON-RPC message to a writer.
@@ -13,6 +13,28 @@ async fn write_framed(writer: &mut (impl AsyncWrite + Unpin), body: &[u8]) {
1313
writer.flush().await.unwrap();
1414
}
1515

16+
async fn read_framed(reader: &mut (impl AsyncRead + Unpin)) -> Vec<u8> {
17+
let mut header = String::new();
18+
loop {
19+
let mut byte = [0u8; 1];
20+
reader.read_exact(&mut byte).await.unwrap();
21+
header.push(byte[0] as char);
22+
if header.ends_with("\r\n\r\n") {
23+
break;
24+
}
25+
}
26+
27+
let length = header
28+
.trim()
29+
.strip_prefix("Content-Length: ")
30+
.unwrap()
31+
.parse()
32+
.unwrap();
33+
let mut body = vec![0u8; length];
34+
reader.read_exact(&mut body).await.unwrap();
35+
body
36+
}
37+
1638
#[tokio::test]
1739
async fn request_response_round_trip() {
1840
// duplex: client_write → server_read, server_write → client_read
@@ -410,3 +432,116 @@ async fn send_request_cancellation_does_not_leak_pending() {
410432
assert_eq!(response.result.unwrap()["ok"], true);
411433
server_task.await.unwrap();
412434
}
435+
436+
#[test]
437+
fn lone_surrogate_yields_unexpected_end_of_hex_escape() {
438+
let error = serde_json::from_slice::<serde_json::Value>(br#""\ud83d""#).unwrap_err();
439+
440+
assert_eq!(
441+
error.to_string(),
442+
"unexpected end of hex escape at line 1 column 8"
443+
);
444+
}
445+
446+
#[tokio::test]
447+
async fn lone_surrogate_frame_is_recovered_without_closing_connection() {
448+
let (client_write, mut server_read) = duplex(4096);
449+
let (mut server_write, client_read) = duplex(4096);
450+
let (notification_tx, _) = broadcast::channel(16);
451+
let (request_tx, _) = mpsc::unbounded_channel();
452+
let client = JsonRpcClient::new(client_write, client_read, notification_tx, request_tx);
453+
454+
let server_task = tokio::spawn(async move {
455+
let request: JsonRpcRequest =
456+
serde_json::from_slice(&read_framed(&mut server_read).await).unwrap();
457+
let response = format!(
458+
r#"{{"jsonrpc":"2.0","id":{},"result":{{"name":"invalid \ud83d value"}}}}"#,
459+
request.id
460+
);
461+
write_framed(&mut server_write, response.as_bytes()).await;
462+
463+
let request: JsonRpcRequest =
464+
serde_json::from_slice(&read_framed(&mut server_read).await).unwrap();
465+
let response = serde_json::json!({
466+
"jsonrpc": "2.0",
467+
"id": request.id,
468+
"result": {"name": "still connected"}
469+
});
470+
write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await;
471+
});
472+
473+
let response = client.send_request("models.list", None).await.unwrap();
474+
assert_eq!(
475+
response.result.unwrap()["name"],
476+
serde_json::json!("invalid \u{FFFD} value")
477+
);
478+
479+
let response = client.send_request("account.getQuota", None).await.unwrap();
480+
assert_eq!(
481+
response.result.unwrap()["name"],
482+
serde_json::json!("still connected")
483+
);
484+
server_task.await.unwrap();
485+
}
486+
487+
#[tokio::test]
488+
async fn unrepairable_frame_remains_fatal() {
489+
let (client_write, mut server_read) = duplex(4096);
490+
let (mut server_write, client_read) = duplex(4096);
491+
let (notification_tx, _) = broadcast::channel(16);
492+
let (request_tx, _) = mpsc::unbounded_channel();
493+
let client = JsonRpcClient::new(client_write, client_read, notification_tx, request_tx);
494+
495+
let server_task = tokio::spawn(async move {
496+
let request: JsonRpcRequest =
497+
serde_json::from_slice(&read_framed(&mut server_read).await).unwrap();
498+
let response = format!(
499+
r#"{{"jsonrpc":"2.0","id":{},"result":{{"surrogate":"\ud83d","escape":"\q"}}}}"#,
500+
request.id
501+
);
502+
write_framed(&mut server_write, response.as_bytes()).await;
503+
});
504+
505+
let error = tokio::time::timeout(
506+
std::time::Duration::from_secs(2),
507+
client.send_request("models.list", None),
508+
)
509+
.await
510+
.expect("unrepairable frame did not terminate the pending request")
511+
.unwrap_err();
512+
513+
assert_eq!(error.to_string(), "request cancelled");
514+
assert!(error.is_transport_failure());
515+
server_task.await.unwrap();
516+
}
517+
518+
#[tokio::test]
519+
async fn valid_pairs_and_escaped_backslashes_are_untouched() {
520+
let (client_write, mut server_read) = duplex(4096);
521+
let (mut server_write, client_read) = duplex(4096);
522+
let (notification_tx, _) = broadcast::channel(16);
523+
let (request_tx, _) = mpsc::unbounded_channel();
524+
let client = JsonRpcClient::new(client_write, client_read, notification_tx, request_tx);
525+
526+
let server_task = tokio::spawn(async move {
527+
let request: JsonRpcRequest =
528+
serde_json::from_slice(&read_framed(&mut server_read).await).unwrap();
529+
let response = format!(
530+
r#"{{"jsonrpc":"2.0","id":{},"result":{{"emoji":"\ud83d\ude00","path":"C:\\ud83d","invalid":"\ud83d"}}}}"#,
531+
request.id
532+
);
533+
write_framed(&mut server_write, response.as_bytes()).await;
534+
});
535+
536+
let result = client
537+
.send_request("models.list", None)
538+
.await
539+
.unwrap()
540+
.result
541+
.unwrap();
542+
543+
assert_eq!(result["emoji"], serde_json::json!("😀"));
544+
assert_eq!(result["path"], serde_json::json!(r"C:\ud83d"));
545+
assert_eq!(result["invalid"], serde_json::json!("\u{FFFD}"));
546+
server_task.await.unwrap();
547+
}

0 commit comments

Comments
 (0)