refactor(proto): make echo_packet_len return Result to handle negative lengths

This commit is contained in:
Nils Andreas Svee committed 2026-05-12 13:05:45 +07:00
1 parent b508a94198
commit 20a1d9dc8e
3 files changed
+11 -17

No files matched your search

+4 -4
View File
@@ -1,7 +1,7 @@
use crate::{ use crate::{
flags::{has, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY}, flags::{has, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY},
hmac, hmac,
layout::{try_echo_packet_len, PacketLayout}, layout::{echo_packet_len, PacketLayout},
open::{check_hmac_presence, reject}, open::{check_hmac_presence, reject},
params::Params, params::Params,
validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE, RECV_COUNT_SIZE, validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE, RECV_COUNT_SIZE,
@@ -43,7 +43,7 @@ pub fn encode_echo_request(request: &EchoRequest, hmac_key: Option<&[u8]>) -> Re
flags |= FLAG_HMAC; flags |= FLAG_HMAC;
} }
let layout = PacketLayout::echo(hmac_key.is_some(), &request.params); let layout = PacketLayout::echo(hmac_key.is_some(), &request.params);
let len = try_echo_packet_len(hmac_key.is_some(), &request.params)?; let len = echo_packet_len(hmac_key.is_some(), &request.params)?;
let payload_offset = layout.header_len(); let payload_offset = layout.header_len();
let available_payload_len = len.saturating_sub(payload_offset); let available_payload_len = len.saturating_sub(payload_offset);
if request.payload.len() > available_payload_len { if request.payload.len() > available_payload_len {
@@ -87,7 +87,7 @@ pub fn decode_echo_reply(
actual: packet.len(), actual: packet.len(),
}); });
} }
let expected_len = try_echo_packet_len(has(flags, FLAG_HMAC), params)?; let expected_len = echo_packet_len(has(flags, FLAG_HMAC), params)?;
if packet.len() != expected_len { if packet.len() != expected_len {
return Err(ProtoError::PacketLengthMismatch { return Err(ProtoError::PacketLengthMismatch {
expected: expected_len, expected: expected_len,
@@ -371,7 +371,7 @@ mod tests {
}; };
params.received_stats = ReceivedStats::Both; params.received_stats = ReceivedStats::Both;
let layout = PacketLayout::echo(true, &params); let layout = PacketLayout::echo(true, &params);
let mut packet = Vec::with_capacity(try_echo_packet_len(true, &params).unwrap()); let mut packet = Vec::with_capacity(echo_packet_len(true, &params).unwrap());
write_header(&mut packet, FLAG_REPLY | FLAG_HMAC); write_header(&mut packet, FLAG_REPLY | FLAG_HMAC);
packet.extend_from_slice(&[0; HMAC_SIZE]); packet.extend_from_slice(&[0; HMAC_SIZE]);
packet.extend_from_slice(&0x7896_b6ab_8771_5213u64.to_le_bytes()); packet.extend_from_slice(&0x7896_b6ab_8771_5213u64.to_le_bytes());
+6 -12
View File
@@ -111,13 +111,7 @@ pub fn echo_header_len(hmac: bool, params: &Params) -> usize {
PacketLayout::echo(hmac, params).header_len() PacketLayout::echo(hmac, params).header_len()
} }
pub fn echo_packet_len(hmac: bool, params: &Params) -> usize { pub fn echo_packet_len(hmac: bool, params: &Params) -> Result<usize> {
let header_len = echo_header_len(hmac, params);
let requested = usize::try_from(params.length).unwrap_or(0);
header_len.max(requested)
}
pub fn try_echo_packet_len(hmac: bool, params: &Params) -> Result<usize> {
let header_len = echo_header_len(hmac, params); let header_len = echo_header_len(hmac, params);
let requested = let requested =
usize::try_from(params.length).map_err(|_| ProtoError::NegativePacketLength { usize::try_from(params.length).map_err(|_| ProtoError::NegativePacketLength {
@@ -300,9 +294,9 @@ mod tests {
fn negotiated_length_never_smaller_than_header() { fn negotiated_length_never_smaller_than_header() {
let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both); let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both);
p.length = 20; p.length = 20;
assert_eq!(echo_packet_len(false, &p), 60); assert_eq!(echo_packet_len(false, &p).unwrap(), 60);
p.length = 92; p.length = 92;
assert_eq!(echo_packet_len(false, &p), 92); assert_eq!(echo_packet_len(false, &p).unwrap(), 92);
} }
#[test] #[test]
@@ -311,7 +305,7 @@ mod tests {
p.length = -1; p.length = -1;
assert_eq!( assert_eq!(
try_echo_packet_len(false, &p), echo_packet_len(false, &p),
Err(ProtoError::NegativePacketLength { length: -1 }) Err(ProtoError::NegativePacketLength { length: -1 })
); );
} }
@@ -320,9 +314,9 @@ mod tests {
fn checked_packet_len_preserves_non_negative_header_floor() { fn checked_packet_len_preserves_non_negative_header_floor() {
let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both); let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both);
p.length = 0; p.length = 0;
assert_eq!(try_echo_packet_len(false, &p), Ok(60)); assert_eq!(echo_packet_len(false, &p), Ok(60));
p.length = 92; p.length = 92;
assert_eq!(try_echo_packet_len(false, &p), Ok(92)); assert_eq!(echo_packet_len(false, &p), Ok(92));
} }
} }
+1 -1
View File
@@ -15,7 +15,7 @@ pub use echo::{decode_echo_reply, encode_echo_request, EchoReply, EchoRequest, T
pub use error::{ProtoError, Result}; pub use error::{ProtoError, Result};
pub use flags::*; pub use flags::*;
pub use hmac::{compute_hmac, compute_hmac_in_place, verify_hmac}; pub use hmac::{compute_hmac, compute_hmac_in_place, verify_hmac};
pub use layout::{echo_header_len, echo_packet_len, try_echo_packet_len, PacketLayout}; pub use layout::{echo_header_len, echo_packet_len, PacketLayout};
pub use open::{decode_open_reply, encode_open_request, OpenReply, OpenRequest}; pub use open::{decode_open_reply, encode_open_request, OpenReply, OpenRequest};
pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt}; pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt};