From 1923d957be04726c2735df4492806635c90aa1d5 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Tue, 3 Oct 2023 13:30:11 -0400 Subject: [PATCH 1/3] Add test to abstract rest client and fix line breaks over chunks Signed-off-by: Andrew W. Harn --- .../client/AbstractRestClient.test.ts | 60 +++++++++++++++++++ .../rest/src/client/AbstractRestClient.ts | 16 ++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/packages/rest/__tests__/client/AbstractRestClient.test.ts b/packages/rest/__tests__/client/AbstractRestClient.test.ts index 72b590239..72a55184c 100644 --- a/packages/rest/__tests__/client/AbstractRestClient.test.ts +++ b/packages/rest/__tests__/client/AbstractRestClient.test.ts @@ -561,6 +561,66 @@ describe("AbstractRestClient tests", () => { expect(caughtError).toBeUndefined(); }); + fit("should not error when streaming normalized data", async () => { + + interface IPayload { + data: string; + } + + const mockedStream = new PassThrough(); + const emitter = new MockHttpRequestResponse(); + const newEmit = new MockHttpRequestResponse(); + const receivedArray: [string] = []; + newEmit.on("data", (data: Buffer) => { + receivedArray.push(data.toString()); + }); + const requestFnc = jest.fn((options, callback) => { + ProcessUtils.nextTick(async () => { + + callback(newEmit); + + await ProcessUtils.nextTick(() => { + newEmit.emit("end"); + }); + }); + + return emitter; + }); + + (https.request as any) = requestFnc; + let caughtError; + + try { + + await ProcessUtils.nextTick(() => { + console.log("1"); + mockedStream.emit("data", Buffer.from("ChunkOne\r", "utf8")) + }); + + await ProcessUtils.nextTick(() => { + console.log("2"); + mockedStream.emit("data", Buffer.from("\nChunkTwo", "utf8")); + }); + + await ProcessUtils.nextTick(() => { + console.log("3"); + mockedStream.end(); + }); + + console.log("5"); + await RestClient.putStreamed(new Session({ + hostname: "test", + }), "/resource", [Headers.APPLICATION_JSON], null, mockedStream); + + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(receivedArray.length).toEqual(2); + expect(receivedArray).toEqual(["ChunkOne", "\nChunkTwo"]); + }); + it("should return full response when requested", async () => { const emitter = new MockHttpRequestResponse(); const requestFnc = jest.fn((options, callback) => { diff --git a/packages/rest/src/client/AbstractRestClient.ts b/packages/rest/src/client/AbstractRestClient.ts index e2cee7496..612e370db 100644 --- a/packages/rest/src/client/AbstractRestClient.ts +++ b/packages/rest/src/client/AbstractRestClient.ts @@ -335,11 +335,21 @@ export abstract class AbstractRestClient { // if the user requested streaming write of data to the request, // write the data chunk by chunk to the server let bytesUploaded = 0; + let heldByte: string; options.requestStream.on("data", (data: Buffer) => { this.log.debug("Writing data chunk of length %d from requestStream to clientRequest", data.byteLength); if (this.mNormalizeRequestNewlines) { this.log.debug("Normalizing new lines in request chunk to \\n"); - data = Buffer.from(data.toString().replace(/\r?\n/g, "\n")); + let dataString = data.toString(); + if (heldByte != null) { + dataString = heldByte + dataString; + heldByte = undefined; + } + if (dataString.charAt(dataString.length - 1) === "\r") { + heldByte = dataString.charAt(dataString.length - 1); + dataString = dataString.slice(0,-1); + } + data = Buffer.from(dataString.replace(/\r?\n/g, "\n")); } if (this.mTask != null) { bytesUploaded += data.byteLength; @@ -361,6 +371,10 @@ export abstract class AbstractRestClient { })); }); options.requestStream.on("end", () => { + if (heldByte != null) { + clientRequest.write(Buffer.from(heldByte)); + heldByte = undefined; + } this.log.debug("Finished reading requestStream"); // finish the request clientRequest.end(); From 527efec6fbdb87960a95e6472be8c73c50a722af Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Tue, 3 Oct 2023 15:26:56 -0400 Subject: [PATCH 2/3] Fix unit test for streaming normalized data Signed-off-by: Timothy Johnson --- .../client/AbstractRestClient.test.ts | 42 +++++-------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/packages/rest/__tests__/client/AbstractRestClient.test.ts b/packages/rest/__tests__/client/AbstractRestClient.test.ts index 72a55184c..d777e6f34 100644 --- a/packages/rest/__tests__/client/AbstractRestClient.test.ts +++ b/packages/rest/__tests__/client/AbstractRestClient.test.ts @@ -561,64 +561,44 @@ describe("AbstractRestClient tests", () => { expect(caughtError).toBeUndefined(); }); - fit("should not error when streaming normalized data", async () => { - - interface IPayload { - data: string; - } - - const mockedStream = new PassThrough(); + it("should not error when streaming normalized data", async () => { + const fakeRequestStream = new PassThrough(); const emitter = new MockHttpRequestResponse(); - const newEmit = new MockHttpRequestResponse(); - const receivedArray: [string] = []; - newEmit.on("data", (data: Buffer) => { + const receivedArray: string[] = []; + jest.spyOn(emitter, "write").mockImplementation((data) => { receivedArray.push(data.toString()); }); const requestFnc = jest.fn((options, callback) => { ProcessUtils.nextTick(async () => { - + const newEmit = new MockHttpRequestResponse(); callback(newEmit); - await ProcessUtils.nextTick(() => { newEmit.emit("end"); }); }); - return emitter; }); - (https.request as any) = requestFnc; let caughtError; - try { - await ProcessUtils.nextTick(() => { - console.log("1"); - mockedStream.emit("data", Buffer.from("ChunkOne\r", "utf8")) + fakeRequestStream.write(Buffer.from("ChunkOne\r", "utf8")); }); - await ProcessUtils.nextTick(() => { - console.log("2"); - mockedStream.emit("data", Buffer.from("\nChunkTwo", "utf8")); + fakeRequestStream.write(Buffer.from("\nChunkTwo\r", "utf8")); }); - await ProcessUtils.nextTick(() => { - console.log("3"); - mockedStream.end(); + fakeRequestStream.end(); }); - - console.log("5"); await RestClient.putStreamed(new Session({ hostname: "test", - }), "/resource", [Headers.APPLICATION_JSON], null, mockedStream); - + }), "/resource", [Headers.APPLICATION_JSON], null, fakeRequestStream, false, true); } catch (error) { caughtError = error; } - expect(caughtError).toBeUndefined(); - expect(receivedArray.length).toEqual(2); - expect(receivedArray).toEqual(["ChunkOne", "\nChunkTwo"]); + expect(receivedArray.length).toEqual(3); + expect(receivedArray).toEqual(["ChunkOne", "\nChunkTwo", "\r"]); }); it("should return full response when requested", async () => { From 1fa7577034c00acb5632dac68fe3b0bcf1d54399 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Wed, 4 Oct 2023 08:46:38 -0400 Subject: [PATCH 3/3] Add changelog Signed-off-by: Andrew W. Harn --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05ffb5f41..4e52385d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the Imperative package will be documented in this file. +## Recent Changes + +- BugFix: Fixed normalization on stream chunk boundaries [#1815](https://github.com/zowe/zowe-cli/issues/1815) + ## `5.18.1` - BugFix: Fixed merging of profile properties in `ProfileInfo.createSession`. [#1008](https://github.com/zowe/imperative/issues/1008)