Skip to content

Commit 4d61b89

Browse files
CopilotByron
andcommitted
Address code review feedback - fix Windows test and improve validation
Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
1 parent a8ac275 commit 4d61b89

File tree

2 files changed

+14
-8
lines changed

2 files changed

+14
-8
lines changed

gix-url/src/simple_url.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ pub enum ParseError {
2121
InvalidDomainCharacter,
2222
}
2323

24+
/// Check if a character is valid in a URL scheme.
25+
/// Valid scheme characters: alphanumeric, +, -, or .
26+
fn is_valid_scheme_char(c: char) -> bool {
27+
c.is_ascii_alphanumeric() || c == '+' || c == '-' || c == '.'
28+
}
29+
2430
impl<'a> ParsedUrl<'a> {
2531
/// Parse a URL string into its components.
2632
/// Expected format: scheme://[user[:password]@]host[:port]/path
@@ -45,8 +51,8 @@ impl<'a> ParsedUrl<'a> {
4551
return Err(ParseError::RelativeUrlWithoutBase);
4652
}
4753

48-
// Validate scheme characters - schemes should only contain alphanumeric, +, -, or .
49-
if !scheme.chars().all(|c| c.is_ascii_alphanumeric() || c == '+' || c == '-' || c == '.') {
54+
// Validate scheme characters
55+
if !scheme.chars().all(is_valid_scheme_char) {
5056
return Err(ParseError::RelativeUrlWithoutBase);
5157
}
5258

@@ -138,6 +144,10 @@ impl<'a> ParsedUrl<'a> {
138144
let port = potential_port
139145
.parse::<u16>()
140146
.map_err(|_| ParseError::InvalidPort)?;
147+
// Validate port is in valid range (1-65535, port 0 is invalid)
148+
if port == 0 {
149+
return Err(ParseError::InvalidPort);
150+
}
141151
return Ok((Some(host), Some(port)));
142152
}
143153
}

gix-url/tests/url/parse/file.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,9 @@ mod windows {
141141

142142
#[test]
143143
fn url_from_absolute_path() -> crate::Result {
144+
// Test with a Windows path directly instead of using url::Url::from_directory_path
144145
assert_url(
145-
url::Url::from_directory_path(r"c:\users\1")
146-
.expect("valid")
147-
.to_file_path()
148-
.expect("valid path")
149-
.to_string_lossy()
150-
.as_ref(),
146+
r"C:\users\1\",
151147
url_alternate(Scheme::File, None, None, None, br"C:\users\1\"),
152148
)?;
153149
// A special hack to support URLs on windows that are prefixed with `/` even though absolute.

0 commit comments

Comments
 (0)