Skip to content

Commit

Permalink
Merge eff35d7 into 0bbf862
Browse files Browse the repository at this point in the history
  • Loading branch information
onebytegone committed Apr 1, 2019
2 parents 0bbf862 + eff35d7 commit 81e4d20
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 24 deletions.
33 changes: 28 additions & 5 deletions src/Response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,29 @@ export default class Response {
* @param o the object to send in the response
*/
public jsonp(o: unknown): Response {
const queryParamName = this.app.getSetting('jsonp callback name') || 'callback',
callbackFunctionName = this._request.query[queryParamName as string];
const queryParamName = this.app.getSetting('jsonp callback name') || 'callback';

if (_.isString(callbackFunctionName)) {
this._body = `/**/ typeof ${callbackFunctionName} === 'function' && ${callbackFunctionName}(${JSON.stringify(o)});`;
return this.type('text/javascript; charset=utf-8').end();
let callbackFunctionName = this._request.query[queryParamName as string];

if (_.isArray(callbackFunctionName)) {
callbackFunctionName = callbackFunctionName[0];
}

if (_.isString(callbackFunctionName) && this._isValidJSONPCallback(callbackFunctionName)) {
const stringified = JSON.stringify(o)
.replace(/\u2028/g, '\\u2028')
.replace(/\u2029/g, '\\u2029');

// NOTE: The `/**/` is a security mitigation for "Rosetta Flash JSONP abuse", see
// silvermine/lambda-express#38. The `typeof` is to prevent errors on the client
// if the callback function doesn't exist, see expressjs/express#1773.
this._body = `/**/ typeof ${callbackFunctionName} === 'function' && ${callbackFunctionName}(${stringified});`;

return this.type('text/javascript; charset=utf-8')
// `nosniff` is set to mitigate "Rosetta Flash JSONP abuse", see
// silvermine/lambda-express#38
.set('X-Content-Type-Options', 'nosniff')
.end();
}

return this.json(o);
Expand Down Expand Up @@ -595,4 +612,10 @@ export default class Response {
// format: https://expressjs.com/en/api.html#res.format
// sendFile: https://expressjs.com/en/api.html#res.sendFile

protected _isValidJSONPCallback(name?: string): boolean {
// The "disable" is due to eslint erring because of the `\[`
// eslint-disable-next-line no-useless-escape
return !name || _.isEmpty(name) ? false : /^[\[\]\w$.]+$/.test(name);
}

}
180 changes: 161 additions & 19 deletions tests/Response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ import { RequestEvent, CookieOpts } from '../src/request-response-types';
// function type used for reusable test extension below
type Extender = (resp: Response, output: any) => void;

// dummy class to make protected functions unit testable
class TestResponse extends Response {

public _isValidJSONPCallback(name?: string): boolean {
return super._isValidJSONPCallback(name);
}

}

describe('Response', () => {
const EMPTY_CB = (): void => {}; // eslint-disable-line no-empty-function

Expand Down Expand Up @@ -680,28 +689,40 @@ describe('Response', () => {
});

describe('jsonp', () => {
const test = (evt: RequestEvent, msg: string | false, extender?: Extender, overrideQueryParamName?: string | false): void => {
let o = { foo: 'bar' },
queryParamName = overrideQueryParamName || 'callback',
body = `/**/ typeof fooFunction === 'function' && fooFunction(${JSON.stringify(o)});`;
const test = (
evt: RequestEvent,
msg: string | false,
extender?: Extender,
overrides: { queryParamName?: string | false; queryParamValues?: string[]; responseObject?: any } = {},
): void => {
const o = overrides.responseObject || { foo: 'bar' };

let queryParamValues = overrides.queryParamValues;

if (!queryParamValues || queryParamValues.length === 0) {
queryParamValues = [ 'fooFunction' ];
}

const expectedBody = `/**/ typeof ${queryParamValues[0]} === 'function' && ${queryParamValues[0]}(${JSON.stringify(o)});`;

// A false `queryParamName` means no query parameter was present
if (overrides.queryParamName !== false) {
let queryParamName = overrides.queryParamName || 'callback';

if (overrideQueryParamName === false) {
// false means no query parameter was present
body = JSON.stringify(o);
} else {
if (evt.multiValueQueryStringParameters) {
// TODO: test with multiple _different_ values
evt.multiValueQueryStringParameters[queryParamName] = [ 'fooFunction' ];
evt.multiValueQueryStringParameters[queryParamName] = queryParamValues;
}
if (evt.queryStringParameters) {
evt.queryStringParameters[queryParamName] = 'fooFunction';
evt.queryStringParameters[queryParamName] = queryParamValues[0];
}
}

let output = makeOutput(200, msg, body);
const output = makeOutput(200, msg, expectedBody);

resp = new Response(app, new Request(app, evt, handlerContext()), cb);
output.multiValueHeaders['Content-Type'] = [ 'text/javascript; charset=utf-8' ];
// See silvermine/lambda-express#38
output.multiValueHeaders['X-Content-Type-Options'] = [ 'nosniff' ];

if (extender) {
extender(resp, output);
Expand Down Expand Up @@ -733,17 +754,105 @@ describe('Response', () => {
app.setSetting('jsonp callback name', 'cbFnName');
};

it('works with custom callback param name - APIGW', () => { test(apiGatewayRequest(), false, ext2, 'cbFnName'); });
it('works with custom callback param name - ALB', () => { test(albRequest(), 'OK', ext2, 'cbFnName'); });
it('works with custom callback param name - ALB MV', () => { test(albMultiValHeadersRequest(), 'OK', ext2, 'cbFnName'); });
it('works with custom callback param name - APIGW', () => {
test(apiGatewayRequest(), false, ext2, { queryParamName: 'cbFnName' });
});
it('works with custom callback param name - ALB', () => {
test(albRequest(), 'OK', ext2, { queryParamName: 'cbFnName' });
});
it('works with custom callback param name - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', ext2, { queryParamName: 'cbFnName' });
});

const ext3: Extender = (_r, o): void => {
const expectJSON: Extender = (_r, o): void => {
o.multiValueHeaders['Content-Type'] = [ 'application/json; charset=utf-8' ];
delete o.multiValueHeaders['X-Content-Type-Options'];
o.body = JSON.stringify({ foo: 'bar' });
};

it('works like JSON when no callback in query - APIGW', () => { test(apiGatewayRequest(), false, ext3, false); });
it('works like JSON when no callback in query - ALB', () => { test(albRequest(), 'OK', ext3, false); });
it('works like JSON when no callback in query - ALB MV', () => { test(albMultiValHeadersRequest(), 'OK', ext3, false); });
it('works like JSON when no callback in query - APIGW', () => {
test(apiGatewayRequest(), false, expectJSON, { queryParamName: false });
});
it('works like JSON when no callback in query - ALB', () => {
test(albRequest(), 'OK', expectJSON, { queryParamName: false });
});
it('works like JSON when no callback in query - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', expectJSON, { queryParamName: false });
});

it('works like JSON when non-callback param is in query - APIGW', () => {
test(apiGatewayRequest(), false, expectJSON, { queryParamName: 'notcallback' });
});
it('works like JSON when non-callback param is in query - ALB', () => {
test(albRequest(), 'OK', expectJSON, { queryParamName: 'notcallback' });
});
it('works like JSON when non-callback param is in query - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', expectJSON, { queryParamName: 'notcallback' });
});

it('uses the first callback param value listed - APIGW', () => {
test(apiGatewayRequest(), false, undefined, {
queryParamValues: [ 'callbackOne', 'callbackTwo' ],
});
});
it('uses the first callback param value listed - ALB', () => {
test(albRequest(), 'OK', undefined, {
queryParamValues: [ 'callbackOne', 'callbackTwo' ],
});
});
it('uses the first callback param value listed - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', undefined, {
queryParamValues: [ 'callbackOne', 'callbackTwo' ],
});
});

it('allows for the callback param value to contain an array index - APIGW', () => {
test(apiGatewayRequest(), false, undefined, {
queryParamValues: [ 'callbacks[123]' ],
});
});
it('allows for the callback param value to contain an array index - ALB', () => {
test(albRequest(), 'OK', undefined, {
queryParamValues: [ 'callbacks[123]' ],
});
});
it('allows for the callback param value to contain an array index - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', undefined, {
queryParamValues: [ 'callbacks[123]' ],
});
});

it('returns JSON when callback param value contains invalid characters - APIGW', () => {
test(apiGatewayRequest(), false, expectJSON, {
queryParamValues: [ 'bad;fn()' ],
});
});
it('returns JSON when callback param value contains invalid characters - ALB', () => {
test(albRequest(), 'OK', expectJSON, {
queryParamValues: [ 'bad;fn()' ],
});
});
it('returns JSON when callback param value contains invalid characters - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', expectJSON, {
queryParamValues: [ 'bad;fn()' ],
});
});

const utfInput = { str: 'newline \u2028 paragraph \u2029 end' };

const ext3: Extender = (_r, o): void => {
o.body = '/**/ typeof fooFunction === \'function\' && fooFunction({"str":"newline \\u2028 paragraph \\u2029 end"});';
};

it('escapes UTF newline and paragraph separators - APIGW', () => {
test(apiGatewayRequest(), false, ext3, { responseObject: utfInput });
});
it('escapes UTF newline and paragraph separators - ALB', () => {
test(albRequest(), 'OK', ext3, { responseObject: utfInput });
});
it('escapes UTF newline and paragraph separators - ALB MV', () => {
test(albMultiValHeadersRequest(), 'OK', ext3, { responseObject: utfInput });
});
});

describe('redirect', () => {
Expand Down Expand Up @@ -936,4 +1045,37 @@ describe('Response', () => {

});

describe('_isValidJSONPCallback', () => {
let response: TestResponse;

beforeEach(() => {
response = new TestResponse(app, sampleReq, _.noop);
});

it('returns true for valid JSONP callbacks', () => {
expect(response._isValidJSONPCallback('callback')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('callback_12345')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('_abc')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('$')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('funcs[123]')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('window.callback')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('document.do_something[42]')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('run.$')).to.strictlyEqual(true);
// Technically "valid" with the current implmentation, but not but not
// in JS. Please don't actually use these forms.
expect(response._isValidJSONPCallback('[brackets]')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('snake_ca$e')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('$_[..]_$')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('currencies[$]')).to.strictlyEqual(true);
expect(response._isValidJSONPCallback('Well...the_data_is_back')).to.strictlyEqual(true);
});

it('returns false for invalid JSONP callbacks', () => {
expect(response._isValidJSONPCallback()).to.strictlyEqual(false);
expect(response._isValidJSONPCallback(undefined)).to.strictlyEqual(false);
expect(response._isValidJSONPCallback('')).to.strictlyEqual(false);
expect(response._isValidJSONPCallback('bad;func()')).to.strictlyEqual(false);
});
});

});

0 comments on commit 81e4d20

Please sign in to comment.