From 269364168955bd85ccb4f5965c5934334e5863ef Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Thu, 20 Nov 2025 12:48:36 +0000 Subject: [PATCH 01/10] Add url package with helpers for url operations --- utils/url/url.go | 139 ++++++++++++++++++++++++++++++++++++++++++ utils/url/url_test.go | 17 ++++++ 2 files changed, 156 insertions(+) create mode 100644 utils/url/url.go create mode 100644 utils/url/url_test.go diff --git a/utils/url/url.go b/utils/url/url.go new file mode 100644 index 0000000000..7f56e96b1e --- /dev/null +++ b/utils/url/url.go @@ -0,0 +1,139 @@ +package url + +import ( + "strings" + + "github.com/ARM-software/golang-utils/utils/commonerrors" + "github.com/ARM-software/golang-utils/utils/reflection" +) + +// HasMatchingPathSegments checks whether two path strings match based on their segments. +func HasMatchingPathSegments(pathA, pathB string) (match bool, err error) { + if reflection.IsEmpty(pathA) { + err = commonerrors.UndefinedVariable("pathA") + return + } + + pathASegments := SplitPath(pathA) + pathBSegments := SplitPath(pathB) + if len(pathASegments) != len(pathBSegments) { + return + } + + for i := range pathBSegments { + pathBSeg, pathASeg := pathBSegments[i], pathASegments[i] + if pathBSeg != pathASeg { + return + } + } + + match = true + return +} + +// HasMatchingPathSegmentsWithParams is similar to MatchingPathSegments but considers segments as matching if at least one of them contains a path parameter. +// +// HasMatchingPathSegmentsWithParams("/some/{param}/path", "/some/{param}/path") // true +// HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/{param}/path") // true +// HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/def/path") // false +func HasMatchingPathSegmentsWithParams(pathA, pathB string) (match bool, err error) { + if reflection.IsEmpty(pathA) { + err = commonerrors.UndefinedVariable("pathA") + return + } + + pathASegments := SplitPath(pathA) + pathBSegments := SplitPath(pathB) + if len(pathASegments) != len(pathBSegments) { + return + } + + for i := range pathBSegments { + pathBSeg, pathASeg := pathBSegments[i], pathASegments[i] + if IsParamSegment(pathASeg) { + if pathBSeg == "" { + return + } + continue + } + + if IsParamSegment(pathBSeg) { + if pathASeg == "" { + return + } + continue + } + if pathBSeg != pathASeg { + return + } + } + + match = true + return +} + +// SplitPath returns a slice of the individual segments that make up the path string. It looks for the default "/" path separator when splitting. +func SplitPath(path string) []string { + return SplitPathWithSeparator(path, "/") +} + +// SplitPathWithSeparator is similar to SplitPath but allows for specifying the path separator to look for when splitting. +func SplitPathWithSeparator(path string, separator string) []string { + path = strings.TrimSpace(path) + if path == "" || path == separator { + return nil + } + + path = strings.Trim(path, separator) + segments := strings.Split(path, separator) + out := segments[:0] + for _, p := range segments { + if p != "" { + out = append(out, p) + } + } + return out +} + +// IsParamSegment checks whether the segment string is a path parameter +func IsParamSegment(segment string) bool { + return len(segment) >= 2 && segment[0] == '{' && segment[len(segment)-1] == '}' +} + +// JoinPaths returns a single concatenated path string from the supplied paths and correctly sets the default "/" separator between them. +func JoinPaths(paths ...string) (joinedPath string, err error) { + return JoinPathsWithSeparator("/", paths...) +} + +// JoinPathsWithSeparator is similar to JoinPaths but allows for specifying the path separator to use. +func JoinPathsWithSeparator(separator string, paths ...string) (joinedPath string, err error) { + if paths == nil { + err = commonerrors.UndefinedVariable("paths") + return + } + if len(paths) == 0 { + return + } + + if len(paths) == 1 { + joinedPath = paths[0] + return + } + + joinedPath = paths[0] + for _, p := range paths[1:] { + pathAHasSlashSuffix := strings.HasSuffix(joinedPath, separator) + pathBHasSlashPrefix := strings.HasPrefix(p, separator) + + switch { + case pathAHasSlashSuffix && pathBHasSlashPrefix: + joinedPath += p[1:] + case !pathAHasSlashSuffix && !pathBHasSlashPrefix: + joinedPath += separator + p + default: + joinedPath += p + } + } + + return +} diff --git a/utils/url/url_test.go b/utils/url/url_test.go new file mode 100644 index 0000000000..e26692dfb4 --- /dev/null +++ b/utils/url/url_test.go @@ -0,0 +1,17 @@ +package url + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestUrl_IsParamSegment(t *testing.T) { + t.Run("true", func(t *testing.T) { + assert.True(t, IsParamSegment("{abc}")) + }) + + t.Run("false", func(t *testing.T) { + assert.False(t, IsParamSegment("abc")) + }) +} From ca7704b4f6f88de3a504c800426af76fee69d4c0 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Thu, 20 Nov 2025 16:34:50 +0000 Subject: [PATCH 02/10] Refactor url functions and add tests --- utils/url/url.go | 17 ++- utils/url/url_test.go | 259 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 6 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index 7f56e96b1e..ccff978256 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -7,6 +7,8 @@ import ( "github.com/ARM-software/golang-utils/utils/reflection" ) +const defaultPathSeparator = "/" + // HasMatchingPathSegments checks whether two path strings match based on their segments. func HasMatchingPathSegments(pathA, pathB string) (match bool, err error) { if reflection.IsEmpty(pathA) { @@ -72,15 +74,15 @@ func HasMatchingPathSegmentsWithParams(pathA, pathB string) (match bool, err err return } -// SplitPath returns a slice of the individual segments that make up the path string. It looks for the default "/" path separator when splitting. +// SplitPath returns a slice containing the individual segments that make up the path string. It looks for the default "/" path separator when splitting. func SplitPath(path string) []string { - return SplitPathWithSeparator(path, "/") + return SplitPathWithSeparator(path, defaultPathSeparator) } // SplitPathWithSeparator is similar to SplitPath but allows for specifying the path separator to look for when splitting. func SplitPathWithSeparator(path string, separator string) []string { path = strings.TrimSpace(path) - if path == "" || path == separator { + if reflection.IsEmpty(path) || path == separator { return nil } @@ -88,7 +90,7 @@ func SplitPathWithSeparator(path string, separator string) []string { segments := strings.Split(path, separator) out := segments[:0] for _, p := range segments { - if p != "" { + if !reflection.IsEmpty(p) { out = append(out, p) } } @@ -102,7 +104,7 @@ func IsParamSegment(segment string) bool { // JoinPaths returns a single concatenated path string from the supplied paths and correctly sets the default "/" separator between them. func JoinPaths(paths ...string) (joinedPath string, err error) { - return JoinPathsWithSeparator("/", paths...) + return JoinPathsWithSeparator(defaultPathSeparator, paths...) } // JoinPathsWithSeparator is similar to JoinPaths but allows for specifying the path separator to use. @@ -114,12 +116,15 @@ func JoinPathsWithSeparator(separator string, paths ...string) (joinedPath strin if len(paths) == 0 { return } - if len(paths) == 1 { joinedPath = paths[0] return } + if reflection.IsEmpty(separator) { + separator = defaultPathSeparator + } + joinedPath = paths[0] for _, p := range paths[1:] { pathAHasSlashSuffix := strings.HasSuffix(joinedPath, separator) diff --git a/utils/url/url_test.go b/utils/url/url_test.go index e26692dfb4..9fb426cb2a 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -1,8 +1,11 @@ package url import ( + "fmt" "testing" + "github.com/ARM-software/golang-utils/utils/commonerrors" + "github.com/ARM-software/golang-utils/utils/commonerrors/errortest" "github.com/stretchr/testify/assert" ) @@ -15,3 +18,259 @@ func TestUrl_IsParamSegment(t *testing.T) { assert.False(t, IsParamSegment("abc")) }) } + +func TestUrl_JoinPaths(t *testing.T) { + tests := []struct { + name string + paths []string + result string + error error + }{ + { + "empty paths", + []string{}, + "", + nil, + }, + { + "undefined paths", + nil, + "", + commonerrors.ErrUndefined, + }, + { + "one path", + []string{"abc/123"}, + "abc/123", + nil, + }, + { + "two paths", + []string{"abc/123", "def/456"}, + "abc/123/def/456", + nil, + }, + { + "two paths with leading and trailing slashes", + []string{"abc/123/", "/def/456"}, + "abc/123/def/456", + nil, + }, + { + "multiple paths", + []string{"abc/123", "def/456", "zzz", "{param1}", "999"}, + "abc/123/def/456/zzz/{param1}/999", + nil, + }, + } + + for i := range tests { + test := tests[i] + t.Run(fmt.Sprintf("JoinPaths_%v", test.name), func(t *testing.T) { + joinedPaths, err := JoinPaths(test.paths...) + + errortest.AssertError(t, err, test.error) + assert.Equal(t, test.result, joinedPaths) + }) + } +} + +func TestUrl_JoinPathsWithSeparator(t *testing.T) { + tests := []struct { + name string + paths []string + separator string + result string + error error + }{ + { + "empty paths", + []string{}, + "/", + "", + nil, + }, + { + "undefined paths", + nil, + "~", + "", + commonerrors.ErrUndefined, + }, + { + "one path with custom separator", + []string{"abc/123"}, + "|", + "abc/123", + nil, + }, + { + "two paths with custom separator", + []string{"abc/123", "def/456"}, + "|", + "abc/123|def/456", + nil, + }, + { + "two paths with empty separator", + []string{"abc/123", "def/456"}, + "", + "abc/123/def/456", + nil, + }, + { + "two paths with whitespace separator", + []string{"abc/123", "def/456"}, + " ", + "abc/123/def/456", + nil, + }, + { + "two paths with leading and trailing slashes and custom separator", + []string{"abc/123/", "/def/456"}, + "#", + "abc/123/#/def/456", + nil, + }, + { + "multiple paths with custom separator", + []string{"abc/123", "def/456", "zzz", "{param1}", "999"}, + "~", + "abc/123~def/456~zzz~{param1}~999", + nil, + }, + } + + for i := range tests { + test := tests[i] + t.Run(fmt.Sprintf("JoinPathsWithSeparator_%v", test.name), func(t *testing.T) { + joinedPaths, err := JoinPathsWithSeparator(test.separator, test.paths...) + + errortest.AssertError(t, err, test.error) + assert.Equal(t, test.result, joinedPaths) + }) + } +} + +func TestUrl_SplitPath(t *testing.T) { + tests := []struct { + name string + path string + result []string + error error + }{ + { + "empty path", + "", + []string{}, + nil, + }, + { + "path with one segment", + "abc", + []string{"abc"}, + nil, + }, + { + "path with two segments", + "abc/123", + []string{"abc", "123"}, + nil, + }, + { + "path with multiple segments", + "abc/123/def/456", + []string{"abc", "123", "def", "456"}, + nil, + }, + { + "path with multiple segments including param segment", + "abc/123/def/456/zzz/{param1}/999", + []string{"abc", "123", "def", "456", "zzz", "{param1}", "999"}, + nil, + }, + } + + for i := range tests { + test := tests[i] + t.Run(fmt.Sprintf("JoinPaths_%v", test.name), func(t *testing.T) { + segments := SplitPath(test.path) + + if test.result != nil { + for i, s := range segments { + assert.Equal(t, test.result[i], s) + } + } else { + assert.Nil(t, segments) + } + }) + } +} + +func TestUrl_SplitPathWithSeparator(t *testing.T) { + tests := []struct { + name string + path string + separator string + result []string + error error + }{ + { + "empty path", + "", + "", + []string{}, + nil, + }, + { + "path with one segment and custom separator", + "abc", + "|", + []string{"abc"}, + nil, + }, + { + "path with two segments and custom separator", + "abc|123", + "|", + []string{"abc", "123"}, + nil, + }, + { + "path with multiple segments and custom separator", + "abc~123~def~456", + "~", + []string{"abc", "123", "def", "456"}, + nil, + }, + { + "path with multiple segments including param segment and custom separator", + "abc~123~def~456~zzz~{param1}~999", + "~", + []string{"abc", "123", "def", "456", "zzz", "{param1}", "999"}, + nil, + }, + { + "path with multiple segments including param segment and custom separator with other separators", + "abc~123/def~456~zzz~{param1}|999", + "~", + []string{"abc", "123/def", "456", "zzz", "{param1}|999"}, + nil, + }, + } + + for i := range tests { + test := tests[i] + t.Run(fmt.Sprintf("JoinPaths_%v", test.name), func(t *testing.T) { + segments := SplitPathWithSeparator(test.path, test.separator) + + if test.result != nil { + for i, s := range segments { + assert.Equal(t, test.result[i], s) + } + } else { + assert.Nil(t, segments) + } + }) + } +} From 3ddbc3d00df232b43f0f0f2db5b90732ad07cad7 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Mon, 24 Nov 2025 11:37:59 +0000 Subject: [PATCH 03/10] Refactor matching path segment functions and add tests --- utils/url/url.go | 80 +++++++++----------- utils/url/url_test.go | 170 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 201 insertions(+), 49 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index ccff978256..d6c4d79294 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -9,38 +9,49 @@ import ( const defaultPathSeparator = "/" -// HasMatchingPathSegments checks whether two path strings match based on their segments. -func HasMatchingPathSegments(pathA, pathB string) (match bool, err error) { - if reflection.IsEmpty(pathA) { - err = commonerrors.UndefinedVariable("pathA") - return - } - - pathASegments := SplitPath(pathA) - pathBSegments := SplitPath(pathB) - if len(pathASegments) != len(pathBSegments) { - return - } - - for i := range pathBSegments { - pathBSeg, pathASeg := pathBSegments[i], pathASegments[i] - if pathBSeg != pathASeg { - return - } - } +// IsParamSegment checks whether the segment string is a path parameter +func IsParamSegment(segment string) bool { + return len(segment) >= 2 && segment[0] == '{' && segment[len(segment)-1] == '}' +} - match = true - return +// HasMatchingPathSegments checks whether two path strings match based on their segments. +func HasMatchingPathSegments(pathA, pathB string) (bool, error) { + return hasMatchingPathSegments(pathA, pathB, func(segmentA, segmentB string) bool { + return segmentA == segmentB + }) } -// HasMatchingPathSegmentsWithParams is similar to MatchingPathSegments but considers segments as matching if at least one of them contains a path parameter. +// HasMatchingPathSegmentsWithParams is similar to HasMatchingPathSegments but also considers segments as matching if at least one of them contains a path parameter. // // HasMatchingPathSegmentsWithParams("/some/{param}/path", "/some/{param}/path") // true // HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/{param}/path") // true // HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/def/path") // false -func HasMatchingPathSegmentsWithParams(pathA, pathB string) (match bool, err error) { +func HasMatchingPathSegmentsWithParams(pathA, pathB string) (bool, error) { + return hasMatchingPathSegments(pathA, pathB, func(pathASeg, pathBSeg string) bool { + switch { + case IsParamSegment(pathASeg): + return pathBSeg != "" + case IsParamSegment(pathBSeg): + return pathASeg != "" + default: + return pathASeg == pathBSeg + } + }) +} + +func hasMatchingPathSegments(pathA, pathB string, segmentMatcherFn func(segmentA, segmentB string) bool) (match bool, err error) { if reflection.IsEmpty(pathA) { - err = commonerrors.UndefinedVariable("pathA") + err = commonerrors.UndefinedVariable("path A") + return + } + + if reflection.IsEmpty(pathB) { + err = commonerrors.UndefinedVariable("path B") + return + } + + if segmentMatcherFn == nil { + err = commonerrors.UndefinedVariable("segment matcher function") return } @@ -51,21 +62,7 @@ func HasMatchingPathSegmentsWithParams(pathA, pathB string) (match bool, err err } for i := range pathBSegments { - pathBSeg, pathASeg := pathBSegments[i], pathASegments[i] - if IsParamSegment(pathASeg) { - if pathBSeg == "" { - return - } - continue - } - - if IsParamSegment(pathBSeg) { - if pathASeg == "" { - return - } - continue - } - if pathBSeg != pathASeg { + if !segmentMatcherFn(pathASegments[i], pathBSegments[i]) { return } } @@ -97,11 +94,6 @@ func SplitPathWithSeparator(path string, separator string) []string { return out } -// IsParamSegment checks whether the segment string is a path parameter -func IsParamSegment(segment string) bool { - return len(segment) >= 2 && segment[0] == '{' && segment[len(segment)-1] == '}' -} - // JoinPaths returns a single concatenated path string from the supplied paths and correctly sets the default "/" separator between them. func JoinPaths(paths ...string) (joinedPath string, err error) { return JoinPathsWithSeparator(defaultPathSeparator, paths...) diff --git a/utils/url/url_test.go b/utils/url/url_test.go index 9fb426cb2a..7a7cca9ce7 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -1,7 +1,6 @@ package url import ( - "fmt" "testing" "github.com/ARM-software/golang-utils/utils/commonerrors" @@ -19,6 +18,167 @@ func TestUrl_IsParamSegment(t *testing.T) { }) } +func TestUrl_HasMatchingPathSegments(t *testing.T) { + tests := []struct { + name string + pathA string + pathB string + result bool + err error + }{ + { + "empty pathA", + "", + "abc/123", + false, + commonerrors.ErrUndefined, + }, + { + "empty pathB", + "abc/123", + "", + false, + commonerrors.ErrUndefined, + }, + { + "identical paths", + "abc/123", + "abc/123", + true, + nil, + }, + { + "identical paths with multiple segments", + "abc/123/def/456/zzz", + "abc/123/def/456/zzz", + true, + nil, + }, + { + "root paths", + "/", + "/", + true, + nil, + }, + { + "paths with different segment values", + "abc/123", + "abc/456", + false, + nil, + }, + { + "paths with different lengths", + "abc/123", + "abc/123/456", + false, + nil, + }, + { + "path with trailing slashes", + "/abc/123/", + "abc/123", + true, + nil, + }, + } + + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + match, err := HasMatchingPathSegments(test.pathA, test.pathB) + + errortest.AssertError(t, err, test.err) + assert.Equal(t, test.result, match) + }) + } +} + +func TestUrl_HasMatchingPathSegmentsWithParams(t *testing.T) { + tests := []struct { + name string + pathA string + pathB string + result bool + err error + }{ + { + "empty pathA", + "", + "abc/123", + false, + commonerrors.ErrUndefined, + }, + { + "empty pathB", + "abc/123", + "", + false, + commonerrors.ErrUndefined, + }, + { + "identical paths", + "abc/123", + "abc/123", + true, + nil, + }, + { + "identical paths with multiple segments", + "abc/123/def/456/zzz", + "abc/123/def/456/zzz", + true, + nil, + }, + { + "path with parameter segment", + "/abc/{id}/123", + "/abc/123/123", + true, + nil, + }, + { + "both paths with matching parameter segments", + "/abc/{param}/123", + "/abc/{param}/123", + true, + nil, + }, + { + "paths with different segments", + "/abc/123/xyz", + "/def/123/zzz", + false, + nil, + }, + { + "paths with different segments with parameter", + "/abc/{param}/123", + "/def/123/zzz", + false, + nil, + }, + { + "paths with different lengths and params", + "/abc/{param}", + "/abc/{param}/123", + false, + nil, + }, + } + + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + match, err := HasMatchingPathSegmentsWithParams(test.pathA, test.pathB) + + errortest.AssertError(t, err, test.err) + assert.Equal(t, test.result, match) + }) + } +} + func TestUrl_JoinPaths(t *testing.T) { tests := []struct { name string @@ -66,7 +226,7 @@ func TestUrl_JoinPaths(t *testing.T) { for i := range tests { test := tests[i] - t.Run(fmt.Sprintf("JoinPaths_%v", test.name), func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { joinedPaths, err := JoinPaths(test.paths...) errortest.AssertError(t, err, test.error) @@ -143,7 +303,7 @@ func TestUrl_JoinPathsWithSeparator(t *testing.T) { for i := range tests { test := tests[i] - t.Run(fmt.Sprintf("JoinPathsWithSeparator_%v", test.name), func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { joinedPaths, err := JoinPathsWithSeparator(test.separator, test.paths...) errortest.AssertError(t, err, test.error) @@ -193,7 +353,7 @@ func TestUrl_SplitPath(t *testing.T) { for i := range tests { test := tests[i] - t.Run(fmt.Sprintf("JoinPaths_%v", test.name), func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { segments := SplitPath(test.path) if test.result != nil { @@ -261,7 +421,7 @@ func TestUrl_SplitPathWithSeparator(t *testing.T) { for i := range tests { test := tests[i] - t.Run(fmt.Sprintf("JoinPaths_%v", test.name), func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { segments := SplitPathWithSeparator(test.path, test.separator) if test.result != nil { From ac9800f3e7cdf527d2c595b3d7090b1b5d1429fa Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Mon, 24 Nov 2025 11:39:46 +0000 Subject: [PATCH 04/10] Add news file --- changes/20251124113919.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/20251124113919.feature diff --git a/changes/20251124113919.feature b/changes/20251124113919.feature new file mode 100644 index 0000000000..9772d95fcc --- /dev/null +++ b/changes/20251124113919.feature @@ -0,0 +1 @@ +:sparkles: `[url]` Add a new package containing url helper functions From c006ec6d395267d14f13beda758df971b3932487 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Mon, 24 Nov 2025 11:56:11 +0000 Subject: [PATCH 05/10] Fix linting --- utils/url/url_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/url/url_test.go b/utils/url/url_test.go index 7a7cca9ce7..36701e8930 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -3,9 +3,10 @@ package url import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/ARM-software/golang-utils/utils/commonerrors" "github.com/ARM-software/golang-utils/utils/commonerrors/errortest" - "github.com/stretchr/testify/assert" ) func TestUrl_IsParamSegment(t *testing.T) { From aef1477c63b598e9f7653ae9795abfbf2bcce06a Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Mon, 24 Nov 2025 17:22:22 +0000 Subject: [PATCH 06/10] PR suggestions --- utils/url/url.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index d6c4d79294..eb589308d5 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -9,14 +9,17 @@ import ( const defaultPathSeparator = "/" -// IsParamSegment checks whether the segment string is a path parameter +// The expected function signature for checking whether two path segments match. +type PathSegmentMatcherFunc = func(segmentA, segmentB string) bool + +// IsParamSegment checks whether the segment string is a path parameter as described by the OpenAPI spec (see https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.0.md#path-templating). func IsParamSegment(segment string) bool { - return len(segment) >= 2 && segment[0] == '{' && segment[len(segment)-1] == '}' + return len(segment) >= 2 && strings.HasPrefix(segment, "{") && strings.HasSuffix(segment, "}") } // HasMatchingPathSegments checks whether two path strings match based on their segments. func HasMatchingPathSegments(pathA, pathB string) (bool, error) { - return hasMatchingPathSegments(pathA, pathB, func(segmentA, segmentB string) bool { + return MatchingPathSegments(pathA, pathB, func(segmentA, segmentB string) bool { return segmentA == segmentB }) } @@ -27,19 +30,20 @@ func HasMatchingPathSegments(pathA, pathB string) (bool, error) { // HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/{param}/path") // true // HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/def/path") // false func HasMatchingPathSegmentsWithParams(pathA, pathB string) (bool, error) { - return hasMatchingPathSegments(pathA, pathB, func(pathASeg, pathBSeg string) bool { + return MatchingPathSegments(pathA, pathB, func(pathASeg, pathBSeg string) bool { switch { case IsParamSegment(pathASeg): - return pathBSeg != "" + return !reflection.IsEmpty(pathBSeg) case IsParamSegment(pathBSeg): - return pathASeg != "" + return !reflection.IsEmpty(pathASeg) default: return pathASeg == pathBSeg } }) } -func hasMatchingPathSegments(pathA, pathB string, segmentMatcherFn func(segmentA, segmentB string) bool) (match bool, err error) { +// MatchingPathSegments checks whether two path strings match based on their segments using the provided matcher function. +func MatchingPathSegments(pathA, pathB string, matcherFn PathSegmentMatcherFunc) (match bool, err error) { if reflection.IsEmpty(pathA) { err = commonerrors.UndefinedVariable("path A") return @@ -50,7 +54,7 @@ func hasMatchingPathSegments(pathA, pathB string, segmentMatcherFn func(segmentA return } - if segmentMatcherFn == nil { + if matcherFn == nil { err = commonerrors.UndefinedVariable("segment matcher function") return } @@ -62,7 +66,7 @@ func hasMatchingPathSegments(pathA, pathB string, segmentMatcherFn func(segmentA } for i := range pathBSegments { - if !segmentMatcherFn(pathASegments[i], pathBSegments[i]) { + if !matcherFn(pathASegments[i], pathBSegments[i]) { return } } From d88f2c52f4f24c9f2d6f4ef353f23c5cb91d72d9 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Wed, 26 Nov 2025 11:02:43 +0000 Subject: [PATCH 07/10] Refactor url helper functions --- utils/url/url.go | 180 ++++++++++-------- utils/url/url_test.go | 425 ++++++++++++++++++++---------------------- 2 files changed, 300 insertions(+), 305 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index eb589308d5..0c7d442c6a 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -1,27 +1,52 @@ package url import ( + netUrl "net/url" + "path" + "regexp" "strings" "github.com/ARM-software/golang-utils/utils/commonerrors" "github.com/ARM-software/golang-utils/utils/reflection" ) -const defaultPathSeparator = "/" +const ( + defaultPathSeparator = "/" + minimumPathParameterLength = 3 +) + +var validParamRegex = regexp.MustCompile(`^\{[A-Za-z0-9_-]+\}$`) + +// The expected signature for path segment matcher functions. +type PathSegmentMatcherFunc = func(segmentA, segmentB string) (match bool, err error) + +// ValidatePathParameter checks whether a path parameter is valid. An error is returned if it is invalid. +// Version 3.1.0 of the OpenAPI spec provides some guidance for path parameter values (see https://spec.openapis.org/oas/v3.1.0.html#path-templating) +func ValidatePathParameter(parameter string) error { + if reflection.IsEmpty(parameter) || len(parameter) < minimumPathParameterLength { + return commonerrors.Newf(commonerrors.ErrInvalid, "parameter segment %q must have length greater than or equal to three", parameter) + } + + unescapedSegment, err := netUrl.PathUnescape(parameter) + if err != nil { + return commonerrors.WrapErrorf(commonerrors.ErrInvalid, err, "an error occurred during path unescaping for parameter %q", parameter) + } -// The expected function signature for checking whether two path segments match. -type PathSegmentMatcherFunc = func(segmentA, segmentB string) bool + if !validParamRegex.MatchString(unescapedSegment) { + return commonerrors.Newf(commonerrors.ErrInvalid, "parameter %q unescaped to %q can only contain alphanumeric characters, dashes, underscores, and a single pair of braces", parameter, unescapedSegment) + } + + return nil +} -// IsParamSegment checks whether the segment string is a path parameter as described by the OpenAPI spec (see https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.0.md#path-templating). -func IsParamSegment(segment string) bool { - return len(segment) >= 2 && strings.HasPrefix(segment, "{") && strings.HasSuffix(segment, "}") +// IsPathParameter checks whether the parameter string is a path parameter as described by the OpenAPI spec (see https://spec.openapis.org/oas/v3.0.0.html#path-templating). +func IsPathParameter(parameter string) bool { + return !reflection.IsEmpty(parameter) && strings.HasPrefix(parameter, "{") && strings.HasSuffix(parameter, "}") } -// HasMatchingPathSegments checks whether two path strings match based on their segments. -func HasMatchingPathSegments(pathA, pathB string) (bool, error) { - return MatchingPathSegments(pathA, pathB, func(segmentA, segmentB string) bool { - return segmentA == segmentB - }) +// HasMatchingPathSegments checks whether two path strings match based on their segments by doing a simple equality check on each path segment pair. +func HasMatchingPathSegments(pathA, pathB string) (match bool, err error) { + return MatchingPathSegments(pathA, pathB, BasicEqualityPathSegmentMatcher) } // HasMatchingPathSegmentsWithParams is similar to HasMatchingPathSegments but also considers segments as matching if at least one of them contains a path parameter. @@ -29,17 +54,39 @@ func HasMatchingPathSegments(pathA, pathB string) (bool, error) { // HasMatchingPathSegmentsWithParams("/some/{param}/path", "/some/{param}/path") // true // HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/{param}/path") // true // HasMatchingPathSegmentsWithParams("/some/abc/path", "/some/def/path") // false -func HasMatchingPathSegmentsWithParams(pathA, pathB string) (bool, error) { - return MatchingPathSegments(pathA, pathB, func(pathASeg, pathBSeg string) bool { - switch { - case IsParamSegment(pathASeg): - return !reflection.IsEmpty(pathBSeg) - case IsParamSegment(pathBSeg): - return !reflection.IsEmpty(pathASeg) - default: - return pathASeg == pathBSeg +func HasMatchingPathSegmentsWithParams(pathA, pathB string) (match bool, err error) { + return MatchingPathSegments(pathA, pathB, BasicEqualityPathSegmentWithParamMatcher) +} + +// BasicEqualityPathSegmentMatcher is a PathSegmentMatcherFunc that performs direct string comparison of two path segments. +func BasicEqualityPathSegmentMatcher(segmentA, segmentB string) (match bool, err error) { + match = segmentA == segmentB + return +} + +// BasicEqualityPathSegmentWithParamMatcher is a PathSegmentMatcherFunc that is similar to BasicEqualityPathSegmentMatcher but accounts for path parameter segments. +func BasicEqualityPathSegmentWithParamMatcher(segmentA, segmentB string) (match bool, err error) { + if IsPathParameter(segmentA) { + if errValidatePathASeg := ValidatePathParameter(segmentA); errValidatePathASeg != nil { + err = commonerrors.WrapErrorf(commonerrors.ErrInvalid, errValidatePathASeg, "an error occurred while validating path parameter %q", segmentA) + return } - }) + + match = !reflection.IsEmpty(segmentB) + return + } + + if IsPathParameter(segmentB) { + if errValidatePathBSeg := ValidatePathParameter(segmentB); errValidatePathBSeg != nil { + err = commonerrors.WrapErrorf(commonerrors.ErrInvalid, errValidatePathBSeg, "an error occurred while validating path parameter %q", segmentB) + return + } + + match = !reflection.IsEmpty(segmentA) + return + } + + return BasicEqualityPathSegmentMatcher(segmentA, segmentB) } // MatchingPathSegments checks whether two path strings match based on their segments using the provided matcher function. @@ -59,14 +106,32 @@ func MatchingPathSegments(pathA, pathB string, matcherFn PathSegmentMatcherFunc) return } - pathASegments := SplitPath(pathA) - pathBSegments := SplitPath(pathB) + unescapedPathA, errPathASeg := netUrl.PathUnescape(pathA) + if errPathASeg != nil { + err = commonerrors.WrapErrorf(commonerrors.ErrUnexpected, errPathASeg, "an error occurred while unescaping path %q", pathA) + return + } + + unescapedPathB, errPathBSeg := netUrl.PathUnescape(pathB) + if errPathBSeg != nil { + err = commonerrors.WrapErrorf(commonerrors.ErrUnexpected, errPathBSeg, "an error occurred while unescaping path %q", pathB) + return + } + + pathASegments := SplitPath(unescapedPathA) + pathBSegments := SplitPath(unescapedPathB) if len(pathASegments) != len(pathBSegments) { return } for i := range pathBSegments { - if !matcherFn(pathASegments[i], pathBSegments[i]) { + match, err = matcherFn(pathASegments[i], pathBSegments[i]) + if err != nil { + err = commonerrors.WrapErrorf(commonerrors.ErrUnexpected, err, "an error occurred during execution of the matcher function for path segments %q and %q", pathASegments[i], pathBSegments[i]) + return + } + + if !match { return } } @@ -75,66 +140,19 @@ func MatchingPathSegments(pathA, pathB string, matcherFn PathSegmentMatcherFunc) return } -// SplitPath returns a slice containing the individual segments that make up the path string. It looks for the default "/" path separator when splitting. -func SplitPath(path string) []string { - return SplitPathWithSeparator(path, defaultPathSeparator) -} - -// SplitPathWithSeparator is similar to SplitPath but allows for specifying the path separator to look for when splitting. -func SplitPathWithSeparator(path string, separator string) []string { - path = strings.TrimSpace(path) - if reflection.IsEmpty(path) || path == separator { +// SplitPath returns a slice containing the individual segments that make up the path string p. +// It looks for the default forward slash path separator when splitting. +func SplitPath(p string) []string { + p = strings.TrimSpace(p) + if reflection.IsEmpty(p) || p == defaultPathSeparator { return nil } - path = strings.Trim(path, separator) - segments := strings.Split(path, separator) - out := segments[:0] - for _, p := range segments { - if !reflection.IsEmpty(p) { - out = append(out, p) - } + p = path.Clean(p) + p = strings.Trim(p, defaultPathSeparator) + if reflection.IsEmpty(p) { + return []string{} } - return out -} - -// JoinPaths returns a single concatenated path string from the supplied paths and correctly sets the default "/" separator between them. -func JoinPaths(paths ...string) (joinedPath string, err error) { - return JoinPathsWithSeparator(defaultPathSeparator, paths...) -} -// JoinPathsWithSeparator is similar to JoinPaths but allows for specifying the path separator to use. -func JoinPathsWithSeparator(separator string, paths ...string) (joinedPath string, err error) { - if paths == nil { - err = commonerrors.UndefinedVariable("paths") - return - } - if len(paths) == 0 { - return - } - if len(paths) == 1 { - joinedPath = paths[0] - return - } - - if reflection.IsEmpty(separator) { - separator = defaultPathSeparator - } - - joinedPath = paths[0] - for _, p := range paths[1:] { - pathAHasSlashSuffix := strings.HasSuffix(joinedPath, separator) - pathBHasSlashPrefix := strings.HasPrefix(p, separator) - - switch { - case pathAHasSlashSuffix && pathBHasSlashPrefix: - joinedPath += p[1:] - case !pathAHasSlashSuffix && !pathBHasSlashPrefix: - joinedPath += separator + p - default: - joinedPath += p - } - } - - return + return strings.Split(p, defaultPathSeparator) } diff --git a/utils/url/url_test.go b/utils/url/url_test.go index 36701e8930..8ab7a06cb6 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -1,6 +1,7 @@ package url import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -9,14 +10,106 @@ import ( "github.com/ARM-software/golang-utils/utils/commonerrors/errortest" ) -func TestUrl_IsParamSegment(t *testing.T) { - t.Run("true", func(t *testing.T) { - assert.True(t, IsParamSegment("{abc}")) - }) +func TestUrl_IsPathParameter(t *testing.T) { + tests := []struct { + name string + parameter string + result bool + }{ + { + "valid", + "{abc}", + true, + }, + { + "with encoded underscore", + "{abc%5F1}", // unescaped as '{abc_1}' + true, + }, + { + "missing left brace", + "abc}", + false, + }, + { + "missing right brace", + "{abc", + false, + }, + { + "missing both braces", + "abc", + false, + }, + { + "with encoded asterisk", + "{abc%2A123}", // unescaped as '{abc*123}' + true, + }, + { + "with encoded space", + "{%20abc%20}", // unescaped as '{ abc }' + true, + }, + } + + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.result, IsPathParameter(test.parameter)) + }) + } +} + +func TestUrl_ValidatePathParameter(t *testing.T) { + tests := []struct { + name string + parameter string + err error + }{ + { + "valid", + "{abc}", + nil, + }, + { + "with encoded underscore", + "{abc%5F1}", // unescaped as '{abc_1}' + nil, + }, + { + "missing left brace", + "abc}", + commonerrors.ErrInvalid, + }, + { + "missing right brace", + "{abc", + commonerrors.ErrInvalid, + }, + { + "missing both braces", + "abc", + commonerrors.ErrInvalid, + }, + { + "with encoded asterisk", + "{abc%2A123}", // unescaped as '{abc*123}' + commonerrors.ErrInvalid, + }, + { + "with encoded space", + "{%20abc%20}", // unescaped as '{ abc }' + commonerrors.ErrInvalid, + }, + } - t.Run("false", func(t *testing.T) { - assert.False(t, IsParamSegment("abc")) - }) + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + errortest.AssertError(t, ValidatePathParameter(test.parameter), test.err) + }) + } } func TestUrl_HasMatchingPathSegments(t *testing.T) { @@ -83,13 +176,33 @@ func TestUrl_HasMatchingPathSegments(t *testing.T) { true, nil, }, + { + "paths with repeated slashes", + "//abc///123/", + "abc//123/////", + true, + nil, + }, + { + "path with valid encoding", + "abc/123%5F456", // unescaped as 'abc/123_456' + "abc/123_456", + true, + nil, + }, + { + "path with invalid encoding", + "abc/%$#%*123", + "abc/123", + false, + commonerrors.ErrUnexpected, + }, } for i := range tests { test := tests[i] t.Run(test.name, func(t *testing.T) { match, err := HasMatchingPathSegments(test.pathA, test.pathB) - errortest.AssertError(t, err, test.err) assert.Equal(t, test.result, match) }) @@ -125,6 +238,13 @@ func TestUrl_HasMatchingPathSegmentsWithParams(t *testing.T) { true, nil, }, + { + "identical paths with repeated slashes", + "abc///123//", + "//abc/123///", + true, + nil, + }, { "identical paths with multiple segments", "abc/123/def/456/zzz", @@ -146,6 +266,13 @@ func TestUrl_HasMatchingPathSegmentsWithParams(t *testing.T) { true, nil, }, + { + "both paths with different parameter segments", + "/abc/{id}/123", + "/abc/{val}/123", + true, + nil, + }, { "paths with different segments", "/abc/123/xyz", @@ -167,255 +294,111 @@ func TestUrl_HasMatchingPathSegmentsWithParams(t *testing.T) { false, nil, }, - } - - for i := range tests { - test := tests[i] - t.Run(test.name, func(t *testing.T) { - match, err := HasMatchingPathSegmentsWithParams(test.pathA, test.pathB) - - errortest.AssertError(t, err, test.err) - assert.Equal(t, test.result, match) - }) - } -} - -func TestUrl_JoinPaths(t *testing.T) { - tests := []struct { - name string - paths []string - result string - error error - }{ - { - "empty paths", - []string{}, - "", - nil, - }, { - "undefined paths", - nil, - "", - commonerrors.ErrUndefined, - }, - { - "one path", - []string{"abc/123"}, + "path with valid encoding in parameter segment", + "abc/{param%2D1}", // unescaped as 'abc/{param-1}' "abc/123", + true, nil, }, { - "two paths", - []string{"abc/123", "def/456"}, - "abc/123/def/456", - nil, - }, - { - "two paths with leading and trailing slashes", - []string{"abc/123/", "/def/456"}, - "abc/123/def/456", - nil, - }, - { - "multiple paths", - []string{"abc/123", "def/456", "zzz", "{param1}", "999"}, - "abc/123/def/456/zzz/{param1}/999", - nil, + "path with invalid encoding in parameter segment", + "abc/{%$#%*param}", + "abc/123", + false, + commonerrors.ErrUnexpected, }, } for i := range tests { test := tests[i] t.Run(test.name, func(t *testing.T) { - joinedPaths, err := JoinPaths(test.paths...) - - errortest.AssertError(t, err, test.error) - assert.Equal(t, test.result, joinedPaths) + match, err := HasMatchingPathSegmentsWithParams(test.pathA, test.pathB) + errortest.AssertError(t, err, test.err) + assert.Equal(t, test.result, match) }) } } -func TestUrl_JoinPathsWithSeparator(t *testing.T) { +func TestUrl_MatchingPathSegments(t *testing.T) { tests := []struct { name string - paths []string - separator string - result string - error error + pathA string + pathB string + matcherFn PathSegmentMatcherFunc + result bool + err error }{ { - "empty paths", - []string{}, - "/", - "", - nil, - }, - { - "undefined paths", - nil, - "~", + "empty pathA", "", + "abc/123", + BasicEqualityPathSegmentMatcher, + false, commonerrors.ErrUndefined, }, { - "one path with custom separator", - []string{"abc/123"}, - "|", + "empty pathB", "abc/123", - nil, - }, - { - "two paths with custom separator", - []string{"abc/123", "def/456"}, - "|", - "abc/123|def/456", - nil, - }, - { - "two paths with empty separator", - []string{"abc/123", "def/456"}, "", - "abc/123/def/456", - nil, - }, - { - "two paths with whitespace separator", - []string{"abc/123", "def/456"}, - " ", - "abc/123/def/456", - nil, - }, - { - "two paths with leading and trailing slashes and custom separator", - []string{"abc/123/", "/def/456"}, - "#", - "abc/123/#/def/456", - nil, - }, - { - "multiple paths with custom separator", - []string{"abc/123", "def/456", "zzz", "{param1}", "999"}, - "~", - "abc/123~def/456~zzz~{param1}~999", - nil, - }, - } - - for i := range tests { - test := tests[i] - t.Run(test.name, func(t *testing.T) { - joinedPaths, err := JoinPathsWithSeparator(test.separator, test.paths...) - - errortest.AssertError(t, err, test.error) - assert.Equal(t, test.result, joinedPaths) - }) - } -} - -func TestUrl_SplitPath(t *testing.T) { - tests := []struct { - name string - path string - result []string - error error - }{ - { - "empty path", - "", - []string{}, - nil, + BasicEqualityPathSegmentMatcher, + false, + commonerrors.ErrUndefined, }, { - "path with one segment", - "abc", - []string{"abc"}, + "path with valid encoding", + "abc/123%5F456", // unescaped as 'abc/123_456' + "abc/123_456", + BasicEqualityPathSegmentMatcher, + true, nil, }, { - "path with two segments", + "path with invalid encoding", + "abc/%$#%*123", "abc/123", - []string{"abc", "123"}, - nil, - }, - { - "path with multiple segments", - "abc/123/def/456", - []string{"abc", "123", "def", "456"}, - nil, - }, - { - "path with multiple segments including param segment", - "abc/123/def/456/zzz/{param1}/999", - []string{"abc", "123", "def", "456", "zzz", "{param1}", "999"}, - nil, - }, - } - - for i := range tests { - test := tests[i] - t.Run(test.name, func(t *testing.T) { - segments := SplitPath(test.path) - - if test.result != nil { - for i, s := range segments { - assert.Equal(t, test.result[i], s) - } - } else { - assert.Nil(t, segments) - } - }) - } -} - -func TestUrl_SplitPathWithSeparator(t *testing.T) { - tests := []struct { - name string - path string - separator string - result []string - error error - }{ - { - "empty path", - "", - "", - []string{}, - nil, - }, - { - "path with one segment and custom separator", - "abc", - "|", - []string{"abc"}, - nil, + BasicEqualityPathSegmentMatcher, + false, + commonerrors.ErrUnexpected, }, { - "path with two segments and custom separator", - "abc|123", - "|", - []string{"abc", "123"}, + "paths with different segments with parameter", + "/abc/{param}/123", + "/def/123/zzz", + BasicEqualityPathSegmentWithParamMatcher, + false, nil, }, { - "path with multiple segments and custom separator", - "abc~123~def~456", - "~", - []string{"abc", "123", "def", "456"}, + "paths with different lengths and params", + "/abc/{param}", + "/abc/{param}/123", + BasicEqualityPathSegmentWithParamMatcher, + false, nil, }, { - "path with multiple segments including param segment and custom separator", - "abc~123~def~456~zzz~{param1}~999", - "~", - []string{"abc", "123", "def", "456", "zzz", "{param1}", "999"}, + "matching paths when using a custom matcher function", + "/abc/||zzz||/123", + "/abc/||{param}||/123", + func(segmentA string, segmentB string) (match bool, err error) { + segmentA = strings.Trim(segmentA, "|") + segmentB = strings.Trim(segmentB, "|") + return BasicEqualityPathSegmentWithParamMatcher(segmentA, segmentB) + }, + true, nil, }, { - "path with multiple segments including param segment and custom separator with other separators", - "abc~123/def~456~zzz~{param1}|999", - "~", - []string{"abc", "123/def", "456", "zzz", "{param1}|999"}, + "non-matching paths when using a custom matcher function", + "/abc/##zzz||/123", + "/abc/||{param}##/123", + func(segmentA string, segmentB string) (match bool, err error) { + segmentA = strings.Trim(segmentA, "#") + segmentB = strings.Trim(segmentB, "|") + return BasicEqualityPathSegmentWithParamMatcher(segmentA, segmentB) + }, + false, nil, }, } @@ -423,15 +406,9 @@ func TestUrl_SplitPathWithSeparator(t *testing.T) { for i := range tests { test := tests[i] t.Run(test.name, func(t *testing.T) { - segments := SplitPathWithSeparator(test.path, test.separator) - - if test.result != nil { - for i, s := range segments { - assert.Equal(t, test.result[i], s) - } - } else { - assert.Nil(t, segments) - } + match, err := MatchingPathSegments(test.pathA, test.pathB, test.matcherFn) + errortest.AssertError(t, err, test.err) + assert.Equal(t, test.result, match) }) } } From eba5bdf991ac05d8647aad39977f4225a581ba8e Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Wed, 26 Nov 2025 11:21:09 +0000 Subject: [PATCH 08/10] Use collection.ParseListWithCleanup in SplitPath function --- utils/url/url.go | 12 ++++------ utils/url/url_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index 0c7d442c6a..f4fd54069b 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -6,6 +6,7 @@ import ( "regexp" "strings" + "github.com/ARM-software/golang-utils/utils/collection" "github.com/ARM-software/golang-utils/utils/commonerrors" "github.com/ARM-software/golang-utils/utils/reflection" ) @@ -143,16 +144,11 @@ func MatchingPathSegments(pathA, pathB string, matcherFn PathSegmentMatcherFunc) // SplitPath returns a slice containing the individual segments that make up the path string p. // It looks for the default forward slash path separator when splitting. func SplitPath(p string) []string { - p = strings.TrimSpace(p) - if reflection.IsEmpty(p) || p == defaultPathSeparator { - return nil - } - - p = path.Clean(p) - p = strings.Trim(p, defaultPathSeparator) if reflection.IsEmpty(p) { return []string{} } - return strings.Split(p, defaultPathSeparator) + p = path.Clean(p) + p = strings.Trim(p, defaultPathSeparator) + return collection.ParseListWithCleanup(p, defaultPathSeparator) } diff --git a/utils/url/url_test.go b/utils/url/url_test.go index 8ab7a06cb6..c3b5494624 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -412,3 +412,58 @@ func TestUrl_MatchingPathSegments(t *testing.T) { }) } } + +func TestUrl_SplitPath(t *testing.T) { + tests := []struct { + name string + path string + result []string + }{ + { + "empty path", + "", + []string{}, + }, + { + "root path", + "/", + []string{"/"}, + }, + { + "root path with repeated slashes", + "///", + []string{"/"}, + }, + { + "path with one segment", + "abc", + []string{"abc"}, + }, + { + "path with two segments", + "abc/123", + []string{"abc", "123"}, + }, + { + "path with multiple segments", + "abc/123/def/456", + []string{"abc", "123", "def", "456"}, + }, + { + "path with multiple segments including param segment", + "abc/123/def/456/zzz/{param1}/999", + []string{"abc", "123", "def", "456", "zzz", "{param1}", "999"}, + }, + } + + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + segments := SplitPath(test.path) + + for i, s := range segments { + assert.Equal(t, test.result[i], s) + } + }) + } +} From d564f6f3eb995839f57a87f3d7ef3052dbd1eb2d Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Wed, 26 Nov 2025 16:41:35 +0000 Subject: [PATCH 09/10] Update with PR suggestions --- utils/url/url.go | 25 +++++++++++++++++++------ utils/url/url_test.go | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index f4fd54069b..b6ad0df37d 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -16,16 +16,17 @@ const ( minimumPathParameterLength = 3 ) -var validParamRegex = regexp.MustCompile(`^\{[A-Za-z0-9_-]+\}$`) +// Section 3.3 of RFC3986 details valid characters for path segments (see https://datatracker.ietf.org/doc/html/rfc3986#section-3.3) +var validPathRegex = regexp.MustCompile(`^(?:[A-Za-z0-9._~\-!$&'()*+,;=:@{}]|%[0-9A-Fa-f]{2})+$`) -// The expected signature for path segment matcher functions. +// PathSegmentMatcherFunc defines the signature for path segment matcher functions. type PathSegmentMatcherFunc = func(segmentA, segmentB string) (match bool, err error) // ValidatePathParameter checks whether a path parameter is valid. An error is returned if it is invalid. // Version 3.1.0 of the OpenAPI spec provides some guidance for path parameter values (see https://spec.openapis.org/oas/v3.1.0.html#path-templating) func ValidatePathParameter(parameter string) error { - if reflection.IsEmpty(parameter) || len(parameter) < minimumPathParameterLength { - return commonerrors.Newf(commonerrors.ErrInvalid, "parameter segment %q must have length greater than or equal to three", parameter) + if !IsPathParameter(parameter) { + return commonerrors.Newf(commonerrors.ErrInvalid, "parameter %q must not be empty, cannot contain only whitespaces, have a length greater than or equal to three, start with an opening brace, and end with a closing brace", parameter) } unescapedSegment, err := netUrl.PathUnescape(parameter) @@ -33,7 +34,7 @@ func ValidatePathParameter(parameter string) error { return commonerrors.WrapErrorf(commonerrors.ErrInvalid, err, "an error occurred during path unescaping for parameter %q", parameter) } - if !validParamRegex.MatchString(unescapedSegment) { + if !validPathRegex.MatchString(unescapedSegment) { return commonerrors.Newf(commonerrors.ErrInvalid, "parameter %q unescaped to %q can only contain alphanumeric characters, dashes, underscores, and a single pair of braces", parameter, unescapedSegment) } @@ -42,7 +43,19 @@ func ValidatePathParameter(parameter string) error { // IsPathParameter checks whether the parameter string is a path parameter as described by the OpenAPI spec (see https://spec.openapis.org/oas/v3.0.0.html#path-templating). func IsPathParameter(parameter string) bool { - return !reflection.IsEmpty(parameter) && strings.HasPrefix(parameter, "{") && strings.HasSuffix(parameter, "}") + if reflection.IsEmpty(parameter) { + return false + } + + if len(parameter) < minimumPathParameterLength { + return false + } + + if !strings.HasPrefix(parameter, "{") || !strings.HasSuffix(parameter, "}") { + return false + } + + return strings.Count(parameter, "{") == 1 && strings.Count(parameter, "}") == 1 } // HasMatchingPathSegments checks whether two path strings match based on their segments by doing a simple equality check on each path segment pair. diff --git a/utils/url/url_test.go b/utils/url/url_test.go index c3b5494624..9c85054014 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -27,12 +27,17 @@ func TestUrl_IsPathParameter(t *testing.T) { true, }, { - "missing left brace", + "only whitespace", + " ", + false, + }, + { + "missing opening brace", "abc}", false, }, { - "missing right brace", + "missing closing brace", "{abc", false, }, @@ -41,6 +46,11 @@ func TestUrl_IsPathParameter(t *testing.T) { "abc", false, }, + { + "contains multiple braces", + "{{abc}}", + false, + }, { "with encoded asterisk", "{abc%2A123}", // unescaped as '{abc*123}' @@ -51,6 +61,11 @@ func TestUrl_IsPathParameter(t *testing.T) { "{%20abc%20}", // unescaped as '{ abc }' true, }, + { + "with valid special characters", + "{abc$123.zzz~999}", + true, + }, } for i := range tests { @@ -72,18 +87,23 @@ func TestUrl_ValidatePathParameter(t *testing.T) { "{abc}", nil, }, + { + "with valid special characters", + "{abc.-_+$@!123(a)}", + nil, + }, { "with encoded underscore", "{abc%5F1}", // unescaped as '{abc_1}' nil, }, { - "missing left brace", + "missing opening brace", "abc}", commonerrors.ErrInvalid, }, { - "missing right brace", + "missing closing brace", "{abc", commonerrors.ErrInvalid, }, @@ -92,9 +112,19 @@ func TestUrl_ValidatePathParameter(t *testing.T) { "abc", commonerrors.ErrInvalid, }, + { + "contains multiple braces", + "{{abc}}", + commonerrors.ErrInvalid, + }, { "with encoded asterisk", "{abc%2A123}", // unescaped as '{abc*123}' + nil, + }, + { + "with encoded hash", + "{abc%23123}", // unescaped as '{abc#123}' commonerrors.ErrInvalid, }, { From 7597d768f507851cd1bed4f1e8fb75c55c67c1b3 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Thu, 27 Nov 2025 11:49:43 +0000 Subject: [PATCH 10/10] Add ozzo string rule for checking if a value is a valid path parameter --- utils/url/url.go | 10 +++++----- utils/url/url_test.go | 4 ++-- utils/validation/rules.go | 9 +++++++++ utils/validation/rules_test.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/utils/url/url.go b/utils/url/url.go index b6ad0df37d..cb931d1659 100644 --- a/utils/url/url.go +++ b/utils/url/url.go @@ -25,7 +25,7 @@ type PathSegmentMatcherFunc = func(segmentA, segmentB string) (match bool, err e // ValidatePathParameter checks whether a path parameter is valid. An error is returned if it is invalid. // Version 3.1.0 of the OpenAPI spec provides some guidance for path parameter values (see https://spec.openapis.org/oas/v3.1.0.html#path-templating) func ValidatePathParameter(parameter string) error { - if !IsPathParameter(parameter) { + if !MatchesPathParameterSyntax(parameter) { return commonerrors.Newf(commonerrors.ErrInvalid, "parameter %q must not be empty, cannot contain only whitespaces, have a length greater than or equal to three, start with an opening brace, and end with a closing brace", parameter) } @@ -41,8 +41,8 @@ func ValidatePathParameter(parameter string) error { return nil } -// IsPathParameter checks whether the parameter string is a path parameter as described by the OpenAPI spec (see https://spec.openapis.org/oas/v3.0.0.html#path-templating). -func IsPathParameter(parameter string) bool { +// MatchesPathParameterSyntax checks whether the parameter string matches the syntax for a path parameter as described by the OpenAPI spec (see https://spec.openapis.org/oas/v3.0.0.html#path-templating). +func MatchesPathParameterSyntax(parameter string) bool { if reflection.IsEmpty(parameter) { return false } @@ -80,7 +80,7 @@ func BasicEqualityPathSegmentMatcher(segmentA, segmentB string) (match bool, err // BasicEqualityPathSegmentWithParamMatcher is a PathSegmentMatcherFunc that is similar to BasicEqualityPathSegmentMatcher but accounts for path parameter segments. func BasicEqualityPathSegmentWithParamMatcher(segmentA, segmentB string) (match bool, err error) { - if IsPathParameter(segmentA) { + if MatchesPathParameterSyntax(segmentA) { if errValidatePathASeg := ValidatePathParameter(segmentA); errValidatePathASeg != nil { err = commonerrors.WrapErrorf(commonerrors.ErrInvalid, errValidatePathASeg, "an error occurred while validating path parameter %q", segmentA) return @@ -90,7 +90,7 @@ func BasicEqualityPathSegmentWithParamMatcher(segmentA, segmentB string) (match return } - if IsPathParameter(segmentB) { + if MatchesPathParameterSyntax(segmentB) { if errValidatePathBSeg := ValidatePathParameter(segmentB); errValidatePathBSeg != nil { err = commonerrors.WrapErrorf(commonerrors.ErrInvalid, errValidatePathBSeg, "an error occurred while validating path parameter %q", segmentB) return diff --git a/utils/url/url_test.go b/utils/url/url_test.go index 9c85054014..c761dbfda6 100644 --- a/utils/url/url_test.go +++ b/utils/url/url_test.go @@ -10,7 +10,7 @@ import ( "github.com/ARM-software/golang-utils/utils/commonerrors/errortest" ) -func TestUrl_IsPathParameter(t *testing.T) { +func TestUrl_MatchesPathParameterSyntax(t *testing.T) { tests := []struct { name string parameter string @@ -71,7 +71,7 @@ func TestUrl_IsPathParameter(t *testing.T) { for i := range tests { test := tests[i] t.Run(test.name, func(t *testing.T) { - assert.Equal(t, test.result, IsPathParameter(test.parameter)) + assert.Equal(t, test.result, MatchesPathParameterSyntax(test.parameter)) }) } } diff --git a/utils/validation/rules.go b/utils/validation/rules.go index a51331782e..e33ecc36b1 100644 --- a/utils/validation/rules.go +++ b/utils/validation/rules.go @@ -9,6 +9,7 @@ import ( "github.com/ARM-software/golang-utils/utils/commonerrors" "github.com/ARM-software/golang-utils/utils/encoding/base64" + "github.com/ARM-software/golang-utils/utils/url" ) // IsPort validates whether a value is a port using is.Port from github.com/go-ozzo/ozzo-validation/v4. @@ -41,3 +42,11 @@ func isPort(vRaw any) (err error) { // IsBase64 validates whether a value is a base64 encoded string. It is similar to is.Base64 but more generic and robust although less performant. var IsBase64 = validation.NewStringRuleWithError(base64.IsEncoded, is.ErrBase64) + +// IsPathParameter validates whether a value is a valid path parameter of a url. +var IsPathParameter = validation.NewStringRule(isValidPathParameter, "invalid path parameter") + +func isValidPathParameter(value string) bool { + err := url.ValidatePathParameter(value) + return err == nil +} diff --git a/utils/validation/rules_test.go b/utils/validation/rules_test.go index 3987bd2dd2..5499a8a03c 100644 --- a/utils/validation/rules_test.go +++ b/utils/validation/rules_test.go @@ -97,3 +97,31 @@ func TestIsBase64Encoded(t *testing.T) { }) } } + +func TestIsPathParameter(t *testing.T) { + tests := []struct { + input string + expected bool + }{ + {"{abc}", true}, + {"abc}", false}, + {"{abc", false}, + {"abc", false}, + {"{abc$123.zzz~999}", true}, + {"{abc%5F1}", true}, // unescaped as '{abc_1}' + {"{abc#123}", false}, + {" ", false}, + } + + for i := range tests { + test := tests[i] + t.Run(test.input, func(t *testing.T) { + err := validation.Validate(test.input, IsPathParameter) + if test.expected { + require.NoError(t, err) + } else { + errortest.AssertErrorDescription(t, err, "invalid path parameter") + } + }) + } +}