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

Remove cookie support. #485

Merged
merged 2 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 2.7.0 - (2023-07-06)

- Remove cookie support from nodeFetchHttpClient to address [a security issue](https://nvd.nist.gov/vuln/detail/CVE-2023-26136) with the `tough-cookie` package.

## 2.6.6 - (2023-04-10)

- Update dependency `xml2js` version to `^0.5.0`.
Expand Down
40 changes: 3 additions & 37 deletions lib/nodeFetchHttpClient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

import * as tough from "tough-cookie";
import * as http from "http";
import * as https from "https";
import node_fetch from "node-fetch";
Expand All @@ -17,29 +16,13 @@ import { WebResourceLike } from "./webResource";
import { createProxyAgent, ProxyAgent } from "./proxyAgent";

export class NodeFetchHttpClient extends FetchHttpClient {
private readonly cookieJar = new tough.CookieJar(undefined, { looseMode: true });

async fetch(input: CommonRequestInfo, init?: CommonRequestInit): Promise<CommonResponse> {
return (node_fetch(input, init) as unknown) as Promise<CommonResponse>;
}

async prepareRequest(httpRequest: WebResourceLike): Promise<Partial<RequestInit>> {
const requestInit: Partial<RequestInit & { agent?: any }> = {};

if (this.cookieJar && !httpRequest.headers.get("Cookie")) {
const cookieString = await new Promise<string>((resolve, reject) => {
this.cookieJar!.getCookieString(httpRequest.url, (err, cookie) => {
if (err) {
reject(err);
} else {
resolve(cookie);
}
});
});

httpRequest.headers.set("Cookie", cookieString);
}

if (httpRequest.agentSettings) {
const { http: httpAgent, https: httpsAgent } = httpRequest.agentSettings;
if (httpsAgent && httpRequest.url.startsWith("https")) {
Expand Down Expand Up @@ -71,25 +54,8 @@ export class NodeFetchHttpClient extends FetchHttpClient {
return requestInit;
}

async processRequest(operationResponse: HttpOperationResponse): Promise<void> {
if (this.cookieJar) {
const setCookieHeader = operationResponse.headers.get("Set-Cookie");
if (setCookieHeader != undefined) {
await new Promise((resolve, reject) => {
this.cookieJar!.setCookie(
setCookieHeader,
operationResponse.request.url,
{ ignoreError: true },
(err) => {
if (err) {
reject(err);
} else {
resolve();
}
}
);
});
}
}
async processRequest(_operationResponse: HttpOperationResponse): Promise<void> {
/* no_op */
return;
}
}
2 changes: 1 addition & 1 deletion lib/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const Constants = {
* @const
* @type {string}
*/
msRestVersion: "2.6.6",
msRestVersion: "2.7.0",

/**
* Specifies HTTP.
Expand Down
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"email": "azsdkteam@microsoft.com",
"url": "https://github.com/Azure/ms-rest-js"
},
"version": "2.6.6",
"version": "2.7.0",
"description": "Isomorphic client Runtime for Typescript/node.js/browser javascript client libraries generated using AutoRest",
"tags": [
"isomorphic",
Expand Down Expand Up @@ -55,7 +55,6 @@
"abort-controller": "^3.0.0",
"form-data": "^2.5.0",
"node-fetch": "^2.6.7",
"tough-cookie": "^3.0.1",
"tslib": "^1.10.0",
"tunnel": "0.0.6",
"uuid": "^8.3.2",
Expand All @@ -78,7 +77,6 @@
"@types/node-fetch": "^2.3.7",
"@types/semver": "^6.0.1",
"@types/sinon": "^7.0.13",
"@types/tough-cookie": "^2.3.5",
"@types/trusted-types": "^2.0.0",
"@types/tunnel": "0.0.1",
"@types/uuid": "^8.3.2",
Expand Down
2 changes: 1 addition & 1 deletion review/ms-rest-js.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export class DefaultHttpClient extends FetchHttpClient {
// (undocumented)
prepareRequest(httpRequest: WebResourceLike): Promise<Partial<RequestInit>>;
// (undocumented)
processRequest(operationResponse: HttpOperationResponse): Promise<void>;
processRequest(_operationResponse: HttpOperationResponse): Promise<void>;
}

// @public
Expand Down
36 changes: 0 additions & 36 deletions test/defaultHttpClientTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,8 @@ import { RestError } from "../lib/restError";
import { isNode } from "../lib/util/utils";
import { WebResource, HttpRequestBody, TransferProgressEvent } from "../lib/webResource";
import { getHttpMock, HttpMockFacade } from "./mockHttp";
import { TestFunction } from "mocha";
import { CommonResponse } from "../lib/fetchHttpClient";

const nodeIt = (isNode ? it : it.skip) as TestFunction;

function getAbortController(): AbortController {
let controller: AbortController;
if (typeof AbortController === "function") {
Expand Down Expand Up @@ -98,39 +95,6 @@ describe("defaultHttpClient", function () {
}
});

nodeIt("should not overwrite a user-provided cookie (nodejs only)", async function () {
// Cookie is only allowed to be set by the browser based on an actual response Set-Cookie header
httpMock.get("http://my.fake.domain/set-cookie", {
status: 200,
headers: {
"Set-Cookie": "data=123456",
},
});

httpMock.get("http://my.fake.domain/cookie", async (_url, _method, _body, headers) => {
return {
status: 200,
headers: headers,
};
});

const client = getMockedHttpClient();

const request1 = new WebResource("http://my.fake.domain/set-cookie");
const response1 = await client.sendRequest(request1);
response1.headers.get("Set-Cookie")!.should.equal("data=123456");

const request2 = new WebResource("http://my.fake.domain/cookie");
const response2 = await client.sendRequest(request2);
response2.headers.get("Cookie")!.should.equal("data=123456");

const request3 = new WebResource("http://my.fake.domain/cookie", "GET", undefined, undefined, {
Cookie: "data=abcdefg",
});
const response3 = await client.sendRequest(request3);
response3.headers.get("Cookie")!.should.equal("data=abcdefg");
});

it("should allow canceling multiple requests with one token", async function () {
httpMock.post("/fileupload", async () => {
await sleep(1000);
Expand Down
Loading