Merge pull request #2104 from zowe/fix/invalid-member-names

Refactor handling of invalid ds member names

packages/cli/CHANGELOG.md

@@ -5,10 +5,10 @@ All notable changes to the Zowe CLI package will be documented in this file.
 ## Recent Changes
 
 - Enhancement: Prompt for user/password on SSH commands when a token is stored in the config. [#2081](https://github.com/zowe/zowe-cli/pull/2081)
+- 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: Updated `tar` dependency to resolve technical currency [#2101](https://github.com/zowe/zowe-cli/issues/2101)
 - 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)
 

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;
             }

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"
+`;

packages/cli/src/zosfiles/list/am/AllMembers.handler.ts

@@ -27,13 +27,24 @@ 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 {
             const memberList = response.apiResponse.items.map((mem: any) => mem.member);
             commandParameters.response.console.log(memberList.join("\n"));
         }
 
+        if (invalidMemberCount > 0) {
+            response.apiResponse.items.pop();
+        }
+
         return response;
     }
 }

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`

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";
@@ -173,15 +173,15 @@ 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 loggerWarnSpy = jest.spyOn(Logger.prototype, "warn").mockReturnValueOnce("");
 
             const expectedListApiResponse = {
-                items: memberNames.map((memName) => ({ member: memName.replace((List as any).CONTROL_CHAR_REGEX, "\ufffd") })),
+                items: expect.arrayContaining([{ member: "m1" }, { member: "m2" }]),
                 returnedRows: 34,
                 JSONversion: 1
             };
@@ -201,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 () => {

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,24 @@ 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"));
+                    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;
                 }
-                // parse the modified data as JSON
-                response = JSONUtils.parse(data);
             }
 
             return {