From 561ae943e22e2c57f7390765363a86366901bea0 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 27 Mar 2024 07:47:03 -0400 Subject: [PATCH 1/5] Refactor handling of invalid ds member names Signed-off-by: Timothy Johnson --- packages/zosfiles/CHANGELOG.md | 5 +++ .../__unit__/methods/list/List.unit.test.ts | 9 ++-- packages/zosfiles/src/methods/list/List.ts | 41 ++++++------------- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/packages/zosfiles/CHANGELOG.md b/packages/zosfiles/CHANGELOG.md index ed4deaf1c8..27c9a70fa3 100644 --- a/packages/zosfiles/CHANGELOG.md +++ b/packages/zosfiles/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the Zowe z/OS files SDK package will be documented in this file. +## Recent Changes + +- BugFix: Fixed error that could occur when listing data set members that contain control characters in the name. [#2104](https://github.com/zowe/zowe-cli/pull/2104) + ## `7.23.7` - BugFix: Fixed `Upload.bufferToUssFile` not normalizing new lines when uploading plain text. [#2091](https://github.com/zowe/zowe-cli/pull/2091) @@ -19,6 +23,7 @@ All notable changes to the Zowe z/OS files SDK package will be documented in thi - BugFix: Fix behavior where a specified directory was being lowercased on non-PDS datasets when downloading all datasets [#1722](https://github.com/zowe/zowe-cli/issues/1722) ## `7.18.8` + - Enhancement: Patch that adds invalidFileName to ZosFilesMessages ## `7.18.0` diff --git a/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts b/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts index cbe268ff62..e789a5c2de 100644 --- a/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts +++ b/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts @@ -173,15 +173,18 @@ describe("z/OS Files - List", () => { const memberNames = ["m1", "m2"]; const shuffledAsciiChars = String.fromCharCode(...Array.from(Array(256).keys()).sort(() => Math.random() - 0.5)); for (let i = 0; i < 32; i++) { - // Exclude double quotes for now - memberNames.push(shuffledAsciiChars.slice(i * 8, (i + 1) * 8).replace('"', '')); + memberNames.push(shuffledAsciiChars.slice(i * 8, (i + 1) * 8)); } expectStringSpy.mockResolvedValueOnce(`{"items":[\n` + memberNames.map((memName) => ` {"member":"${memName}"}`).join(",\n") + `\n` + `],"returnedRows":${memberNames.length},"JSONversion":1}`); const expectedListApiResponse = { - items: memberNames.map((memName) => ({ member: memName.replace((List as any).CONTROL_CHAR_REGEX, "\ufffd") })), + items: expect.arrayContaining([ + { member: "m1" }, + { member: "m2" }, + { member: expect.stringMatching(/… (\d+) more members/) } + ]), returnedRows: 34, JSONversion: 1 }; diff --git a/packages/zosfiles/src/methods/list/List.ts b/packages/zosfiles/src/methods/list/List.ts index 0961d540c3..5e9d4564e6 100644 --- a/packages/zosfiles/src/methods/list/List.ts +++ b/packages/zosfiles/src/methods/list/List.ts @@ -28,9 +28,6 @@ import { IDsmListOptions } from "./doc/IDsmListOptions"; * This class holds helper functions that are used to list data sets and its members through the z/OS MF APIs */ export class List { - // eslint-disable-next-line no-control-regex - private static CONTROL_CHAR_REGEX = new RegExp(/[\x00-\x1f\x7f\x80-\x9f]/g); - /** * Retrieve all members from a PDS * @@ -77,35 +74,23 @@ export class List { this.log.debug(`Endpoint: ${endpoint}`); - let data = await ZosmfRestClient.getExpectString(session, endpoint, reqHeaders); + const data = await ZosmfRestClient.getExpectString(session, endpoint, reqHeaders); let response: any; try { response = JSONUtils.parse(data); - } catch { - const regex = /"member":\s*"/g; - let match; - const matches = []; - - while ((match = regex.exec(data)) !== null) { - matches.push({ - matchString: match[0], - startIndex: match.index - }); - } - - // Iterate through matches in reverse order - for (let i = matches.length - 1; i >= 0; i--) { - const match = matches[i]; - const memberStartIdx = match.startIndex + match.matchString.length; - const memberNameEndIdx = data.indexOf('"', memberStartIdx + 1); // Find the end of the member name - if (memberNameEndIdx !== -1) { - const memberName = data.substring(memberStartIdx, memberNameEndIdx); - const escapedMemberName = memberName.replace(/(["\\])/g, `\\$1`).replace(this.CONTROL_CHAR_REGEX, "\\ufffd"); - data = data.substring(0, memberStartIdx) + escapedMemberName + data.substring(memberNameEndIdx); - } + } catch (err) { + const match = /in JSON at position (\d+)/.exec(err.message); + if (match != null) { + // Remove invalid member names from end of list and try to parse again + const lineNum = data.slice(0, parseInt(match[1])).split("\n").length - 1; + const lines = data.trim().split("\n"); + lines[lineNum - 1] = lines[lineNum - 1].replace(/,$/, ""); + lines.splice(lineNum, lines.length - lineNum - 1); + response = JSONUtils.parse(lines.join("\n")); + response.items.push({ "member": `… ${response.returnedRows - response.items.length} more members` }); + } else { + throw err; } - // parse the modified data as JSON - response = JSONUtils.parse(data); } return { From da70f23e181296f9f67bd1627baab7a59a912d73 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 27 Mar 2024 16:56:38 -0400 Subject: [PATCH 2/5] Display members with errors in CLI handler Signed-off-by: Timothy Johnson --- .../cli/src/zosfiles/list/am/AllMembers.handler.ts | 6 ++++++ .../__tests__/__unit__/methods/list/List.unit.test.ts | 10 ++++------ packages/zosfiles/src/methods/list/List.ts | 3 ++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts b/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts index 2ace3ddbcb..69e6474956 100644 --- a/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts +++ b/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts @@ -34,6 +34,12 @@ export default class AllMembersHandler extends ZosFilesBaseHandler { commandParameters.response.console.log(memberList.join("\n")); } + if (response.apiResponse.items.length < response.apiResponse.returnedRows) { + const invalidMemberCount = response.apiResponse.returnedRows - response.apiResponse.items.length; + commandParameters.response.console.log(TextUtils.chalk.gray( + `... ${invalidMemberCount} members failed to load due to invalid name errors`)); + } + return response; } } diff --git a/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts b/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts index e789a5c2de..a8fc17ea6a 100644 --- a/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts +++ b/packages/zosfiles/__tests__/__unit__/methods/list/List.unit.test.ts @@ -9,7 +9,7 @@ * */ -import { ImperativeError, Session } from "@zowe/imperative"; +import { ImperativeError, Logger, Session } from "@zowe/imperative"; import { ZosmfRestClient, ZosmfHeaders } from "@zowe/core-for-zowe-sdk"; import { List } from "../../../../src/methods/list/List"; import { ZosFilesMessages } from "../../../../src/constants/ZosFiles.messages"; @@ -178,13 +178,10 @@ describe("z/OS Files - List", () => { expectStringSpy.mockResolvedValueOnce(`{"items":[\n` + memberNames.map((memName) => ` {"member":"${memName}"}`).join(",\n") + `\n` + `],"returnedRows":${memberNames.length},"JSONversion":1}`); + const loggerWarnSpy = jest.spyOn(Logger.prototype, "warn").mockReturnValueOnce(""); const expectedListApiResponse = { - items: expect.arrayContaining([ - { member: "m1" }, - { member: "m2" }, - { member: expect.stringMatching(/… (\d+) more members/) } - ]), + items: expect.arrayContaining([{ member: "m1" }, { member: "m2" }]), returnedRows: 34, JSONversion: 1 }; @@ -204,6 +201,7 @@ describe("z/OS Files - List", () => { }); expect(expectStringSpy).toHaveBeenCalledTimes(1); expect(expectStringSpy).toHaveBeenCalledWith(dummySession, endpoint, [ZosmfHeaders.ACCEPT_ENCODING, ZosmfHeaders.X_IBM_MAX_ITEMS]); + expect(loggerWarnSpy.mock.calls[0][0]).toContain("members failed to load due to invalid name errors"); }); it("should list members from given data set with a matching pattern", async () => { diff --git a/packages/zosfiles/src/methods/list/List.ts b/packages/zosfiles/src/methods/list/List.ts index 5e9d4564e6..10d8a29da0 100644 --- a/packages/zosfiles/src/methods/list/List.ts +++ b/packages/zosfiles/src/methods/list/List.ts @@ -87,7 +87,8 @@ export class List { lines[lineNum - 1] = lines[lineNum - 1].replace(/,$/, ""); lines.splice(lineNum, lines.length - lineNum - 1); response = JSONUtils.parse(lines.join("\n")); - response.items.push({ "member": `… ${response.returnedRows - response.items.length} more members` }); + const invalidMemberCount = response.returnedRows - response.items.length; + this.log.warn(`${invalidMemberCount} members failed to load due to invalid name errors for ${dataSetName}`); } else { throw err; } From 368e038c955cc0e277541d4992a96d9c1b1d65bc Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 28 Mar 2024 09:06:27 -0400 Subject: [PATCH 3/5] Add handler unit test for members with errors Signed-off-by: Timothy Johnson --- .../list/am/AllMembers.handler.unit.test.ts | 112 ++++++++++++------ .../AllMembers.handler.unit.test.ts.snap | 24 ++++ .../zosfiles/list/am/AllMembers.handler.ts | 13 +- 3 files changed, 111 insertions(+), 38 deletions(-) diff --git a/packages/cli/__tests__/zosfiles/__unit__/list/am/AllMembers.handler.unit.test.ts b/packages/cli/__tests__/zosfiles/__unit__/list/am/AllMembers.handler.unit.test.ts index 20d03b4a38..55871bc2ab 100644 --- a/packages/cli/__tests__/zosfiles/__unit__/list/am/AllMembers.handler.unit.test.ts +++ b/packages/cli/__tests__/zosfiles/__unit__/list/am/AllMembers.handler.unit.test.ts @@ -11,20 +11,57 @@ import { List } from "@zowe/zos-files-for-zowe-sdk"; import { UNIT_TEST_ZOSMF_PROF_OPTS } from "../../../../../../../__tests__/__src__/mocks/ZosmfProfileMock"; +import { IHandlerParameters } from "@zowe/imperative"; describe("List AllMembers handler", () => { + const dataSetName = "testing"; + let apiMessage = ""; + let jsonObj: any; + let logMessage = ""; + const fakeHandlerParms: IHandlerParameters = { + arguments: { + $0: "fake", + _: ["fake"], + dataSetName, + ...UNIT_TEST_ZOSMF_PROF_OPTS + }, + response: { + data: { + setMessage: jest.fn((setMsgArgs) => { + apiMessage = setMsgArgs; + }), + setObj: jest.fn((setObjArgs) => { + jsonObj = setObjArgs; + }) + }, + console: { + log: jest.fn((logArgs) => { + logMessage += "\n" + logArgs; + }) + }, + progress: { + startBar: jest.fn((parms) => { + // do nothing + }), + endBar: jest.fn(() => { + // do nothing + }) + } + } + } as any; + describe("process method", () => { + beforeEach(() => { + logMessage = ""; + }); + it("should list all members from a PDS if requested", async () => { // Require the handler and create a new instance const handlerReq = require("../../../../../src/zosfiles/list/am/AllMembers.handler"); const handler = new handlerReq.default(); - const dataSetName = "testing"; // Vars populated by the mocked function let error; - let apiMessage = ""; - let jsonObj; - let logMessage = ""; let fakeSession = null; // Mock the submit JCL function @@ -41,37 +78,44 @@ describe("List AllMembers handler", () => { try { // Invoke the handler with a full set of mocked arguments and response functions - await handler.process({ - arguments: { - $0: "fake", - _: ["fake"], - dataSetName, - ...UNIT_TEST_ZOSMF_PROF_OPTS - }, - response: { - data: { - setMessage: jest.fn((setMsgArgs) => { - apiMessage = setMsgArgs; - }), - setObj: jest.fn((setObjArgs) => { - jsonObj = setObjArgs; - }) - }, - console: { - log: jest.fn((logArgs) => { - logMessage += "\n" + logArgs; - }) - }, - progress: { - startBar: jest.fn((parms) => { - // do nothing - }), - endBar: jest.fn(() => { - // do nothing - }) - } + await handler.process(fakeHandlerParms); + } catch (e) { + error = e; + } + + expect(error).toBeUndefined(); + expect(List.allMembers).toHaveBeenCalledTimes(1); + expect(List.allMembers).toHaveBeenCalledWith(fakeSession, dataSetName, {}); + expect(jsonObj).toMatchSnapshot(); + expect(apiMessage).toMatchSnapshot(); + expect(logMessage).toMatchSnapshot(); + }); + + it("should list all members from a PDS with some errors", async () => { + // Require the handler and create a new instance + const handlerReq = require("../../../../../src/zosfiles/list/am/AllMembers.handler"); + const handler = new handlerReq.default(); + + // Vars populated by the mocked function + let error; + let fakeSession = null; + + // Mock the submit JCL function + List.allMembers = jest.fn(async (session) => { + fakeSession = session; + return { + success: true, + commandResponse: "listed", + apiResponse: { + items: [{member: "test-item"}], + returnedRows: 3 } - } as any); + }; + }); + + try { + // Invoke the handler with a full set of mocked arguments and response functions + await handler.process(fakeHandlerParms); } catch (e) { error = e; } diff --git a/packages/cli/__tests__/zosfiles/__unit__/list/am/__snapshots__/AllMembers.handler.unit.test.ts.snap b/packages/cli/__tests__/zosfiles/__unit__/list/am/__snapshots__/AllMembers.handler.unit.test.ts.snap index 073678e87e..d383d88235 100644 --- a/packages/cli/__tests__/zosfiles/__unit__/list/am/__snapshots__/AllMembers.handler.unit.test.ts.snap +++ b/packages/cli/__tests__/zosfiles/__unit__/list/am/__snapshots__/AllMembers.handler.unit.test.ts.snap @@ -21,3 +21,27 @@ exports[`List AllMembers handler process method should list all members from a P test-item listed" `; + +exports[`List AllMembers handler process method should list all members from a PDS with some errors 1`] = ` +Object { + "apiResponse": Object { + "items": Array [ + Object { + "member": "test-item", + }, + ], + "returnedRows": 3, + }, + "commandResponse": "listed", + "success": true, +} +`; + +exports[`List AllMembers handler process method should list all members from a PDS with some errors 2`] = `""`; + +exports[`List AllMembers handler process method should list all members from a PDS with some errors 3`] = ` +" +test-item +... 2 members failed to load due to invalid name errors +listed" +`; diff --git a/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts b/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts index 69e6474956..41a7196ac3 100644 --- a/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts +++ b/packages/cli/src/zosfiles/list/am/AllMembers.handler.ts @@ -27,6 +27,13 @@ export default class AllMembersHandler extends ZosFilesBaseHandler { responseTimeout: commandParameters.arguments.responseTimeout }); + const invalidMemberCount = response.apiResponse.returnedRows - response.apiResponse.items.length; + if (invalidMemberCount > 0) { + const invalidMemberMsg = `${invalidMemberCount} members failed to load due to invalid name errors`; + response.apiResponse.items.push(commandParameters.arguments.attributes ? + invalidMemberMsg : { member: TextUtils.chalk.gray("... " + invalidMemberMsg) }); + } + if (commandParameters.arguments.attributes && response.apiResponse.items.length > 0) { commandParameters.response.console.log(TextUtils.prettyJson(response.apiResponse.items)); } else { @@ -34,10 +41,8 @@ export default class AllMembersHandler extends ZosFilesBaseHandler { commandParameters.response.console.log(memberList.join("\n")); } - if (response.apiResponse.items.length < response.apiResponse.returnedRows) { - const invalidMemberCount = response.apiResponse.returnedRows - response.apiResponse.items.length; - commandParameters.response.console.log(TextUtils.chalk.gray( - `... ${invalidMemberCount} members failed to load due to invalid name errors`)); + if (invalidMemberCount > 0) { + response.apiResponse.items.pop(); } return response; From 07a7087e5bc41b07444d11e6485d8ee709cb6e66 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 28 Mar 2024 09:10:51 -0400 Subject: [PATCH 4/5] Update CLI changelog Signed-off-by: Timothy Johnson --- packages/cli/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index ae802ce10e..1f3bd54866 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to the Zowe CLI package will be documented in this file. - BugFix: Resolved technical currency by updating `tar` dependency. [#2101](https://github.com/zowe/zowe-cli/issues/2101) - BugFix: Resolved technical currency by updating `markdown-it` dependency. [#2106](https://github.com/zowe/zowe-cli/pull/2106) +- BugFix: Fixed error in `zos-files list all-members` command that could occur when members contain control characters in the name. [#2104](https://github.com/zowe/zowe-cli/pull/2104) ## `7.23.5` From c2103dc6d5cf48dc960f28b167a76fcecb96add8 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Mon, 1 Apr 2024 10:57:31 -0400 Subject: [PATCH 5/5] Fix cli changelog Signed-off-by: Timothy Johnson --- packages/cli/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 1f3bd54866..f5feb71341 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -2,11 +2,14 @@ All notable changes to the Zowe CLI package will be documented in this file. +## Recent Changes + +- BugFix: Fixed error in `zos-files list all-members` command that could occur when members contain control characters in the name. [#2104](https://github.com/zowe/zowe-cli/pull/2104) + ## `7.23.9` - BugFix: Resolved technical currency by updating `tar` dependency. [#2101](https://github.com/zowe/zowe-cli/issues/2101) - BugFix: Resolved technical currency by updating `markdown-it` dependency. [#2106](https://github.com/zowe/zowe-cli/pull/2106) -- BugFix: Fixed error in `zos-files list all-members` command that could occur when members contain control characters in the name. [#2104](https://github.com/zowe/zowe-cli/pull/2104) ## `7.23.5`