hubcio commented on code in PR #2923: URL: https://github.com/apache/iggy/pull/2923#discussion_r3007861280
########## core/common/src/utils/net.rs: ########## @@ -0,0 +1,291 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +use crate::IggyError; +use std::net::{Ipv4Addr, Ipv6Addr}; + +/// Validates that `addr` is syntactically a valid `host:port` string. +/// Does NOT perform DNS resolution. +/// +/// Accepted formats: +/// - `hostname:port` (e.g. `iggy-server:8090`, `localhost:8090`) +/// - `ipv4:port` (e.g. `127.0.0.1:8090`) +/// - `[ipv6]:port` (e.g. `[::1]:8090`) +/// +/// Rejected formats: +/// - Bare IPv6 without brackets (e.g. `::1:8080`) — ambiguous due to colons +/// - Missing port (e.g. `localhost`) +/// - Invalid port (e.g. `localhost:abc`, `localhost:65536`) +pub fn validate_server_address(addr: &str) -> Result<(), IggyError> { + if let Some(rest) = addr.strip_prefix('[') { + // Bracketed IPv6: "[::1]:port" + let close = rest.find(']').ok_or(IggyError::InvalidIpAddress( + addr.to_string(), + "".to_string(), Review Comment: when port is unknown (e.g., missing closing bracket), `""` as the port field produces error messages with a dangling colon like `Invalid IP address: [::1:`. consider passing the full original `addr` for context, or a placeholder like `<missing>`. also, `String::new()` is idiomatic over `"".to_string()` (zero-alloc). same issue at lines 47, 67. ########## core/common/src/utils/net.rs: ########## @@ -0,0 +1,291 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +use crate::IggyError; +use std::net::{Ipv4Addr, Ipv6Addr}; + +/// Validates that `addr` is syntactically a valid `host:port` string. +/// Does NOT perform DNS resolution. +/// +/// Accepted formats: +/// - `hostname:port` (e.g. `iggy-server:8090`, `localhost:8090`) +/// - `ipv4:port` (e.g. `127.0.0.1:8090`) +/// - `[ipv6]:port` (e.g. `[::1]:8090`) +/// +/// Rejected formats: +/// - Bare IPv6 without brackets (e.g. `::1:8080`) — ambiguous due to colons +/// - Missing port (e.g. `localhost`) +/// - Invalid port (e.g. `localhost:abc`, `localhost:65536`) +pub fn validate_server_address(addr: &str) -> Result<(), IggyError> { + if let Some(rest) = addr.strip_prefix('[') { + // Bracketed IPv6: "[::1]:port" + let close = rest.find(']').ok_or(IggyError::InvalidIpAddress( + addr.to_string(), + "".to_string(), + ))?; + let ipv6_str = &rest[..close]; + let port_str = rest[close + 1..] + .strip_prefix(':') + .ok_or(IggyError::InvalidIpAddress( + ipv6_str.to_string(), + "".to_string(), + ))?; + + // Validate IPv6 address + ipv6_str + .parse::<Ipv6Addr>() + .map_err(|_| IggyError::InvalidIpAddress(ipv6_str.to_string(), port_str.to_string()))?; + + if !port_str.parse::<u16>().is_ok_and(|port| port != 0) { + return Err(IggyError::InvalidIpAddress( + ipv6_str.to_string(), + port_str.to_string(), + )); + } + + return Ok(()); + } + // hostname:port or IPv4:port — rsplit_once to split at last colon + let (host, port_str) = addr.rsplit_once(':').ok_or(IggyError::InvalidIpAddress( + addr.to_string(), + "".to_string(), + ))?; + + // Validate host (IPv4 or hostname with RFC 1123 compliance) + if !is_valid_host(host) { + return Err(IggyError::InvalidIpAddress( + host.to_string(), + port_str.to_string(), + )); + } + + if !port_str.parse::<u16>().is_ok_and(|port| port != 0) { + return Err(IggyError::InvalidIpAddress( + host.to_string(), + port_str.to_string(), + )); + } + + Ok(()) +} + +fn is_valid_hostname(host: &str) -> bool { + if host.is_empty() || host.len() > 253 || host.contains(':') { + return false; + } + + host.split('.').all(|label| { + !label.is_empty() + && label.len() <= 63 + && label + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphanumeric()) + && label + .chars() + .last() + .is_some_and(|c| c.is_ascii_alphanumeric()) + && label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') Review Comment: underscores in hostnames are rejected here (only allows alphanumeric and `-`). while RFC 1123 technically forbids underscores, they are widespread in practice - Docker Compose v1 projects, custom container names, SRV records like `_svc._tcp.example.com`. since the pr's primary goal is enabling hostname-based addresses for container environments, consider allowing underscores (`|| c == '_'`). at minimum, document the restriction. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
