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.

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

Fix UTF-8 String len & DELETE Request Body #15314

Merged
merged 3 commits into from
May 18, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion sdk/core/core-rest-pipeline/src/nodeHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ function streamToText(stream: NodeJS.ReadableStream): Promise<string> {
});
}

function getBodyLength(body: RequestBodyType): number | null {
export function getBodyLength(body: RequestBodyType): number | null {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we export I think we have to mark it as internal in a doc comment to avoid the documentation system picking it up, right @deyaaeldeen ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the @internal tag to the method

if (!body) {
return 0;
} else if (Buffer.isBuffer(body)) {
Expand All @@ -309,6 +309,8 @@ function getBodyLength(body: RequestBodyType): number | null {
return null;
} else if (isArrayBuffer(body)) {
return body.byteLength;
} else if (typeof body === "string") {
return Buffer.from(body).length;
sarangan12 marked this conversation as resolved.
Show resolved Hide resolved
} else {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-rest-pipeline/src/policies/redirectPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async function handleRedirect(
// redirected GET request if the redirect url is present in the location header
if (status === 303) {
request.method = "GET";
request.headers.delete("Content-Length");
delete request.body;
}

Expand Down
18 changes: 18 additions & 0 deletions sdk/core/core-rest-pipeline/test/node/getBodyLength.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { assert } from "chai";
import { getBodyLength } from "../../src/nodeHttpClient";

describe("Get Body Length", function() {
it("Gets the length of the ASCII string correctly", function() {
const str: string = "true";
assert.equal(getBodyLength(str), 4);
});

it("Gets the length of the non-ASCII string correctly", function() {
const str: string =
"啊齄丂狛狜隣郎隣兀﨩ˊ〞〡¦℡㈱‐ー﹡﹢﹫、〓ⅰⅹ⒈€㈠㈩ⅠⅫ! ̄ぁんァヶΑ︴АЯаяāɡㄅㄩ─╋︵﹄︻︱︳︴ⅰⅹɑɡ〇〾⿻⺁䜣€";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this insane test string

assert.equal(getBodyLength(str), 194);
});
});