[Recorder] Workaround for the "url-path with single quotes" bug in nock (Azure#13474)

## Problem
As part of moving `data-tables` from `core-http` to `core-https` at Azure#12548, @xirzec has observed that the generated recordings for the node tests was invalid if the url-path has single quotes.
Upon investigating, I could see that the request url at core-http and core-https levels is the same.
The problem is seen because of the switch from `node-fetch`(core-http) to native `https`(core-https).

`node-fetch` encodes the request before sending which encodes the single quotes to `%27` and nock was able to save the fixtures with the encoded request nicely.
When migrated to `https` in core-https, there is a difference in behavior with the URL path, there is no default encoding, leading to the presence of single quotes.

## Bug(in `nock`)
Given the above problem, "nock" does capture the request with single quotes in the URL path, but fails to save the fixture properly leading to invalid recordings on our end.
Reference -  https://github.com/nock/nock/blob/dc04694dcd3186a396e48567bdfabbdad2761f6a/lib/recorder.js#L134

This line 
> lines.push(\`  .${methodName}('${path}', ${JSON.stringify(requestBody)})\`)

Single quotes can be present in the url path though unusual. When the url path has single quotes, the fixture generated by "nock" is incorrect since it doesn't consider the case. 
#### Examples below
- `.delete('/Tables('node')')` 
- `.get('/Tables('node')')` 
- `.post('/Tables('node')', {"TableName":"testTablenode"})`

## Fix
To avoid this problem, we replace the single quotes surrounding the url-path in the recording with backticks(`) if there are any single quotes present in the url-path. This would fix the invalid recordings.
This is backward compatible as well.
This would work for both core-http and core-https.

## To Do
- [x] Add workaround
- [x] Tests for the workaround
- [x] Create an issue in the "nock" repo 
    - [x] nock/nock#2136
- [x] Changelog

Author: HarshaNalluru

sdk/test-utils/recorder/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## 1.0.0 (Unreleased)
 
+## 2020-01-28
+
+- Single quotes can be present in the url path though unusual. When the url path has single quotes, the fixture generated by "nock" is incorrect leading to invalid recordings. [#13474](https://github.com/Azure/azure-sdk-for-js/pull/13474) introduces a workaround to be applied as part of the default customizations done on the generated recordings to fix the issue.
+
 ## 2020-12-02
 
 - Refactored the code to enable `"browser"` module replacement when used with bundlers. Previously, even browser bundled copies of the recorder would carry dependencies on several Node packages, which would lead to unresolved dependency warnings in Rollup regarding node builtins. With this change, the recorder's browser mappings will avoid this issue.

sdk/test-utils/recorder/src/baseRecorder.ts

@@ -8,7 +8,8 @@ import {
   filterSecretsFromStrings,
   filterSecretsRecursivelyFromJSON,
   generateTestRecordingFilePath,
-  decodeHexEncodingIfExistsInNockFixture
+  decodeHexEncodingIfExistsInNockFixture,
+  handleSingleQuotesInUrlPath
 } from "./utils";
 
 /**
@@ -41,7 +42,10 @@ export abstract class BaseRecorder {
   private defaultCustomizationsOnRecordings = !isBrowser()
     ? [
         // Decodes "hex" strings in the response from the recorded fixture if any exists.
-        decodeHexEncodingIfExistsInNockFixture
+        decodeHexEncodingIfExistsInNockFixture,
+        // Nock bug: Single quotes in the path of the url are not handled by nock.
+        // The following is the workaround we use in the recorder until nock fixes it.
+        handleSingleQuotesInUrlPath
       ]
     : [];
 

sdk/test-utils/recorder/src/utils/index.ts

@@ -389,6 +389,48 @@ export function decodeHexEncodingIfExistsInNockFixture(fixture: string): string
   return fixture;
 }
 
+/**
+ * Meant for node recordings only!
+ *
+ * Single quotes can be present in the url path though unusual.
+ * When the url path has single quotes, the fixture generated by "nock" is incorrect
+ * since it doesn't consider the case.
+ *  Examples below:
+ *   .delete('/Tables('node')')
+ *   .get('/Tables('node')')
+ *   .post('/Tables('node')', {"TableName":"testTablenode"})
+ *
+ * The above problem results in invalid recordings.
+ *
+ * To avoid this problem, we replace the single quotes surrounding the url-path in the recording
+ * with backticks(`). This would fix the invalid recordings.
+ *
+ * @private
+ * @param {string} fixture
+ */
+export function handleSingleQuotesInUrlPath(fixture: string): string {
+  let updatedFixture = fixture;
+  if (!isBrowser()) {
+    // Fixtures would contain url-path as shown below
+    // 1) .{method}('{url-path}')
+    // 2) .{method}('{url-path}', {json-object})
+    // Examples:
+    // .get('/Tables('node')')
+    // .post('/Tables('node')', {"TableName":"node"})
+    const matches = fixture.match(/\.(get|put|post|delete)\(\'(.*)\'(\,|\))/);
+
+    if (matches && matches[2]) {
+      const match = matches[2]; // Extracted url-path
+      // If the url-path contains a single quote
+      if (match.search("'") !== -1) {
+        // Replace the occurrence of surrounding single quotes with backticks
+        updatedFixture = fixture.replace("'" + match + "'", "`" + match + "`");
+      }
+    }
+  }
+  return updatedFixture;
+}
+
 /**
  * List of binary content types.
  * Currently, "avro/binary" is the only one present.

sdk/test-utils/recorder/test/node/utils.spec.ts

@@ -3,7 +3,8 @@ import {
   isBrowser,
   testHasChanged,
   isContentTypeInNockFixture,
-  decodeHexEncodingIfExistsInNockFixture
+  decodeHexEncodingIfExistsInNockFixture,
+  handleSingleQuotesInUrlPath
 } from "../../src/utils";
 
 import { nodeRequireRecordingIfExists, findRecordingsFolderPath } from "../../src/utils/recordings";
@@ -394,4 +395,102 @@ describe("NodeJS utils", () => {
       });
     });
   });
+
+  describe("handleSingleQuotesInUrlPath", () => {
+    [
+      {
+        name: `single quotes in get request in the fixture with request body`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .get('/'path'', "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?><QueryRequest><Expression>select * from BlobStorage</Expression></QueryRequest>")
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .get(\`/'path'\`, "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?><QueryRequest><Expression>select * from BlobStorage</Expression></QueryRequest>")
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `single quotes in get request in the fixture with no request body`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .get('/'path'')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .get(\`/'path'\`)
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `single quotes in delete request in the fixture with no request body`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/'path'pathx')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete(\`/'path'pathx\`)
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `no single quotes in get request in the fixture`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/path')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/path')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      },
+      {
+        name: `more than two single quotes in delete request in the fixture`,
+        input: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete('/p'''a't'h')
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`,
+        output: `nock('https://fakestorageaccount.blob.core.windows.net:443', {"encodedQueryParams":true})
+                  .delete(\`/p'''a't'h\`)
+                  .query(true)
+                  .reply(200, "4f626a0131c2", [
+                  'Transfer-Encoding',
+                  'chunked'
+                ]);`
+      }
+    ].forEach((test) => {
+      it(test.name, () => {
+        chai.assert.equal(
+          handleSingleQuotesInUrlPath(test.input),
+          test.output,
+          `Output from "handleSingleQuotesInUrlPath" did not match the expected output`
+        );
+      });
+    });
+  });
 });