Skip to content

Commit

Permalink
Implement custom rate limiter due to security issues
Browse files Browse the repository at this point in the history
Remove the request-rate-limiter dependency because it was relying on an older insecure version of request.
  • Loading branch information
mikerourke committed Jul 22, 2017
1 parent dd8094e commit f7315c3
Show file tree
Hide file tree
Showing 5 changed files with 234 additions and 240 deletions.
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "trello-for-wolves",
"version": "1.4.6",
"version": "1.4.7",
"description": "Node.js wrapper for Trello API...for wolves.",
"engines": {
"node": ">=6.11.0"
Expand Down Expand Up @@ -47,8 +47,10 @@
"LICENSE"
],
"dependencies": {
"async-method": "^0.1.1",
"leaky-bucket": "^2.1.1",
"lodash.snakecase": "^4.1.1",
"request-rate-limiter": "^1.0.2"
"request": "^2.81.0"
},
"devDependencies": {
"apidoc": "^0.17.6",
Expand All @@ -71,7 +73,7 @@
"eslint-config-airbnb-base": "^11.2.0",
"eslint-plugin-flowtype": "^2.33.0",
"eslint-plugin-import": "^2.2.0",
"flow-bin": "^0.49.1",
"flow-bin": "^0.51.0",
"flow-copy-source": "^1.1.0",
"jsonfile": "^3.0.0",
"mocha": "^3.4.2",
Expand Down
14 changes: 5 additions & 9 deletions src/utils/api-request.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
/* @flow */

/* External dependencies */
import RateLimiter from 'request-rate-limiter';

/* Internal dependencies */
import { ApiCallResponseError } from '../utils/errors';
import RequestRateLimiter from '../utils/request-rate-limiter';

/* Types */
import type { HttpMethod } from '../types';
Expand Down Expand Up @@ -57,23 +55,21 @@ const performApiRequest = (
// Build the configuration object for sending the request.
const requestConfig = getRequestConfig(httpMethod, requestUrl, queryArgs);

const limiter = new RateLimiter({
rate: 100,
interval: 10,
const limiter = new RequestRateLimiter({
backoffTime,
maxWaitingTime,
});

limiter.request(requestConfig, (error, response) => {
/* instanbul ignore next */
/* instanbul ignore if */
if (error) {
reject(new Error(`Error performing request: ${error}`));
}
/* instanbul ignore next */
/* instanbul ignore if */
if (!response) {
reject(new Error('No response present when performing request.'));
}
const { statusCode = 400, body, ...responseData } = response;
const { statusCode /* instanbul ignore next */ = 400, body, ...responseData } = response;
if (statusCode > 299 || statusCode < 200) {
reject(new ApiCallResponseError(statusCode, httpMethod, requestUrl, body));
}
Expand Down
10 changes: 7 additions & 3 deletions src/utils/query-args-stringifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const getKeyForQueryString = (key: string): string => {
}

// Ensure this doesn't get converted to one word.
/* istanbul ignore if */
if (key === 'cardBoard') {
return 'card_board';
}
Expand All @@ -119,11 +120,14 @@ const getKeyForQueryString = (key: string): string => {
if (recasedKey.includes('_state')) {
recasedKey = recasedKey.replace('_state', 'State');
}
} else /* istanbul ignore if */ if (recasedKey.includes('sort_by')) {
/* istanbul ignore else */
} else if (recasedKey.includes('sort_by')) {
recasedKey = recasedKey.replace('sort_by', 'sortBy');
} else /* istanbul ignore if */ if (recasedKey.includes('sort_order')) {
/* istanbul ignore else */
} else if (recasedKey.includes('sort_order')) {
recasedKey = recasedKey.replace('sort_order', 'sortOrder');
} else /* istanbul ignore if */ if (recasedKey.includes('start_index')) {
/* istanbul ignore else */
} else if (recasedKey.includes('start_index')) {
recasedKey = recasedKey.replace('start_index', 'startIndex');
}

Expand Down
120 changes: 120 additions & 0 deletions src/utils/request-rate-limiter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/* @flow */

/**
* This code was taken from the request-rate-limiter library on npm. I added Flow
* typing and removed some of the logging functionality. I copied it over because
* the library is using an old version of the request library that is insecure.
* @see https://www.npmjs.com/package/request-rate-limiter
*/

/* External dependencies */
import asyncMethod from 'async-method';
import request from 'request';
import LeakyBucket from 'leaky-bucket';

export default class RequestRateLimiter {
bucket: Object;
backoffTime: number;
backoffTimer: any;
request: Function;

constructor(
options: {
backoffTime: number,
maxWaitingTime: number,
},
) {
const { backoffTime, maxWaitingTime } = options;
this.backoffTime = backoffTime;
this.bucket = new LeakyBucket({
capacity: 100,
interval: 10,
maxWaitingTime,
});
this.assignRequest();
}

/* istanbul ignore next */
isConfigAFunction(config: Object | Function): boolean {
return !!(config && config.constructor && config.call && config.apply);
}

assignRequest(): void {
this.request = asyncMethod((requestConfig, callback) => {
const executeRequest = (error, executeCallback) => {
const config = executeCallback || requestConfig;
/* istanbul ignore if */
if (error) {
this.handleError(config, callback);
} else {
/* istanbul ignore if */
if (typeof config !== 'object' && this.isConfigAFunction(config)) { // eslint-disable-line
config(null, () => {
this.backoff(() => executeRequest(null, config));
});
} else {
this.performRequest(config, callback);
}
}
};
this.bucket.throttle(executeRequest);
});
}

/* istanbul ignore next */
handleError(config: Object | Function, callback: Function): void {
const errorMessage = 'The request was not executed because it would not be scheduled within ' +
'the max waiting time!';
/* istanbul ignore if */
if (typeof config !== 'object' && this.isConfigAFunction(config)) {
config(new Error(errorMessage));
/* istanbul ignore else */
} else {
callback(new Error(errorMessage));
}
}

backoff(
/* istanbul ignore next */
config?: Object | Function = {},
/* istanbul ignore next */
callback?: Function = () => {},
): void {
if (!this.backoffTimer) {
this.backoffTimer = setTimeout(() => {
this.backoffTimer = null;
}, this.backoffTime * 1000);

this.bucket.pause(this.backoffTime);
}
this.bucket.reAdd((error) => {
/* istanbul ignore if */
if (error) {
this.handleError(config, callback);
} else {
/* istanbul ignore if */
if (typeof config !== 'object' && this.isConfigAFunction(config)) { // eslint-disable-line
config();
} else {
this.performRequest(config, callback);
}
}
});
}

performRequest(
config: Object | Function,
callback: Function,
): void {
request(config, (error, response) => {
/* istanbul ignore if */
if (error) {
callback(error);
} else if (response.statusCode === 429) {
this.backoff(config, callback);
} else {
callback(null, response);
}
});
}
}
Loading

0 comments on commit f7315c3

Please sign in to comment.