Skip to content

Commit

Permalink
[core-http] support disabling decompression in node-fetch client (Azu…
Browse files Browse the repository at this point in the history
…re#7878)

Azure Blob Storage allows setting content encoding of a blob that stores
compressed data. Browsers or other web clients can decompress accordingly when
consuming the content of the blob. For NodeJS our node-fetch http client supports
gzip/defalte content encoding by default thus de-compress the data in
`download()` and similar methods. This causes problems, mainly,

- upload and download using storage sdk clients are inconsistent.
- retry for partial response wouldn't work in the decompressed stream.

This change adds support to set the `compress` option to false using a request
policy.

Related to Azure#6411.
  • Loading branch information
jeremymeng committed Mar 25, 2020
1 parent aa8d4eb commit 78917a5
Show file tree
Hide file tree
Showing 10 changed files with 240 additions and 3 deletions.
1 change: 1 addition & 0 deletions sdk/core/core-http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
],
"browser": {
"./es/src/policies/msRestUserAgentPolicy.js": "./es/src/policies/msRestUserAgentPolicy.browser.js",
"./es/src/policies/disableResponseDecompressionPolicy.js": "./es/src/policies/disableResponseDecompressionPolicy.browser.js",
"./es/src/policies/proxyPolicy.js": "./es/src/policies/proxyPolicy.browser.js",
"./es/src/util/base64.js": "./es/src/util/base64.browser.js",
"./es/src/util/xml.js": "./es/src/util/xml.browser.js",
Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-http/src/coreHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export { throttlingRetryPolicy } from "./policies/throttlingRetryPolicy";
export { getDefaultProxySettings, proxyPolicy } from "./policies/proxyPolicy";
export { redirectPolicy, RedirectOptions } from "./policies/redirectPolicy";
export { keepAlivePolicy, KeepAliveOptions } from "./policies/keepAlivePolicy";
export { disableResponseDecompressionPolicy } from "./policies/disableResponseDecompressionPolicy";
export { signingPolicy } from "./policies/signingPolicy";
export {
userAgentPolicy,
Expand Down
4 changes: 3 additions & 1 deletion sdk/core/core-http/src/nodeFetchHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class NodeFetchHttpClient extends FetchHttpClient {
}

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

if (this.cookieJar && !httpRequest.headers.get("Cookie")) {
const cookieString = await new Promise<string>((resolve, reject) => {
Expand All @@ -111,6 +111,8 @@ export class NodeFetchHttpClient extends FetchHttpClient {
// Set the http(s) agent
requestInit.agent = this.getOrCreateAgent(httpRequest);

requestInit.compress = httpRequest.decompressResponse;

return requestInit;
}

Expand Down
5 changes: 5 additions & 0 deletions sdk/core/core-http/src/pipelineOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,9 @@ export interface InternalPipelineOptions extends PipelineOptions {
* Options to configure request/response logging.
*/
loggingOptions?: LogPolicyOptions;

/**
* Configure whether to decompress response according to Accept-Encoding header (node-fetch only)
*/
decompressResponse?: boolean;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

/*
* NOTE: When moving this file, please update "browser" section in package.json
*/
import { BaseRequestPolicy, RequestPolicy, RequestPolicyOptions } from './requestPolicy';
import { WebResource } from '../webResource';
import { HttpOperationResponse } from '../httpOperationResponse';

const DisbleResponseDecompressionNotSupportedInBrowser = new Error("DisableResponseDecompressionPolicy is not supported in browser environment");

/**
* {@link DisableResponseDecompressionPolicy} is not supported in browser and attempting
* to use it will results in error being thrown.
*/
export function disableResponseDecompressionPolicy() {
return {
create: (_nextPolicy: RequestPolicy, _options: RequestPolicyOptions) => {
throw DisbleResponseDecompressionNotSupportedInBrowser;
}
};
}

export class DisableResponseDecompressionPolicy extends BaseRequestPolicy {
constructor(
nextPolicy: RequestPolicy,
options: RequestPolicyOptions,
) {
super(nextPolicy, options);
throw DisbleResponseDecompressionNotSupportedInBrowser;
}

public async sendRequest(_request: WebResource): Promise<HttpOperationResponse> {
throw DisbleResponseDecompressionNotSupportedInBrowser;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { BaseRequestPolicy, RequestPolicy, RequestPolicyOptions } from './requestPolicy';
import { WebResource } from '../webResource';
import { HttpOperationResponse } from '../httpOperationResponse';

/**
* Returns a request policy factory that can be used to create an instance of
* {@link DisableResponseDecompressionPolicy}.
*/
export function disableResponseDecompressionPolicy() {
return {
create: (nextPolicy: RequestPolicy, options: RequestPolicyOptions) => {
return new DisableResponseDecompressionPolicy(nextPolicy, options);
}
};
}

/**
* A policy to disable response decompression according to Accept-Encoding header
* https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding
*/
export class DisableResponseDecompressionPolicy extends BaseRequestPolicy {
/**
* Creates an instance of DisableResponseDecompressionPolicy.
*
* @param {RequestPolicy} nextPolicy
* @param {RequestPolicyOptions} options
*/
constructor(
nextPolicy: RequestPolicy,
options: RequestPolicyOptions
) {
super(nextPolicy, options);
}

/**
* Sends out request.
*
* @param {WebResource} request
* @returns {Promise<HttpOperationResponse>}
*/
public async sendRequest(request: WebResource): Promise<HttpOperationResponse> {
request.decompressResponse = false;
return this._nextPolicy.sendRequest(request);
}
}
5 changes: 5 additions & 0 deletions sdk/core/core-http/src/serviceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import { logger } from "./log";
import { InternalPipelineOptions } from "./pipelineOptions";
import { DefaultKeepAliveOptions, keepAlivePolicy } from "./policies/keepAlivePolicy";
import { tracingPolicy } from "./policies/tracingPolicy";
import { disableResponseDecompressionPolicy } from './policies/disableResponseDecompressionPolicy';

/**
* Options to configure a proxy for outgoing requests (Node.js only).
Expand Down Expand Up @@ -707,6 +708,10 @@ export function createPipelineFromOptions(

requestPolicyFactories.push(logPolicy(loggingOptions));

if (isNode && pipelineOptions.decompressResponse === false) {
requestPolicyFactories.push(disableResponseDecompressionPolicy());
}

return {
httpClient: pipelineOptions.httpClient,
requestPolicyFactories
Expand Down
15 changes: 13 additions & 2 deletions sdk/core/core-http/src/webResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ export interface WebResourceLike {
* If the connection should be reused.
*/
keepAlive?: boolean;
/**
* Whether or not to decompress response according to Accept-Encoding header (node-fetch only)
*/
decompressResponse?: boolean;
/**
* A unique identifier for the request. Used for logging and tracing.
*/
Expand Down Expand Up @@ -194,6 +198,10 @@ export class WebResource implements WebResourceLike {
timeout: number;
proxySettings?: ProxySettings;
keepAlive?: boolean;
/**
* Whether or not to decompress response according to Accept-Encoding header (node-fetch only)
*/
decompressResponse?: boolean;
requestId: string;

abortSignal?: AbortSignalLike;
Expand Down Expand Up @@ -222,7 +230,8 @@ export class WebResource implements WebResourceLike {
onUploadProgress?: (progress: TransferProgressEvent) => void,
onDownloadProgress?: (progress: TransferProgressEvent) => void,
proxySettings?: ProxySettings,
keepAlive?: boolean
keepAlive?: boolean,
decompressResponse?: boolean
) {
this.streamResponseBody = streamResponseBody;
this.url = url || "";
Expand All @@ -238,6 +247,7 @@ export class WebResource implements WebResourceLike {
this.onDownloadProgress = onDownloadProgress;
this.proxySettings = proxySettings;
this.keepAlive = keepAlive;
this.decompressResponse = decompressResponse;
this.requestId = this.headers.get("x-ms-client-request-id") || generateUuid();
}

Expand Down Expand Up @@ -482,7 +492,8 @@ export class WebResource implements WebResourceLike {
this.onUploadProgress,
this.onDownloadProgress,
this.proxySettings,
this.keepAlive
this.keepAlive,
this.decompressResponse
);

if (this.formData) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import "chai/register-should";
import { RequestPolicyOptions } from "../../src/policies/requestPolicy";
import { WebResource } from "../../src/webResource";
import { HttpHeaders } from "../../src/httpHeaders";
import { disableResponseDecompressionPolicy, DisableResponseDecompressionPolicy } from "../../src/policies/disableResponseDecompressionPolicy";
import { HttpClient, ServiceClient, Serializer } from '../../src/coreHttp';

describe("DisableResponseDecompressionPolicy (browser)", function() {
const emptyRequestPolicy = {
sendRequest: (_: WebResource) =>
Promise.resolve({
request: new WebResource(),
status: 404,
headers: new HttpHeaders(undefined)
})
};

const emptyPolicyOptions = new RequestPolicyOptions();

describe("for browser", () => {
it("should throw an Error while constructing object", () => {
const construct = () =>
new DisableResponseDecompressionPolicy(emptyRequestPolicy, emptyPolicyOptions);
construct.should.throw();
});

it("should throw an Error while using the factory method", async () => {
let request: WebResource;
const httpClient: HttpClient = {
sendRequest: (req) => {
request = req;
return Promise.resolve({
request,
status: 200,
headers: new HttpHeaders(),
bodyAsText: "[1,2,3]"
});
}
};

const client1 = new ServiceClient(undefined, {
httpClient,
requestPolicyFactories: [disableResponseDecompressionPolicy()]
});

try {
await client1.sendOperationRequest(
{},
{
serializer: new Serializer(),
httpMethod: "GET",
baseUrl: "httpbin.org",
responses: {
200: {
bodyMapper: {
type: {
name: "Sequence",
element: {
type: {
name: "Number"
}
}
}
}
}
}
}
);
throw new Error("Error should have been thrown already.")
} catch (err) {
err.should.be.an("Error");
(err as Error).message.should.equal("DisableResponseDecompressionPolicy is not supported in browser environment")
}
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import "chai/register-should";
import { RequestPolicyOptions } from "../../src/policies/requestPolicy";
import { WebResource } from "../../src/webResource";
import { HttpHeaders } from "../../src/httpHeaders";
import { disableResponseDecompressionPolicy, DisableResponseDecompressionPolicy } from "../../src/policies/disableResponseDecompressionPolicy";
import { HttpOperationResponse } from '../../src/coreHttp';

describe("DisableResponseDecompressionPolicy (node)", function() {

function responseOf(r: WebResource): HttpOperationResponse {
return {
request: r,
status: 404,
headers: new HttpHeaders(undefined)
}
}
const verifyDecompressionDisabledPolicy = {
sendRequest: async(request: WebResource) => {
request.decompressResponse!.should.equal(false);
return Promise.resolve(responseOf(request))
}
};

const emptyPolicyOptions = new RequestPolicyOptions();

describe("for Node.js", function() {
it("factory passes correct option", async function() {
const factory = disableResponseDecompressionPolicy();
const policy = factory.create(verifyDecompressionDisabledPolicy, emptyPolicyOptions) as DisableResponseDecompressionPolicy;
const request = new WebResource();
await policy.sendRequest(request);
});
it("sets correct option through constructor", async function() {
const policy = new DisableResponseDecompressionPolicy(verifyDecompressionDisabledPolicy, emptyPolicyOptions);
const request = new WebResource();
await policy.sendRequest(request);
});

it("should assign option to the web request", async () => {
const policy = new DisableResponseDecompressionPolicy(verifyDecompressionDisabledPolicy, emptyPolicyOptions);
const request = new WebResource();
await policy.sendRequest(request);
});
});
});

0 comments on commit 78917a5

Please sign in to comment.