Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept special chars in resource path of the Open API #3429

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 68 additions & 4 deletions adapter/internal/oasparser/envoyconf/envoyconf_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,42 @@ func TestGenerateRegex(t *testing.T) {
message: "when the resource ends with *, trailing characters substitution passes",
isMatched: false,
},
{
inputpath: "/2.0/pet/{petId}/images/*",
userInputPath: "/2x0/pet/123/images/123-foo.png",
message: "when the version contains dots, those dots should not matched with any character",
isMatched: false,
},
{
inputpath: "/v2.0/pet/{petId}/images/*",
userInputPath: "/v2x0/pet/123/images/123-foo.png",
message: "when the version contains dots and starts with 'v', dots should not matched with any character",
isMatched: false,
},
{
inputpath: "/v2/pet/{petId}/images/*",
userInputPath: "/v2/pet/123/images/123-foo.png",
message: "when the resource ends with * and path params in the middle passes",
isMatched: true,
},
{
inputpath: "/v2/pet/{petId}/images/*",
userInputPath: "/v2/pet123/images/123-foo.png",
message: "when the resource ends with * and path params in the middle fails",
isMatched: false,
},
{
inputpath: "/v2/pet/{petId}/images)/*",
userInputPath: "/v2/pet/123/images)/123-foo.png",
message: "when the resource ends with *, special char in the path and path params in the middle passes",
renuka-fernando marked this conversation as resolved.
Show resolved Hide resolved
isMatched: true,
},
{
inputpath: "/v2/pet/{petId}/images)/*",
userInputPath: "/v2/pet/123/images/123-foo.png",
message: "when the resource ends with *, path params in the middle and special char in the path should exists",
isMatched: false,
},
{
inputpath: "/v2/pet/{petId}.api",
userInputPath: "/v2/pet/findByIdstatus=availabe",
Expand Down Expand Up @@ -434,7 +470,8 @@ func TestGenerateRegex(t *testing.T) {
if item.message == "when the resource ends with *" {
log.Default().Println(resultPattern, item.userInputPath, err)
}
assert.Equal(t, item.isMatched, resultIsMatching, item.message)
assert.Equal(t, item.isMatched, resultIsMatching, item.message,
map[string]string{"inputpath": item.inputpath, "resultPattern": resultPattern, "userInputPath": item.userInputPath})
assert.Nil(t, err)
}
}
Expand Down Expand Up @@ -501,11 +538,38 @@ func TestGenerateSubstitutionString(t *testing.T) {
"when input path has a path param and a wildcard at the end",
true,
},
{
"/v2.1.1/{petId}/*",
"/basepath/v2.1.1/\\1",
"when input path has sem version, a path param and a wildcard at the end",
true,
},
{
"/v2.1.1/{petId}/*",
"/basepath/v2.1.2/\\1",
"when input path has sem version, a path param and a wildcard at the end with not matching sem version",
false,
},
{
"/v2.1.1/{petId}/hello)/*",
"/basepath/v2.1.1/\\1/hello)",
"when input path has sem version, special char in the path, a path param and a wildcard at the end",
true,
},
{
"/v2.1.1/{petId}/hello)/*",
"/basepath/v2.1.1/\\1/hello\\)",
"when input path has sem version, special char in the path, a path param and a wildcard at the end with not matching special char",
false,
},
}
for _, item := range dataItems {
generatedSubstitutionString := generateSubstitutionString(item.inputPath, "/basepath")
isEqual := generatedSubstitutionString == item.expectedSubsString
assert.Equal(t, item.shouldEqual, isEqual, item.message)
if item.shouldEqual {
assert.Equal(t, item.expectedSubsString, generatedSubstitutionString, item.message)
} else {
assert.NotEqual(t, item.expectedSubsString, generatedSubstitutionString, item.message)
}
}
}

Expand Down Expand Up @@ -557,7 +621,7 @@ func TestGenerateRegexSegment(t *testing.T) {
}

for _, item := range dataItems {
generatedPathRegexSegment := generatePathRegexSegment(item.inputPath)
generatedPathRegexSegment := generatePathRegexSegment(item.inputPath, false)
isEqual := generatedPathRegexSegment == item.regexSegment
assert.Equal(t, item.shouldEqual, isEqual, item.message)
}
Expand Down
34 changes: 17 additions & 17 deletions adapter/internal/oasparser/envoyconf/routes_with_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,12 +878,12 @@ func createRoute(params *routeCreateParams) *routev3.Route {
if strings.Contains(resourcePath, "?") {
resourcePath = strings.Split(resourcePath, "?")[0]
}
resourceRegex := generatePathRegexSegment(resourcePath)
resourceRegex := generatePathRegexSegment(resourcePath, false)
substitutionString := generateSubstitutionString(resourcePath, endpointBasepath)
if strings.HasSuffix(resourcePath, "/*") {
resourceRegex = strings.TrimSuffix(resourceRegex, "((/(.*))*)")
}
pathRegex := "^" + GetUpdatedRegexToMatchDots(basePath) + resourceRegex
pathRegex := "^" + regexp.QuoteMeta(basePath) + resourceRegex

if xWso2Basepath != "" {
action = &routev3.Route_Route{
Expand Down Expand Up @@ -1340,19 +1340,25 @@ func generateRoutePaths(xWso2Basepath, basePath, resourcePath string) string {

// generatePathRegexSegment - generates a regex segment for a given resource path
// resourcePath - resource path of the api
// options - boolean parameter to indicate whether to generate path segment for substitution string
func generatePathRegexSegment(resourcePath string, options ...bool) string {
// skipEscapeMetaChars - skip escaping meta chars (special chars like `.(){}`.) True this for route substitution path.
func generatePathRegexSegment(resourcePath string, skipEscapeMetaChars bool) string {
renuka-fernando marked this conversation as resolved.
Show resolved Hide resolved
pathParaRegex := "([^/]+)"
wildCardRegex := "((/(.*))*)"
trailingSlashRegex := "(/{0,1})"

pathParaReplaceRegex := `{([^}]+)}`
wildCardReplaceRegex := "/*"
if !skipEscapeMetaChars {
resourcePath = regexp.QuoteMeta(resourcePath)
pathParaReplaceRegex = `\\{([^}]+)\\}`
wildCardReplaceRegex = "/\\*"
}

resourceRegex := ""
matcher := regexp.MustCompile(`{([^}]+)}`)
matcher := regexp.MustCompile(pathParaReplaceRegex)
resourceRegex = matcher.ReplaceAllString(resourcePath, pathParaRegex)
if !(len(options) > 0 && options[0]) {
resourceRegex = GetUpdatedRegexToMatchDots(resourceRegex)
}
if strings.HasSuffix(resourceRegex, "/*") {
resourceRegex = strings.TrimSuffix(resourceRegex, "/*") + wildCardRegex
if strings.HasSuffix(resourceRegex, wildCardReplaceRegex) {
resourceRegex = strings.TrimSuffix(resourceRegex, wildCardReplaceRegex) + wildCardRegex
} else {
resourceRegex = strings.TrimSuffix(resourceRegex, "/") + trailingSlashRegex
}
Expand Down Expand Up @@ -1415,12 +1421,6 @@ func basepathConsistent(basePath string) string {
return modifiedBasePath
}

// GetUpdatedRegexToMatchDots returns the regex to match the "." character in an existing regex.
func GetUpdatedRegexToMatchDots(regex string) string {
// Match "." character in the regex by replacing it with "\\."
return strings.ReplaceAll(regex, ".", "\\.")
}

// generateRegex generates regex for the resources which have path paramaters
// such that the envoy configuration can use it as a route.
// If path has path parameters ({id}), append a regex pattern (pathParaRegex).
Expand All @@ -1429,7 +1429,7 @@ func GetUpdatedRegexToMatchDots(regex string) string {
// TODO: (VirajSalaka) Improve regex specifically for strings, integers etc.
func generateRegex(fullpath string) string {
endRegex := "(\\?([^/]+))?"
newPath := generatePathRegexSegment(fullpath)
newPath := generatePathRegexSegment(fullpath, false)
return "^" + newPath + endRegex + "$"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class ApiLevelRatelimitTestCase {
public void createApiProject() throws Exception {
API api = new API();
api.setName("ratelimit");
api.setContext("v2/ratelimitService");
api.setContext("v1.0.0/ratelimitService");
api.setVersion("1.0.0");
api.setProvider("admin");

Expand All @@ -55,7 +55,7 @@ public void createApiProject() throws Exception {

testKey = TokenUtil.getJWT(api, applicationDto, "Unlimited", TestConstant.KEY_TYPE_PRODUCTION,
3600, null, true);
endpointURL = Utils.getServiceURLHttps("/v2/ratelimitService/pet/findByStatus");
endpointURL = Utils.getServiceURLHttps("/v1.0.0/ratelimitService/pet/findByStatus");
headers.put("Internal-Key", testKey);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class OperationLevelRatelimitTestCase {
public void createApiProject() throws Exception {
API api = new API();
api.setName("ratelimit");
api.setContext("v2/operationLevelRL");
api.setContext("v1.0.0/operationLevelRL");
api.setVersion("1.0.0");
api.setProvider("admin");

Expand All @@ -57,7 +57,7 @@ public void createApiProject() throws Exception {

@Test(description = "Test operation level 3 permin rate-limiting with envoy rate-limit service")
public void testRateLimitsWithEnvoyRateLimitService3PerMin() throws Exception {
String endpointURL = Utils.getServiceURLHttps("/v2/operationLevelRL/pet/findByStatus");
String endpointURL = Utils.getServiceURLHttps("/v1.0.0/operationLevelRL/pet/findByStatus");
Assert.assertTrue(RateLimitUtils.isThrottled(
RateLimitUtils.sendMultipleRequests(endpointURL, headers, 4)),
"Operation level rate-limit 3 per min testcase failed.");
Expand All @@ -66,22 +66,22 @@ public void testRateLimitsWithEnvoyRateLimitService3PerMin() throws Exception {
@Test(description = "Test operation level 5 per min rate-limiting with envoy rate-limit service")
public void testRateLimitsWithEnvoyRateLimitService5PerMin() throws Exception {
String requestData = "Payload to create pet 5";
String endpointURL1 = Utils.getServiceURLHttps("/v2/operationLevelRL/pet/3");
String endpointURL2 = Utils.getServiceURLHttps("/v2/operationLevelRL/pet/4");
String endpointURL1 = Utils.getServiceURLHttps("/v1.0.0/operationLevelRL/pet/3");
String endpointURL2 = Utils.getServiceURLHttps("/v1.0.0/operationLevelRL/pet/4");
RateLimitUtils.sendMultipleRequests(endpointURL2, headers, 2);
Assert.assertTrue(RateLimitUtils.isThrottled(
RateLimitUtils.sendMultipleRequests(endpointURL1, headers, 4))
, "Operation level rate-limit 5 per min testcase failed.");
HttpResponse response = HttpsClientRequest.doPost(Utils.getServiceURLHttps(
"/v2/operationLevelRL/pet/5"), requestData, headers);
"/v1.0.0/operationLevelRL/pet/5"), requestData, headers);
Assert.assertNotNull(response, "Response value recieved as null");
Assert.assertEquals(response.getResponseCode(), HttpStatus.SC_OK, "Response code mismatched");
Assert.assertEquals(response.getData(), requestData, "Request data and response data is not equal ");
}

@Test(description = "Test an operation without defining envoy rate-limits")
public void testOperationWithoutEnvoyRateLimits() throws Exception {
String endpointURL = Utils.getServiceURLHttps("/v2/operationLevelRL/pets/findByTags");
String endpointURL = Utils.getServiceURLHttps("/v1.0.0/operationLevelRL/pets/findByTags");
HttpResponse response = HttpsClientRequest.doGet(endpointURL, headers);
Map<String,String> responseHeadersMap = response.getHeaders();
Assert.assertTrue(!responseHeadersMap.containsKey("x-ratelimit-limit"), "x-ratelimit-limit header available");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: v4.1.0
data:
id: 2b547d7c-4f7b-4f31-ad6b-05b4960dc123
name: operationLevelRateLimit
context: /v2/operationLevelRL
context: /v1.0.0/operationLevelRL
version: 1.0.0
provider: admin
lifeCycleStatus: PUBLISHED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: v4.1.0
data:
id: 2b547d7c-4f7b-4f31-ad6b-05b4960dcbf1
name: ratelimit
context: /v2/ratelimitService
context: /v1.0.0/ratelimitService
version: 1.0.0
apiThrottlingPolicy: 5PerMinute
throttlingLimit:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ x-wso2-endpoints:
urls:
- http://mockBackend:2390/v2 # same base path as x-wso2-production-endpoints
type: loadbalance
x-wso2-basePath: /v2/operationLevelRL
x-wso2-basePath: /v1.0.0/operationLevelRL
schemes:
- http
paths:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ x-wso2-endpoints:
urls:
- http://mockBackend:2390/v2 # same base path as x-wso2-production-endpoints
type: loadbalance
x-wso2-basePath: /v2/ratelimitService
x-wso2-basePath: /v1.0.0/ratelimitService
schemes:
- http
paths:
Expand Down
Loading