Skip to content

Commit d564f6f

Browse files
committed
Update with PR suggestions
1 parent eba5bdf commit d564f6f

File tree

2 files changed

+53
-10
lines changed

2 files changed

+53
-10
lines changed

utils/url/url.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,25 @@ const (
1616
minimumPathParameterLength = 3
1717
)
1818

19-
var validParamRegex = regexp.MustCompile(`^\{[A-Za-z0-9_-]+\}$`)
19+
// Section 3.3 of RFC3986 details valid characters for path segments (see https://datatracker.ietf.org/doc/html/rfc3986#section-3.3)
20+
var validPathRegex = regexp.MustCompile(`^(?:[A-Za-z0-9._~\-!$&'()*+,;=:@{}]|%[0-9A-Fa-f]{2})+$`)
2021

21-
// The expected signature for path segment matcher functions.
22+
// PathSegmentMatcherFunc defines the signature for path segment matcher functions.
2223
type PathSegmentMatcherFunc = func(segmentA, segmentB string) (match bool, err error)
2324

2425
// ValidatePathParameter checks whether a path parameter is valid. An error is returned if it is invalid.
2526
// 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)
2627
func ValidatePathParameter(parameter string) error {
27-
if reflection.IsEmpty(parameter) || len(parameter) < minimumPathParameterLength {
28-
return commonerrors.Newf(commonerrors.ErrInvalid, "parameter segment %q must have length greater than or equal to three", parameter)
28+
if !IsPathParameter(parameter) {
29+
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)
2930
}
3031

3132
unescapedSegment, err := netUrl.PathUnescape(parameter)
3233
if err != nil {
3334
return commonerrors.WrapErrorf(commonerrors.ErrInvalid, err, "an error occurred during path unescaping for parameter %q", parameter)
3435
}
3536

36-
if !validParamRegex.MatchString(unescapedSegment) {
37+
if !validPathRegex.MatchString(unescapedSegment) {
3738
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)
3839
}
3940

@@ -42,7 +43,19 @@ func ValidatePathParameter(parameter string) error {
4243

4344
// 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).
4445
func IsPathParameter(parameter string) bool {
45-
return !reflection.IsEmpty(parameter) && strings.HasPrefix(parameter, "{") && strings.HasSuffix(parameter, "}")
46+
if reflection.IsEmpty(parameter) {
47+
return false
48+
}
49+
50+
if len(parameter) < minimumPathParameterLength {
51+
return false
52+
}
53+
54+
if !strings.HasPrefix(parameter, "{") || !strings.HasSuffix(parameter, "}") {
55+
return false
56+
}
57+
58+
return strings.Count(parameter, "{") == 1 && strings.Count(parameter, "}") == 1
4659
}
4760

4861
// HasMatchingPathSegments checks whether two path strings match based on their segments by doing a simple equality check on each path segment pair.

utils/url/url_test.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,17 @@ func TestUrl_IsPathParameter(t *testing.T) {
2727
true,
2828
},
2929
{
30-
"missing left brace",
30+
"only whitespace",
31+
" ",
32+
false,
33+
},
34+
{
35+
"missing opening brace",
3136
"abc}",
3237
false,
3338
},
3439
{
35-
"missing right brace",
40+
"missing closing brace",
3641
"{abc",
3742
false,
3843
},
@@ -41,6 +46,11 @@ func TestUrl_IsPathParameter(t *testing.T) {
4146
"abc",
4247
false,
4348
},
49+
{
50+
"contains multiple braces",
51+
"{{abc}}",
52+
false,
53+
},
4454
{
4555
"with encoded asterisk",
4656
"{abc%2A123}", // unescaped as '{abc*123}'
@@ -51,6 +61,11 @@ func TestUrl_IsPathParameter(t *testing.T) {
5161
"{%20abc%20}", // unescaped as '{ abc }'
5262
true,
5363
},
64+
{
65+
"with valid special characters",
66+
"{abc$123.zzz~999}",
67+
true,
68+
},
5469
}
5570

5671
for i := range tests {
@@ -72,18 +87,23 @@ func TestUrl_ValidatePathParameter(t *testing.T) {
7287
"{abc}",
7388
nil,
7489
},
90+
{
91+
"with valid special characters",
92+
"{abc.-_+$@!123(a)}",
93+
nil,
94+
},
7595
{
7696
"with encoded underscore",
7797
"{abc%5F1}", // unescaped as '{abc_1}'
7898
nil,
7999
},
80100
{
81-
"missing left brace",
101+
"missing opening brace",
82102
"abc}",
83103
commonerrors.ErrInvalid,
84104
},
85105
{
86-
"missing right brace",
106+
"missing closing brace",
87107
"{abc",
88108
commonerrors.ErrInvalid,
89109
},
@@ -92,9 +112,19 @@ func TestUrl_ValidatePathParameter(t *testing.T) {
92112
"abc",
93113
commonerrors.ErrInvalid,
94114
},
115+
{
116+
"contains multiple braces",
117+
"{{abc}}",
118+
commonerrors.ErrInvalid,
119+
},
95120
{
96121
"with encoded asterisk",
97122
"{abc%2A123}", // unescaped as '{abc*123}'
123+
nil,
124+
},
125+
{
126+
"with encoded hash",
127+
"{abc%23123}", // unescaped as '{abc#123}'
98128
commonerrors.ErrInvalid,
99129
},
100130
{

0 commit comments

Comments
 (0)