mirror of
https://github.com/rustdesk/hbb_common.git
synced 2026-10-26 14:10:26 +00:00
bound the fragmented WebSocket message too, and say what lowering the cap does not promise
The two WebSocket tests both stopped at max_frame_size: a header over the cap and a single large frame after the cap is lifted. Dropping max_message_size from set_max_packet_length left both passing, and that line is what refuses two frames each under the cap that reassemble to a message over it - the other way a peer buys the allocation. A test now sends exactly that, 12 KiB and a 12 KiB continuation against a 16 KiB cap, and fails when the line is dropped. The doc on Stream::set_max_packet_length claimed that lowering the bound never discards a message already being reassembled. That is the TCP codec, which checks only on the header; WebRTC and a fragmented WebSocket message re-check the bound on every fragment and end the message once it passes. The doc now promises only what holds everywhere: set it before the first read, and do not lean on lowering it to constrain a message in progress. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab
This commit is contained in:
1 parent
e999dcea24
commit
f0f1548ba3
2 files changed
+32
-6
No files matched your search
+2
-2
@@ -38,8 +38,8 @@ impl Stream {
|
|||||||
/// legitimately sends is large, so an unauthenticated connection cannot make us hold more than
|
/// legitimately sends is large, so an unauthenticated connection cannot make us hold more than
|
||||||
/// that. Each transport clamps `n` to its own ceiling, so `usize::MAX` restores the default.
|
/// that. Each transport clamps `n` to its own ceiling, so `usize::MAX` restores the default.
|
||||||
///
|
///
|
||||||
/// Set it before the first read of untrusted data: a lower bound applies from the next frame
|
/// Set it before the first read of untrusted data. Lowering it later is not a way to constrain
|
||||||
/// on and does not discard one whose decoding or reassembly has already begun.
|
/// a message already in progress: what that does to one is up to the transport.
|
||||||
#[inline]
|
#[inline]
|
||||||
pub fn set_max_packet_length(&mut self, n: usize) {
|
pub fn set_max_packet_length(&mut self, n: usize) {
|
||||||
match self {
|
match self {
|
||||||
|
|||||||
+30
-4
@@ -555,10 +555,11 @@ mod tests {
|
|||||||
assert_eq!(check_ws("127.0.0.1:34567"), "ws://127.0.0.1:34569");
|
assert_eq!(check_ws("127.0.0.1:34567"), "ws://127.0.0.1:34569");
|
||||||
Config::set_options(options);
|
Config::set_options(options);
|
||||||
}
|
}
|
||||||
// A server-to-client binary frame is unmasked, so it can be put on the wire by hand: the header
|
// A server-to-client frame is unmasked, so it can be put on the wire by hand: the header alone,
|
||||||
// alone, which is all it takes to ask tungstenite for the allocation, or with its payload.
|
// which is all it takes to ask tungstenite for the allocation, or with its payload. `head` is
|
||||||
fn ws_binary_frame(len: usize, with_payload: bool) -> Vec<u8> {
|
// the first byte, FIN and opcode.
|
||||||
let mut f = vec![0x82];
|
fn ws_frame(head: u8, len: usize, with_payload: bool) -> Vec<u8> {
|
||||||
|
let mut f = vec![head];
|
||||||
if len < 126 {
|
if len < 126 {
|
||||||
f.push(len as u8);
|
f.push(len as u8);
|
||||||
} else if len <= u16::MAX as usize {
|
} else if len <= u16::MAX as usize {
|
||||||
@@ -574,6 +575,10 @@ mod tests {
|
|||||||
f
|
f
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn ws_binary_frame(len: usize, with_payload: bool) -> Vec<u8> {
|
||||||
|
ws_frame(0x82, len, with_payload)
|
||||||
|
}
|
||||||
|
|
||||||
async fn ws_loopback() -> (WsFramedStream, TcpStream) {
|
async fn ws_loopback() -> (WsFramedStream, TcpStream) {
|
||||||
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
|
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||||
let addr = listener.local_addr().unwrap();
|
let addr = listener.local_addr().unwrap();
|
||||||
@@ -620,4 +625,25 @@ mod tests {
|
|||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(got.len(), 200_000, "lifting the cap lets a large message through again");
|
assert_eq!(got.len(), 200_000, "lifting the cap lets a large message through again");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Two frames each under the cap that reassemble to a message over it: the frame bound lets
|
||||||
|
// both through, so this is what the message bound alone refuses.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn max_packet_length_bounds_fragmented_message() {
|
||||||
|
const CAP: usize = 16 * 1024;
|
||||||
|
let (mut ws, mut server) = ws_loopback().await;
|
||||||
|
ws.set_max_packet_length(CAP);
|
||||||
|
|
||||||
|
// Binary with FIN clear, then a continuation with FIN set: 12 KiB each, 24 KiB together.
|
||||||
|
server.write_all(&ws_frame(0x02, 12 * 1024, true)).await.unwrap();
|
||||||
|
server.write_all(&ws_frame(0x80, 12 * 1024, true)).await.unwrap();
|
||||||
|
match timeout(Duration::from_secs(5), ws.next()).await {
|
||||||
|
Ok(Some(Err(e))) => assert!(e.to_string().contains("Message too long"), "{}", e),
|
||||||
|
Ok(other) => panic!(
|
||||||
|
"expected a refusal, got {:?}",
|
||||||
|
other.map(|r| r.map(|b| b.len()))
|
||||||
|
),
|
||||||
|
Err(_) => panic!("next() hung on a fragmented message over the cap"),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user