Skip to content

Commit 4eab479

Browse files
committed
refactor(p2p): remove fragile FromStr parsing for signaling types
Remove FromStr implementations for SignalingMethod and HttpSignalingInfo that used fragile slash-delimited string parsing. These are no longer reachable from user input after the legacy address format removal, and were only used by custom Serialize/Deserialize impls. Switch to derived Serialize/Deserialize for SignalingMethod, HttpSignalingInfo, PathPrefix, and ProxyScheme.
1 parent dabc334 commit 4eab479

File tree

3 files changed

+38
-1099
lines changed

3 files changed

+38
-1099
lines changed

crates/p2p/src/webrtc/mod.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ pub use signal::{
1818
};
1919

2020
mod signaling_method;
21-
pub use signaling_method::{
22-
HttpSignalingInfo, PathPrefix, ProxyScheme, SignalingMethod, SignalingMethodParseError,
23-
};
21+
pub use signaling_method::{HttpSignalingInfo, PathPrefix, ProxyScheme, SignalingMethod};
2422

2523
mod connection_auth;
2624
pub use connection_auth::{ConnectionAuth, ConnectionAuthEncrypted};

crates/p2p/src/webrtc/signaling_method/http.rs

Lines changed: 4 additions & 277 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,13 @@
3030
//! method variant. HTTPS is recommended for production environments to
3131
//! protect signaling data and prevent tampering during transmission.
3232
33-
use std::{fmt, str::FromStr};
33+
use std::fmt;
3434

3535
use binprot_derive::{BinProtRead, BinProtWrite};
3636
use serde::{Deserialize, Serialize};
3737

3838
use crate::webrtc::Host;
3939

40-
use super::SignalingMethodParseError;
41-
4240
/// HTTP signaling server connection information.
4341
///
4442
/// `HttpSignalingInfo` encapsulates the network location information needed
@@ -74,7 +72,9 @@ use super::SignalingMethodParseError;
7472
/// port: 443,
7573
/// };
7674
/// ```
77-
#[derive(BinProtWrite, BinProtRead, Eq, PartialEq, Ord, PartialOrd, Debug, Clone)]
75+
#[derive(
76+
BinProtWrite, BinProtRead, Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Serialize, Deserialize,
77+
)]
7878
pub struct HttpSignalingInfo {
7979
/// The host address for the HTTP signaling server.
8080
///
@@ -129,276 +129,3 @@ impl From<([u8; 4], u16)> for HttpSignalingInfo {
129129
}
130130
}
131131
}
132-
133-
impl FromStr for HttpSignalingInfo {
134-
type Err = SignalingMethodParseError;
135-
136-
/// Parses a string representation into HTTP signaling info.
137-
///
138-
/// This method parses path-like strings that contain host and port
139-
/// information separated by forward slashes. The expected format is
140-
/// `{host}/{port}` or `/{host}/{port}`.
141-
///
142-
/// # Format
143-
///
144-
/// - Input: `{host}/{port}` (leading slash optional)
145-
/// - Host: Domain name, IPv4, IPv6, or multiaddr format
146-
/// - Port: 16-bit unsigned integer (0-65535)
147-
///
148-
/// # Examples
149-
///
150-
/// ```
151-
/// use mina::signaling_method::HttpSignalingInfo;
152-
///
153-
/// // Domain and port
154-
/// let info: HttpSignalingInfo = "signal.example.com/443".parse()?;
155-
///
156-
/// // IPv4 and port
157-
/// let info: HttpSignalingInfo = "192.168.1.100/8080".parse()?;
158-
///
159-
/// // With leading slash
160-
/// let info: HttpSignalingInfo = "/localhost/8080".parse()?;
161-
/// ```
162-
///
163-
/// # Errors
164-
///
165-
/// Returns [`SignalingMethodParseError`] for:
166-
/// - Missing host or port components
167-
/// - Invalid host format (not a valid hostname, IP, or multiaddr)
168-
/// - Invalid port number (not a valid 16-bit unsigned integer)
169-
fn from_str(s: &str) -> Result<Self, Self::Err> {
170-
let mut iter = s.split('/').filter(|v| !v.trim().is_empty());
171-
let host_str = iter
172-
.next()
173-
.ok_or(SignalingMethodParseError::NotEnoughArgs)?;
174-
let host = Host::from_str(host_str)
175-
.map_err(|err| SignalingMethodParseError::HostParseError(err.to_string()))?;
176-
177-
let port = iter
178-
.next()
179-
.ok_or(SignalingMethodParseError::NotEnoughArgs)?
180-
.parse::<u16>()
181-
.map_err(|err| SignalingMethodParseError::PortParseError(err.to_string()))?;
182-
183-
Ok(Self { host, port })
184-
}
185-
}
186-
187-
impl Serialize for HttpSignalingInfo {
188-
/// Serializes the HTTP signaling info as a string.
189-
///
190-
/// This uses the `Display` implementation to convert the signaling
191-
/// info to its string representation for serialization. The output
192-
/// format is `/{host}/{port}`.
193-
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
194-
where
195-
S: serde::Serializer,
196-
{
197-
serializer.serialize_str(&self.to_string())
198-
}
199-
}
200-
201-
impl<'de> serde::Deserialize<'de> for HttpSignalingInfo {
202-
/// Deserializes HTTP signaling info from a string.
203-
///
204-
/// This uses the [`FromStr`] implementation to parse the string
205-
/// representation back into an [`HttpSignalingInfo`] instance.
206-
/// The expected format is `{host}/{port}` or `/{host}/{port}`.
207-
///
208-
/// # Errors
209-
///
210-
/// Returns a deserialization error if the string cannot be parsed
211-
/// as valid HTTP signaling info (invalid host, port, or format).
212-
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
213-
where
214-
D: serde::Deserializer<'de>,
215-
{
216-
let s: String = Deserialize::deserialize(deserializer)?;
217-
s.parse().map_err(serde::de::Error::custom)
218-
}
219-
}
220-
221-
#[cfg(test)]
222-
mod tests {
223-
//! Unit tests for HttpSignalingInfo parsing
224-
//!
225-
//! Run these tests with:
226-
//! ```bash
227-
//! cargo test -p p2p signaling_method::http::tests
228-
//! ```
229-
230-
use super::*;
231-
use crate::webrtc::Host;
232-
use std::net::Ipv4Addr;
233-
234-
#[test]
235-
fn test_from_str_valid_domain_and_port() {
236-
let info: HttpSignalingInfo = "example.com/8080".parse().unwrap();
237-
assert_eq!(info.host, Host::Domain("example.com".to_string()));
238-
assert_eq!(info.port, 8080);
239-
}
240-
241-
#[test]
242-
fn test_from_str_valid_domain_and_port_with_leading_slash() {
243-
let info: HttpSignalingInfo = "/example.com/8080".parse().unwrap();
244-
assert_eq!(info.host, Host::Domain("example.com".to_string()));
245-
assert_eq!(info.port, 8080);
246-
}
247-
248-
#[test]
249-
fn test_from_str_valid_ipv4_and_port() {
250-
let info: HttpSignalingInfo = "192.168.1.1/443".parse().unwrap();
251-
assert_eq!(info.host, Host::Ipv4(Ipv4Addr::new(192, 168, 1, 1)));
252-
assert_eq!(info.port, 443);
253-
}
254-
255-
#[test]
256-
fn test_from_str_valid_ipv6_and_port() {
257-
let info: HttpSignalingInfo = "[::1]/8080".parse().unwrap();
258-
assert!(matches!(info.host, Host::Ipv6(_)));
259-
assert_eq!(info.port, 8080);
260-
}
261-
262-
#[test]
263-
fn test_from_str_valid_localhost_and_standard_ports() {
264-
let info: HttpSignalingInfo = "localhost/80".parse().unwrap();
265-
assert_eq!(info.host, Host::Domain("localhost".to_string()));
266-
assert_eq!(info.port, 80);
267-
268-
let info: HttpSignalingInfo = "localhost/443".parse().unwrap();
269-
assert_eq!(info.host, Host::Domain("localhost".to_string()));
270-
assert_eq!(info.port, 443);
271-
}
272-
273-
#[test]
274-
fn test_from_str_valid_high_port_number() {
275-
let info: HttpSignalingInfo = "example.com/65535".parse().unwrap();
276-
assert_eq!(info.host, Host::Domain("example.com".to_string()));
277-
assert_eq!(info.port, 65535);
278-
}
279-
280-
#[test]
281-
fn test_from_str_missing_host() {
282-
let result: Result<HttpSignalingInfo, _> = "/8080".parse();
283-
assert!(result.is_err());
284-
assert!(matches!(
285-
result.unwrap_err(),
286-
SignalingMethodParseError::NotEnoughArgs
287-
));
288-
}
289-
290-
#[test]
291-
fn test_from_str_missing_port() {
292-
let result: Result<HttpSignalingInfo, _> = "example.com".parse();
293-
assert!(result.is_err());
294-
assert!(matches!(
295-
result.unwrap_err(),
296-
SignalingMethodParseError::NotEnoughArgs
297-
));
298-
}
299-
300-
#[test]
301-
fn test_from_str_empty_string() {
302-
let result: Result<HttpSignalingInfo, _> = "".parse();
303-
assert!(result.is_err());
304-
assert!(matches!(
305-
result.unwrap_err(),
306-
SignalingMethodParseError::NotEnoughArgs
307-
));
308-
}
309-
310-
#[test]
311-
fn test_from_str_only_slashes() {
312-
let result: Result<HttpSignalingInfo, _> = "///".parse();
313-
assert!(result.is_err());
314-
assert!(matches!(
315-
result.unwrap_err(),
316-
SignalingMethodParseError::NotEnoughArgs
317-
));
318-
}
319-
320-
#[test]
321-
fn test_from_str_invalid_port_not_number() {
322-
let result: Result<HttpSignalingInfo, _> = "example.com/abc".parse();
323-
assert!(result.is_err());
324-
assert!(matches!(
325-
result.unwrap_err(),
326-
SignalingMethodParseError::PortParseError(_)
327-
));
328-
}
329-
330-
#[test]
331-
fn test_from_str_invalid_port_too_large() {
332-
let result: Result<HttpSignalingInfo, _> = "example.com/99999".parse();
333-
assert!(result.is_err());
334-
assert!(matches!(
335-
result.unwrap_err(),
336-
SignalingMethodParseError::PortParseError(_)
337-
));
338-
}
339-
340-
#[test]
341-
fn test_from_str_invalid_port_negative() {
342-
let result: Result<HttpSignalingInfo, _> = "example.com/-1".parse();
343-
assert!(result.is_err());
344-
assert!(matches!(
345-
result.unwrap_err(),
346-
SignalingMethodParseError::PortParseError(_)
347-
));
348-
}
349-
350-
#[test]
351-
fn test_from_str_invalid_host_empty() {
352-
let result: Result<HttpSignalingInfo, _> = "/8080".parse();
353-
assert!(result.is_err());
354-
assert!(matches!(
355-
result.unwrap_err(),
356-
SignalingMethodParseError::NotEnoughArgs
357-
));
358-
}
359-
360-
#[test]
361-
fn test_from_str_extra_components_ignored() {
362-
// Should only use first two non-empty components
363-
let info: HttpSignalingInfo = "example.com/8080/extra/stuff".parse().unwrap();
364-
assert_eq!(info.host, Host::Domain("example.com".to_string()));
365-
assert_eq!(info.port, 8080);
366-
}
367-
368-
#[test]
369-
fn test_from_str_whitespace_in_components() {
370-
// Components with whitespace should be trimmed by the split filter
371-
let result: Result<HttpSignalingInfo, _> = " / /8080".parse();
372-
assert!(result.is_err());
373-
assert!(matches!(
374-
result.unwrap_err(),
375-
SignalingMethodParseError::NotEnoughArgs
376-
));
377-
}
378-
379-
#[test]
380-
fn test_roundtrip_display_and_from_str() {
381-
let original = HttpSignalingInfo {
382-
host: Host::Domain("signal.example.com".to_string()),
383-
port: 443,
384-
};
385-
386-
let serialized = original.to_string();
387-
let deserialized: HttpSignalingInfo = serialized.parse().unwrap();
388-
389-
assert_eq!(original, deserialized);
390-
}
391-
392-
#[test]
393-
fn test_roundtrip_ipv4() {
394-
let original = HttpSignalingInfo {
395-
host: Host::Ipv4(Ipv4Addr::new(127, 0, 0, 1)),
396-
port: 8080,
397-
};
398-
399-
let serialized = original.to_string();
400-
let deserialized: HttpSignalingInfo = serialized.parse().unwrap();
401-
402-
assert_eq!(original, deserialized);
403-
}
404-
}

0 commit comments

Comments
 (0)