fix: don't panic on a peer's obfuscated port (u16) — widen the field
GetPeerAddressResponse narrowed obfuscated_port (u16) into a u8 via try_into().unwrap(), panicking for any real obfuscated port (>255) and unwinding the client's single ops-loop thread — hanging all downloads, searches and peer events. Since download() now auto-resolves peer addresses via GetPeerAddress, a download to any obfuscation-enabled peer could crash the whole client. Widen Peer::obfuscated_port to u16 and stop truncating.
This commit is contained in:
@@ -1137,8 +1137,13 @@ impl Client {
|
|||||||
port,
|
port,
|
||||||
None,
|
None,
|
||||||
0,
|
0,
|
||||||
obfuscation_type.try_into().unwrap(),
|
// obfuscation_type is a small enum; a
|
||||||
obfuscated_port.try_into().unwrap(),
|
// real obfuscated_port is a full u16 and
|
||||||
|
// must not be truncated into a u8 (which
|
||||||
|
// panicked and took down the ops thread).
|
||||||
|
u8::try_from(obfuscation_type)
|
||||||
|
.unwrap_or(0),
|
||||||
|
obfuscated_port,
|
||||||
);
|
);
|
||||||
let client_context_clone =
|
let client_context_clone =
|
||||||
client_context.clone();
|
client_context.clone();
|
||||||
|
|||||||
@@ -89,7 +89,7 @@ pub struct Peer {
|
|||||||
pub token: Option<u32>,
|
pub token: Option<u32>,
|
||||||
pub privileged: Option<u8>,
|
pub privileged: Option<u8>,
|
||||||
pub unknown: Option<u8>,
|
pub unknown: Option<u8>,
|
||||||
pub obfuscated_port: Option<u8>,
|
pub obfuscated_port: Option<u16>,
|
||||||
}
|
}
|
||||||
impl Peer {
|
impl Peer {
|
||||||
#[allow(clippy::too_many_arguments, dead_code)]
|
#[allow(clippy::too_many_arguments, dead_code)]
|
||||||
@@ -102,7 +102,7 @@ impl Peer {
|
|||||||
token: Option<u32>,
|
token: Option<u32>,
|
||||||
privileged: u8,
|
privileged: u8,
|
||||||
unknown: u8,
|
unknown: u8,
|
||||||
obfuscated_port: u8,
|
obfuscated_port: u16,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
username,
|
username,
|
||||||
@@ -146,10 +146,28 @@ impl Peer {
|
|||||||
token: Some(token),
|
token: Some(token),
|
||||||
privileged: Some(privileged),
|
privileged: Some(privileged),
|
||||||
unknown: Some(unknown),
|
unknown: Some(unknown),
|
||||||
obfuscated_port: Some(obfuscated_port),
|
obfuscated_port: Some(u16::from(obfuscated_port)),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
#[test]
|
||||||
|
fn new_accepts_a_full_range_obfuscated_port() {
|
||||||
|
// A real obfuscated port is a u16; it must be stored without truncation.
|
||||||
|
// (The GetPeerAddressResponse path previously narrowed it to u8 and
|
||||||
|
// panicked for any value > 255, crashing the client's ops thread.)
|
||||||
|
let peer = Peer::new(
|
||||||
|
"u".to_string(),
|
||||||
|
ConnectionType::P,
|
||||||
|
"1.2.3.4".to_string(),
|
||||||
|
1,
|
||||||
|
None,
|
||||||
|
0,
|
||||||
|
0,
|
||||||
|
51770,
|
||||||
|
);
|
||||||
|
assert_eq!(peer.obfuscated_port, Some(51770));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn new_from_message_returns_none_on_invalid_connection_type() {
|
fn new_from_message_returns_none_on_invalid_connection_type() {
|
||||||
// username "ab", connection_type "X" (not P/F/D) from an untrusted server.
|
// username "ab", connection_type "X" (not P/F/D) from an untrusted server.
|
||||||
|
|||||||
Reference in New Issue
Block a user