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.

Sign up for GitHub

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

Jump to bottom

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

Merged
1 commit merged into from
Jan 29, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions sdk/test-utils/recorder/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@

## 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.
Expand Down
8 changes: 2 additions & 6 deletions sdk/test-utils/recorder/src/baseRecorder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {
filterSecretsFromStrings,
filterSecretsRecursivelyFromJSON,
generateTestRecordingFilePath,
decodeHexEncodingIfExistsInNockFixture,
handleSingleQuotesInUrlPath
decodeHexEncodingIfExistsInNockFixture
} from "./utils";

/**
Expand Down Expand Up @@ -42,10 +41,7 @@ export abstract class BaseRecorder {
private defaultCustomizationsOnRecordings = !isBrowser()
? [
// Decodes "hex" strings in the response from the recorded fixture if any exists.
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
decodeHexEncodingIfExistsInNockFixture
]
: [];

Expand Down
42 changes: 0 additions & 42 deletions sdk/test-utils/recorder/src/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,48 +389,6 @@ 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.
Expand Down
101 changes: 1 addition & 100 deletions sdk/test-utils/recorder/test/node/utils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import {
isBrowser,
testHasChanged,
isContentTypeInNockFixture,
decodeHexEncodingIfExistsInNockFixture,
handleSingleQuotesInUrlPath
decodeHexEncodingIfExistsInNockFixture
} from "../../src/utils";

import { nodeRequireRecordingIfExists, findRecordingsFolderPath } from "../../src/utils/recordings";
Expand Down Expand Up @@ -395,102 +394,4 @@ 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`
);
});
});
});
});