fix: bug where non-variabled path colons weren't being escaped (#423)

readmeio · May 11, 2021 · 89b9ddb · 89b9ddb
1 parent 76bb481
commit 89b9ddb
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 13 deletions.
diff --git a/__tests__/tooling/__fixtures__/multiple-securities.json b/__tests__/tooling/__fixtures__/multiple-securities.json
@@ -220,9 +220,6 @@
       }
     }
   },
-  "x-explorer-enabled": true,
-  "x-samples-enabled": true,
-  "x-samples-languages": ["curl", "node", "ruby", "javascript", "python"],
   "components": {
     "securitySchemes": {
       "oauthScheme": {

diff --git a/__tests__/tooling/__fixtures__/path-variable-quirks.json b/__tests__/tooling/__fixtures__/path-variable-quirks.json
@@ -0,0 +1,36 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "Path variable quirks",
+    "version": "1.0.0"
+  },
+  "servers": [
+    {
+      "url": "https://api.example.com"
+    }
+  ],
+  "paths": {
+    "/people/{personIdType}:{personId}": {
+      "post": {
+        "parameters": [
+          {
+            "name": "personIdType",
+            "in": "path",
+            "required": true,
+            "schema": {
+              "type": "string"
+            }
+          },
+          {
+            "name": "personId",
+            "in": "path",
+            "required": true,
+            "schema": {
+              "type": "string"
+            }
+          }
+        ]
+      }
+    }
+  }
+}
diff --git a/__tests__/tooling/__fixtures__/server-variables.json b/__tests__/tooling/__fixtures__/server-variables.json
@@ -1,5 +1,9 @@
 {
   "openapi": "3.0.0",
+  "info": {
+    "title": "Server variables",
+    "version": "1.0.0"
+  },
   "servers": [
     {
       "url": "https://{name}.example.com:{port}/{basePath}",
@@ -16,10 +20,6 @@
       }
     }
   ],
-  "info": {
-    "version": "1.0.0",
-    "title": "Server variables"
-  },
   "paths": {
     "/post": {
       "post": {
@@ -29,8 +29,5 @@
         "responses": {}
       }
     }
-  },
-  "x-explorer-enabled": true,
-  "x-samples-enabled": true,
-  "x-samples-languages": ["curl", "node", "ruby", "javascript", "python"]
+  }
 }
diff --git a/__tests__/tooling/index.test.js b/__tests__/tooling/index.test.js
@@ -3,6 +3,7 @@ const $RefParser = require('@apidevtools/json-schema-ref-parser');
 const { Operation } = require('../../tooling');
 const petstore = require('@readme/oas-examples/3.0/json/petstore.json');
 const circular = require('./__fixtures__/circular.json');
+const pathVariableQuirks = require('./__fixtures__/path-variable-quirks.json');
 const petstoreServerVars = require('./__fixtures__/petstore-server-vars.json');
 const serverVariables = require('./__fixtures__/server-variables.json');
 
@@ -601,6 +602,39 @@ describe('#findOperation()', () => {
     });
   });
 
+  it('should return result if path contains non-variabled colons', () => {
+    const oas = new Oas(pathVariableQuirks);
+    const uri = 'https://api.example.com/people/GWID:3';
+    const method = 'post';
+
+    const res = oas.findOperation(uri, method);
+    expect(res).toMatchObject({
+      url: {
+        origin: 'https://api.example.com',
+        path: '/people/:personIdType::personId',
+        nonNormalizedPath: '/people/{personIdType}:{personId}',
+        slugs: { ':personIdType': 'GWID', ':personId': '3' },
+        method: 'POST',
+      },
+      operation: {
+        parameters: [
+          {
+            name: 'personIdType',
+            in: 'path',
+            required: true,
+            schema: { type: 'string' },
+          },
+          {
+            name: 'personId',
+            in: 'path',
+            required: true,
+            schema: { type: 'string' },
+          },
+        ],
+      },
+    });
+  });
+
   it('should return result if in server variable defaults', () => {
     const oas = new Oas(serverVariables);
     const uri = 'https://demo.example.com:443/v2/post';
@@ -821,6 +855,21 @@ describe('#getOperation()', () => {
       expect(operation.path).toBe('/api/esm');
       expect(operation.method).toBe('put');
     });
+
+    it('should be able to find a match on a url that contains colons', () => {
+      const oas = new Oas(pathVariableQuirks);
+      const source = {
+        url: 'https://api.example.com/people/GWID:3',
+        method: 'post',
+      };
+
+      const method = source.method.toLowerCase();
+      const operation = oas.getOperation(source.url, method);
+
+      expect(operation).toBeDefined();
+      expect(operation.path).toBe('/people/{personIdType}:{personId}');
+      expect(operation.method).toBe('post');
+    });
   });
 });
 

diff --git a/tooling/index.js b/tooling/index.js
@@ -60,7 +60,12 @@ function transformUrlIntoRegex(url) {
 }
 
 function normalizePath(path) {
-  return path.replace(/{(.*?)}/g, ':$1');
+  // In addition to transforming `{pathParam}` into `:pathParam` we also need to escape cases where a non-variabled
+  // colon is next to a variabled-colon because if we don't `path-to-regexp` won't be able to correct identify where the
+  // variable starts.
+  //
+  // For example if the URL is `/post/:param1::param2` we'll be escaping it to `/post/:param1\::param2`.
+  return path.replace(/{(.*?)}/g, ':$1').replace(/::/, '\\::');
 }
 
 function generatePathMatches(paths, pathName, origin) {
@@ -81,7 +86,7 @@ function generatePathMatches(paths, pathName, origin) {
       return {
         url: {
           origin,
-          path: cleanedPath,
+          path: cleanedPath.replace(/\\::/, '::'),
           nonNormalizedPath: path,
           slugs,
         },