Merge pull request #485 from astegmaier/ansteg/remove-cookie-support

Remove cookie support.

Changelog.md

@@ -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`.

lib/nodeFetchHttpClient.ts

@@ -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";
@@ -17,8 +16,6 @@ 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>;
   }
@@ -26,20 +23,6 @@ export class NodeFetchHttpClient extends FetchHttpClient {
   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")) {
@@ -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;
   }
 }

lib/util/constants.ts

@@ -7,7 +7,7 @@ export const Constants = {
    * @const
    * @type {string}
    */
-  msRestVersion: "2.6.6",
+  msRestVersion: "2.7.0",
 
   /**
    * Specifies HTTP.

package.json

@@ -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",
@@ -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",
@@ -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",

review/ms-rest-js.api.md

@@ -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

test/defaultHttpClientTests.ts

@@ -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") {
@@ -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);