Skip to content

Commit 94bf347

Browse files
committed
Fix test cases
1 parent 1bc53d7 commit 94bf347

File tree

3 files changed

+90
-47
lines changed

3 files changed

+90
-47
lines changed

dsc/tests/dsc_functions.tests.ps1

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -860,25 +860,29 @@ Describe 'tests for function expressions' {
860860
$out.results[0].result.actualState.output | Should -Be $expected
861861
}
862862

863-
It 'uri function works for: <expression>' -TestCases @(
864-
@{ expression = "[uri('https://example.com/', 'path/file.html')]"; expected = 'https://example.com/path/file.html' }
865-
@{ expression = "[uri('https://example.com/', '/path/file.html')]"; expected = 'https://example.com/path/file.html' }
866-
@{ expression = "[uri('https://example.com/api/v1', 'users')]"; expected = 'https://example.com/api/users' }
867-
@{ expression = "[uri('https://example.com/api/v1', '/users')]"; expected = 'https://example.com/api/users' }
868-
@{ expression = "[uri('https://example.com', 'path')]"; expected = 'https://example.com/path' }
869-
@{ expression = "[uri('https://example.com', '/path')]"; expected = 'https://example.com/path' }
870-
@{ expression = "[uri('https://api.example.com/v2/resource/', 'item/123')]"; expected = 'https://api.example.com/v2/resource/item/123' }
871-
@{ expression = "[uri('https://example.com/api/', 'search?q=test')]"; expected = 'https://example.com/api/search?q=test' }
872-
@{ expression = "[uri('https://example.com/', '')]"; expected = 'https://example.com/' }
873-
@{ expression = "[uri('http://example.com/', 'page.html')]"; expected = 'http://example.com/page.html' }
874-
@{ expression = "[uri('https://example.com:8080/', 'api')]"; expected = 'https://example.com:8080/api' }
875-
@{ expression = "[uri(concat('https://example.com', '/'), 'path')]"; expected = 'https://example.com/path' }
876-
@{ expression = "[uri('//example.com/', 'path')]"; expected = '//example.com/path' }
877-
@{ expression = "[uri('https://example.com/', '//foo')]"; expected = 'https://foo/' }
878-
@{ expression = "[uri('https://example.com/a/b/c/', 'd/e/f')]"; expected = 'https://example.com/a/b/c/d/e/f' }
879-
@{ expression = "[uri('https://example.com/old/path', 'new')]"; expected = 'https://example.com/old/new' }
863+
It 'uri function works for: <base> + <relative>' -TestCases @(
864+
@{ base = 'https://example.com/'; relative = 'path/file.html' }
865+
@{ base = 'https://example.com/'; relative = '/path/file.html' }
866+
@{ base = 'https://example.com/api/v1'; relative = 'users' }
867+
@{ base = 'https://example.com/api/v1'; relative = '/users' }
868+
@{ base = 'https://example.com'; relative = 'path' }
869+
@{ base = 'https://example.com'; relative = '/path' }
870+
@{ base = 'https://api.example.com/v2/resource/'; relative = 'item/123' }
871+
@{ base = 'https://example.com/api/'; relative = 'search?q=test' }
872+
@{ base = 'https://example.com/'; relative = '' }
873+
@{ base = 'http://example.com/'; relative = 'page.html' }
874+
@{ base = 'https://example.com:8080/'; relative = 'api' }
875+
@{ base = 'https://example.com/'; relative = '//foo' }
876+
@{ base = 'https://example.com/a/b/c/'; relative = 'd/e/f' }
877+
@{ base = 'https://example.com/old/path'; relative = 'new' }
880878
) {
881-
param($expression, $expected)
879+
param($base, $relative)
880+
881+
$expected = ''
882+
$success = [uri]::TryCreate([uri]$base, [uri]$relative, [ref]$expected)
883+
$success | Should -BeTrue
884+
885+
$expression = "[uri('$base', '$relative')]"
882886
$escapedExpression = $expression -replace "'", "''"
883887
$config_yaml = @"
884888
`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
@@ -889,15 +893,24 @@ Describe 'tests for function expressions' {
889893
output: '$escapedExpression'
890894
"@
891895
$out = $config_yaml | dsc config get -f - | ConvertFrom-Json
892-
$out.results[0].result.actualState.output | Should -Be $expected
896+
$out.results[0].result.actualState.output | Should -BeExactly $expected.AbsoluteUri
893897
}
894898

895-
It 'uri function error handling: <expression>' -TestCases @(
896-
@{ expression = "[uri('', 'path')]" ; expectedError = 'baseUri parameter cannot be empty' }
897-
@{ expression = "[uri('https://example.com/', '///foo')]" ; expectedError = 'Invalid URI|invalid sequence' }
899+
It 'uri function error handling: <base> + <relative>' -TestCases @(
900+
@{ base = ''; relative = 'path'; expectedError = 'baseUri parameter cannot be empty' }
901+
@{ base = 'example.com'; relative = 'path'; expectedError = 'baseUri must be an absolute URI' }
902+
@{ base = '/relative/path'; relative = 'file.txt'; expectedError = 'baseUri must be an absolute URI' }
903+
@{ base = 'https://example.com/'; relative = '///foo'; expectedError = 'Invalid URI|invalid sequence' }
898904
) {
899-
param($expression, $expectedError)
905+
param($base, $relative, $expectedError)
906+
907+
if ($base -ne '') {
908+
$testUri = $null
909+
$dotnetSuccess = [uri]::TryCreate([uri]$base, [uri]$relative, [ref]$testUri)
910+
$dotnetSuccess | Should -BeFalse -Because ".NET Uri.TryCreate should also fail for base='$base' relative='$relative'"
911+
}
900912

913+
$expression = "[uri('$base', '$relative')]"
901914
$config_yaml = @"
902915
`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
903916
resources:

lib/dsc-lib/locales/en-us.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,7 @@ invoked = "uniqueString function"
531531
[functions.uri]
532532
description = "Creates an absolute URI by combining the baseUri and the relativeUri string"
533533
emptyBaseUri = "The baseUri parameter cannot be empty. An absolute URI requires a valid base URI."
534+
notAbsoluteUri = "The baseUri must be an absolute URI (must include a scheme such as http:// or https://)."
534535
invalidRelativeUri = "Invalid URI: The relative URI contains an invalid sequence. Three or more consecutive slashes (///) are not allowed."
535536

536537
[functions.uriComponent]

lib/dsc-lib/src/functions/uri.rs

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ fn combine_uri(base_uri: &str, relative_uri: &str) -> Result<String, DscError> {
4141
if base_uri.is_empty() {
4242
return Err(DscError::Parser(t!("functions.uri.emptyBaseUri").to_string()));
4343
}
44+
45+
if !base_uri.contains("://") {
46+
return Err(DscError::Parser(t!("functions.uri.notAbsoluteUri").to_string()));
47+
}
48+
4449
if relative_uri.is_empty() {
4550
return Ok(base_uri.to_string());
4651
}
@@ -66,20 +71,9 @@ fn combine_uri(base_uri: &str, relative_uri: &str) -> Result<String, DscError> {
6671
return Ok(format!("{scheme}:{relative_uri}/"));
6772
}
6873

69-
let base_ends_with_slash = base_uri.ends_with('/');
7074
let relative_starts_with_slash = relative_uri.starts_with('/');
7175

72-
// Case 1: baseUri ends with trailing slash
73-
if base_ends_with_slash {
74-
if relative_starts_with_slash {
75-
// Combine trailing and leading slash into one
76-
return Ok(format!("{base_uri}{}", &relative_uri[1..]));
77-
}
78-
return Ok(format!("{base_uri}{relative_uri}"));
79-
}
80-
81-
// Case 2: baseUri doesn't end with trailing slash
82-
// Check if baseUri has slashes (aside from // near the front)
76+
// Extract scheme and host from base URI
8377
let scheme_end = if base_uri.starts_with("http://") || base_uri.starts_with("https://") {
8478
base_uri.find("://").map_or(0, |pos| pos + 3)
8579
} else if base_uri.starts_with("//") {
@@ -88,23 +82,39 @@ fn combine_uri(base_uri: &str, relative_uri: &str) -> Result<String, DscError> {
8882
0
8983
};
9084

85+
// If relative URI starts with '/', it replaces the entire path of the base URI
86+
// Keep only scheme://host and append the relative URI
87+
if relative_starts_with_slash {
88+
let after_scheme = &base_uri[scheme_end..];
89+
if let Some(first_slash_pos) = after_scheme.find('/') {
90+
// Base has a path, extract scheme://host only
91+
let scheme_and_host = &base_uri[..scheme_end + first_slash_pos];
92+
return Ok(format!("{scheme_and_host}{relative_uri}"));
93+
}
94+
// Base has no path (e.g., "https://example.com"), just append
95+
return Ok(format!("{base_uri}{relative_uri}"));
96+
}
97+
98+
// Relative URI doesn't start with '/'
99+
// Standard behavior: resolve relative to base
100+
let base_ends_with_slash = base_uri.ends_with('/');
101+
102+
if base_ends_with_slash {
103+
// Base ends with '/', just append
104+
return Ok(format!("{base_uri}{relative_uri}"));
105+
}
106+
107+
// Base doesn't end with '/', need to remove last segment
91108
let after_scheme = &base_uri[scheme_end..];
92109

93110
if let Some(last_slash_pos) = after_scheme.rfind('/') {
111+
// Base has a path with segments, remove last segment
94112
let base_without_last_segment = &base_uri[..=(scheme_end + last_slash_pos)];
95-
if relative_starts_with_slash {
96-
Ok(format!("{}{relative_uri}", &base_without_last_segment[..base_without_last_segment.len() - 1]))
97-
} else {
98-
Ok(format!("{base_without_last_segment}{relative_uri}"))
99-
}
113+
Ok(format!("{base_without_last_segment}{relative_uri}"))
100114
} else {
101115
// No path after scheme (e.g., "https://example.com")
102-
// .NET Uri adds a '/' between host and relative URI
103-
if relative_starts_with_slash {
104-
Ok(format!("{base_uri}{relative_uri}"))
105-
} else {
106-
Ok(format!("{base_uri}/{relative_uri}"))
107-
}
116+
// Add a '/' between host and relative URI
117+
Ok(format!("{base_uri}/{relative_uri}"))
108118
}
109119
}
110120

@@ -138,7 +148,8 @@ mod tests {
138148
fn test_uri_no_trailing_slash_with_leading_slash() {
139149
let mut parser = Statement::new().unwrap();
140150
let result = parser.parse_and_execute("[uri('https://example.com/api/v1', '/users')]", &Context::new()).unwrap();
141-
assert_eq!(result, "https://example.com/api/users");
151+
// When relative starts with '/', it replaces the entire path
152+
assert_eq!(result, "https://example.com/users");
142153
}
143154

144155
#[test]
@@ -243,4 +254,22 @@ mod tests {
243254
let result = parser.parse_and_execute("[uri('https://example.com/', '//foo')]", &Context::new()).unwrap();
244255
assert_eq!(result, "https://foo/");
245256
}
257+
258+
#[test]
259+
fn test_uri_not_absolute_no_scheme() {
260+
let mut parser = Statement::new().unwrap();
261+
let result = parser.parse_and_execute("[uri('example.com', 'path')]", &Context::new());
262+
assert!(result.is_err());
263+
let err = result.unwrap_err();
264+
assert!(err.to_string().contains("absolute"));
265+
}
266+
267+
#[test]
268+
fn test_uri_not_absolute_relative_path() {
269+
let mut parser = Statement::new().unwrap();
270+
let result = parser.parse_and_execute("[uri('/relative/path', 'file.txt')]", &Context::new());
271+
assert!(result.is_err());
272+
let err = result.unwrap_err();
273+
assert!(err.to_string().contains("absolute"));
274+
}
246275
}

0 commit comments

Comments
 (0)