Skip to content

Commit

Permalink
fix: use extend with deep copy where necessary (#106)
Browse files Browse the repository at this point in the history
we recently removed extend as a dependency but this caused a bug - the lack of deep copy in Object.assign causes issues putting together query params
  • Loading branch information
dpopp07 committed Aug 6, 2020
1 parent 3d37a31 commit 566b81f
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 25 deletions.
3 changes: 2 additions & 1 deletion auth/authenticators/basic-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import extend = require('extend');
import { computeBasicAuthHeader, validateInput } from '../utils';
import { Authenticator } from './authenticator';
import { AuthenticateOptions } from './authenticator-interface';
Expand Down Expand Up @@ -71,7 +72,7 @@ export class BasicAuthenticator extends Authenticator {
*/
public authenticate(requestOptions: AuthenticateOptions): Promise<void | Error> {
return new Promise((resolve) => {
requestOptions.headers = Object.assign({}, requestOptions.headers, this.authHeader);
requestOptions.headers = extend(true, {}, requestOptions.headers, this.authHeader);
resolve();
});
}
Expand Down
3 changes: 2 additions & 1 deletion auth/authenticators/bearer-token-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import extend = require('extend');
import { validateInput } from '../utils';
import { Authenticator } from './authenticator';
import { AuthenticateOptions } from './authenticator-interface';
Expand Down Expand Up @@ -76,7 +77,7 @@ export class BearerTokenAuthenticator extends Authenticator {
public authenticate(requestOptions: AuthenticateOptions): Promise<void> {
return new Promise((resolve) => {
const authHeader = { Authorization: `Bearer ${this.bearerToken}` };
requestOptions.headers = Object.assign({}, requestOptions.headers, authHeader);
requestOptions.headers = extend(true, {}, requestOptions.headers, authHeader);
resolve();
});
}
Expand Down
3 changes: 2 additions & 1 deletion auth/authenticators/token-request-based-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import extend = require('extend');
import { OutgoingHttpHeaders } from 'http';
import { JwtTokenManager } from '../token-managers';
import { Authenticator } from './authenticator';
Expand Down Expand Up @@ -118,7 +119,7 @@ export class TokenRequestBasedAuthenticator extends Authenticator {
public authenticate(requestOptions: AuthenticateOptions): Promise<void | Error> {
return this.tokenManager.getToken().then(token => {
const authHeader = { Authorization: `Bearer ${token}` };
requestOptions.headers = Object.assign({}, requestOptions.headers, authHeader);
requestOptions.headers = extend(true, {}, requestOptions.headers, authHeader);
return;
});
}
Expand Down
3 changes: 2 additions & 1 deletion auth/token-managers/cp4d-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import extend = require('extend');
import { computeBasicAuthHeader, validateInput } from '../utils';
import { JwtTokenManager, JwtTokenManagerOptions } from './jwt-token-manager';

Expand Down Expand Up @@ -95,7 +96,7 @@ export class Cp4dTokenManager extends JwtTokenManager {
options: {
url: this.url,
method: 'GET',
headers: Object.assign({}, this.headers, requiredHeaders),
headers: extend(true, {}, this.headers, requiredHeaders),
rejectUnauthorized: !this.disableSslVerification,
}
};
Expand Down
3 changes: 2 additions & 1 deletion auth/token-managers/iam-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import extend = require('extend');
import { OutgoingHttpHeaders } from 'http';
import logger from '../../lib/logger';
import { computeBasicAuthHeader, validateInput } from '../utils';
Expand Down Expand Up @@ -132,7 +133,7 @@ export class IamTokenManager extends JwtTokenManager {
options: {
url: this.url,
method: 'POST',
headers: Object.assign({}, this.headers, requiredHeaders),
headers: extend(true, {}, this.headers, requiredHeaders),
form: {
grant_type: 'urn:ibm:params:oauth:grant-type:apikey',
apikey: this.apikey,
Expand Down
7 changes: 4 additions & 3 deletions lib/request-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import axios from 'axios';
import { AxiosRequestConfig } from 'axios';
import axiosCookieJarSupport from 'axios-cookiejar-support';
import extend = require('extend');
import FormData = require('form-data');
import https = require('https');
import querystring = require('querystring');
Expand Down Expand Up @@ -53,7 +54,7 @@ export class RequestWrapper {
};

// merge axios config into default
Object.assign(axiosConfig, axiosOptions);
extend(true, axiosConfig, axiosOptions);

// if the user explicitly sets `disableSslVerification` to true,
// `rejectUnauthorized` must be set to false in the https agent
Expand Down Expand Up @@ -137,7 +138,7 @@ export class RequestWrapper {
* @throws {Error}
*/
public sendRequest(parameters): Promise<any> {
const options = Object.assign({}, parameters.defaultOptions, parameters.options);
const options = extend(true, {}, parameters.defaultOptions, parameters.options);
const { path, body, form, formData, qs, method, serviceUrl } = options;
let { headers, url } = options;

Expand Down Expand Up @@ -198,7 +199,7 @@ export class RequestWrapper {
if (formData) {
data = multipartForm;
// form-data generates headers that MUST be included or the request will fail
headers = Object.assign({}, headers, multipartForm.getHeaders());
headers = extend(true, {}, headers, multipartForm.getHeaders());
}

// TEMPORARY: Disabling gzipping due to bug in axios until fix is released:
Expand Down
157 changes: 140 additions & 17 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@
"@types/file-type": "~5.2.1",
"@types/isstream": "^0.1.0",
"@types/node": "~10.14.19",
"@types/tough-cookie": "^4.0.0",
"axios": "^0.18.1",
"axios-cookiejar-support": "^1.0.0",
"camelcase": "^5.3.1",
"debug": "^4.1.1",
"dotenv": "^6.2.0",
"expect": "^26.1.0",
"extend": "^3.0.2",
"file-type": "^7.7.1",
"form-data": "^2.3.3",
"isstream": "~0.1.2",
Expand Down
Loading

0 comments on commit 566b81f

Please sign in to comment.