Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for APIGWv2 protocol (#64) #88

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3,228 changes: 1,422 additions & 1,806 deletions package-lock.json

Large diffs are not rendered by default.

22 changes: 11 additions & 11 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,22 @@
"homepage": "https://github.com/silvermine/lambda-express#readme",
"devDependencies": {
"@silvermine/chai-strictly-equal": "1.1.0",
"@silvermine/eslint-config": "3.1.0-beta.0",
"@silvermine/standardization": "2.0.0",
"@silvermine/typescript-config": "0.9.0",
"@types/aws-lambda": "8.10.17",
"@silvermine/eslint-config": "3.2.0",
"@silvermine/standardization": "2.2.3",
"@silvermine/typescript-config": "1.0.0",
"@types/aws-lambda": "8.10.138",
"@types/chai": "4.1.7",
"@types/cookie": "0.3.2",
"@types/mocha": "5.2.5",
"@types/node": "8.10.36",
"@types/node": "20.12.2",
"@types/qs": "6.5.1",
"@types/sinon": "5.0.5",
"@types/underscore": "1.8.9",
"chai": "4.2.0",
"coveralls": "3.0.2",
"cz-conventional-changelog": "3.1.0",
"eslint": "6.8.0",
"grunt": "1.4.1",
"eslint": "^8.0.0",
"grunt": "1.6.1",
"grunt-cli": "1.3.2",
"grunt-concurrent": "2.3.1",
"grunt-contrib-clean": "2.0.0",
Expand All @@ -55,15 +55,15 @@
"sinon": "5.1.1",
"source-map-support": "0.5.9",
"standard-version": "git+https://github.com/jthomerson/standard-version.git#fix-305-header-repeat",
"ts-node": "7.0.1",
"typescript": "3.2.2"
"ts-node": "10.9.2",
"typescript": "4.7.4"
},
"dependencies": {
"@silvermine/toolbox": "0.1.0",
"@silvermine/toolbox": "0.3.0",
"cookie": "0.3.1",
"path-to-regexp": "0.1.7",
"qs": "6.6.0",
"tslib": "1.9.3",
"tslib": "2.6.2",
"underscore": "1.9.1"
}
}
42 changes: 18 additions & 24 deletions src/Application.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Callback, Context } from 'aws-lambda';
import Router from './Router';
import { RequestEvent, HandlerContext } from './request-response-types';
import { StringUnknownMap, Writable } from '@silvermine/toolbox';
import { StringUnknownMap, Writable, isUndefined } from '@silvermine/toolbox';
import { Request, Response } from '.';
import _ from 'underscore';
import { isErrorWithStatusCode } from './interfaces';
Expand Down Expand Up @@ -102,28 +102,22 @@ export default class Application extends Router {
}

private _createHandlerContext(context: Context): HandlerContext {
// keys should exist on both `HandlerContext` and `Context`
const keys: (keyof HandlerContext & keyof Context)[] = [
'functionName', 'functionVersion', 'invokedFunctionArn', 'memoryLimitInMB',
'awsRequestId', 'logGroupName', 'logStreamName', 'identity', 'clientContext',
'getRemainingTimeInMillis',
];

let handlerContext: Writable<HandlerContext>;

handlerContext = _.reduce(keys, (memo, key) => {
let contextValue = context[key];

if (typeof contextValue === 'object' && contextValue) {
// Freeze sub-objects
memo[key] = Object.freeze(_.extend({}, contextValue));
} else if (typeof contextValue !== 'undefined') {
memo[key] = contextValue;
}
return memo;
}, {} as Writable<HandlerContext>);

return Object.freeze(handlerContext);
const newContext: Writable<HandlerContext> = {
functionName: context.functionName,
functionVersion: context.functionVersion,
invokedFunctionArn: context.invokedFunctionArn,
memoryLimitInMB: context.memoryLimitInMB,
awsRequestId: context.awsRequestId,
logGroupName: context.logGroupName,
logStreamName: context.logStreamName,
getRemainingTimeInMillis: context.getRemainingTimeInMillis,
};
if (!isUndefined(context.identity)) {
newContext.identity = Object.freeze({ ...context.identity });
}
if (!isUndefined(context.clientContext)) {
newContext.clientContext = Object.freeze({ ...context.clientContext });
}
return Object.freeze(newContext);
}

}
76 changes: 51 additions & 25 deletions src/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import _ from 'underscore';
import qs from 'qs';
import cookie from 'cookie';
import Application from './Application';
import { RequestEvent, HandlerContext, RequestEventRequestContext, LambdaEventSourceType } from './request-response-types';
import { RequestEvent, HandlerContext, RequestEventRequestContext, LambdaEventSourceType, APIGatewayRequestEventV2, isAPIGatewayRequestEventV2 } from './request-response-types';
import { StringMap, KeyValueStringObject, StringArrayOfStringsMap, StringUnknownMap } from '@silvermine/toolbox';
import ConsoleLogger from './logging/ConsoleLogger';

Expand Down Expand Up @@ -203,7 +203,7 @@ export default class Request {
* Events passed to Lambda handlers by API Gateway and Application Load Balancers
* contain a "request context", which is available in this property.
*/
public readonly requestContext: RequestEventRequestContext;
public readonly requestContext: RequestEventRequestContext | APIGatewayRequestEventV2['requestContext'];

/**
* Contains the `context` object passed to the Lambda function's handler. Rarely used
Expand Down Expand Up @@ -254,32 +254,34 @@ export default class Request {
} else {
event = eventOrRequest;

const parsedQuery = this._parseQuery(event.multiValueQueryStringParameters || {}, event.queryStringParameters || {});
const parsedQuery = isAPIGatewayRequestEventV2(event)
? this._parseQuery({}, {}, event.rawQueryString)
: this._parseQuery(event.multiValueQueryStringParameters || {}, event.queryStringParameters || {}, '')

// Despite the fact that the Express docs say that the `originalUrl` is `baseUrl
// + path`, it's actually always equal to the original URL that initiated the
// request. If, for example, a route handler changes the `url` of a request, the
// `path` is changed too, *but* `originalUrl` stays the same. This would not be
// the case if `originalUrl = `baseUrl + path`. See the documentation on the
// `url` getter for more details.
url = `${event.path}?${parsedQuery.raw}`;
url = `${isAPIGatewayRequestEventV2(event) ? event.rawPath : event.path}?${parsedQuery.raw}`;
originalURL = url;
query = parsedQuery.parsed;
}

this.app = app;
this._event = event;
this._headers = this._parseHeaders(event);
this.method = (event.httpMethod || '').toUpperCase();
this.body = this._parseBody(event.body);
this.method = this._parseMethod(event);
this.body = this._parseBody(event.body || null);

this.eventSourceType = ('elb' in event.requestContext) ? Request.SOURCE_ALB : Request.SOURCE_APIGW;

this.context = context;
this.requestContext = event.requestContext;

// Fields that depend on headers:
this.cookies = this._parseCookies();
this.cookies = this._parseCookies(event);
this.hostname = this._parseHostname();
this.ip = this._parseIP();
this.protocol = this._parseProtocol();
Expand Down Expand Up @@ -476,11 +478,20 @@ export default class Request {
}

private _parseHeaders(evt: RequestEvent): StringArrayOfStringsMap {
const headers = evt.multiValueHeaders || _.mapObject(evt.headers, (v) => { return [ v ]; });
let headers;
if (isAPIGatewayRequestEventV2(evt)) {
// NOTE - APIGWv2 multi-value headers that contain commas in their values will
// not be reconstructed as accurately
headers = _.mapObject(evt.headers, (v) => { return (v || '').split(','); });
} else {
headers = evt.multiValueHeaders || _.mapObject(evt.headers, (v) => { return [ v ]; });
}

return _.reduce(headers, (memo: StringArrayOfStringsMap, v, k) => {
const key = k.toLowerCase();

// evt.multiValueHeaders is a map that can contain undefined values
v ||= [];
memo[key] = v;

if (key === 'referer') {
Expand All @@ -493,8 +504,11 @@ export default class Request {
}, {});
}

private _parseCookies(): StringUnknownMap {
const cookieHeader = this.get('cookie') || '';
private _parseCookies(evt: RequestEvent): StringUnknownMap {
// TODO - is option 1 safe? If so, then it is the simpler approach
// Option 1. join evt.cookies with semicolons
// Option 2. reduce the list, parseing and merging into cookies
Comment on lines +508 to +510
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagging TODO for feedback.

const cookieHeader = isAPIGatewayRequestEventV2(evt) ? (evt.cookies || []).join(';'): this.get('cookie') || '';

if (_.isEmpty(cookieHeader)) {
return {};
Expand Down Expand Up @@ -533,15 +547,27 @@ export default class Request {
return this.requestContext.identity.sourceIp;
}

if ('http' in this.requestContext && !_.isEmpty(this.requestContext.http.sourceIp)) {
return this.requestContext.http.sourceIp;
}

if (!this.app.isEnabled('trust proxy')) {
return;
}

let ip = (this.get('x-forwarded-for') || '').replace(/,.*/, '');
// Since the X-Forwarded-For header may or may not have been split on commas,
// get the first element of the list, and then strip anything after the first comma.
let ip = (this.headerAll('x-forwarded-for') || [ '' ])[0].replace(/,.*/, '').trim();

return _.isEmpty(ip) ? undefined : ip;
}

private _parseMethod(evt: RequestEvent): string {
return (
isAPIGatewayRequestEventV2(evt) ? evt.requestContext.http.method : evt.httpMethod
).toUpperCase();
}

private _parseProtocol(): string | undefined {
if (this.isAPIGW()) {
return 'https';
Expand All @@ -554,27 +580,27 @@ export default class Request {
}
}

private _parseQuery(multiValQuery: StringArrayOfStringsMap, query: StringMap): { raw: string; parsed: KeyValueStringObject } {
let queryString;

private _parseQuery(multiValQuery: Record<string, string[] | undefined>, query: Record<string, string | undefined>, queryString: string | undefined): { raw: string; parsed: KeyValueStringObject } {
// It may seem strange to encode the URI components immediately after decoding them.
// But, this allows us to take values that are encoded and those that are not, then
// decode them to make sure we know they're not encoded, and then encode them so
// that we make an accurate raw query string to set on the URL parts of the request.
// If we simply encoded them, and we received a value that was still encoded
// already, then we would encode the `%` signs, etc, and end up with double-encoded
// values that were not correct.
if (_.isEmpty(multiValQuery)) {
queryString = _.reduce(query, (memo, v, k) => {
return memo + `&${k}=${encodeURIComponent(safeDecode(v))}`;
}, '');
} else {
queryString = _.reduce(multiValQuery, (memo, vals, k) => {
_.each(vals, (v) => {
memo += `&${k}=${encodeURIComponent(safeDecode(v))}`;
});
return memo;
}, '');
if (!queryString) {
if (_.isEmpty(multiValQuery)) {
queryString = _.reduce(query, (memo, v, k) => {
return memo + `&${k}=${encodeURIComponent(safeDecode(v || ''))}`;
}, '');
} else {
queryString = _.reduce(multiValQuery, (memo, vals, k) => {
_.each(vals || [], (v) => {
memo += `&${k}=${encodeURIComponent(safeDecode(v || ''))}`;
});
return memo;
}, '');
}
}

return { raw: queryString, parsed: qs.parse(queryString) };
Expand Down
9 changes: 5 additions & 4 deletions src/Response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from 'underscore';
import cookie from 'cookie';
import { Application, Request } from '.';
import { StringMap, isStringMap, StringArrayOfStringsMap } from '@silvermine/toolbox';
import { CookieOpts, ResponseResult } from './request-response-types';
import { CookieOpts, ResponseResult, isALBResult } from './request-response-types';
import { StatusCodes } from './status-codes';
import { Callback } from 'aws-lambda';
import mimeLookup from './mime/mimeLookup';
Expand Down Expand Up @@ -411,7 +411,7 @@ export default class Response {
body: this._body,
};

if (this.isALB()) {
if (isALBResult(output, this.isALB())) {
// There are some differences in the response format between APIGW and ALB. See
// https://serverless-training.com/articles/api-gateway-vs-application-load-balancer-technical-details/#application-load-balancer-response-event-format-differences

Expand All @@ -432,11 +432,12 @@ export default class Response {
// because it's the safest thing to do. Note that even if you have no headers
// to send, you must at least supply an empty object (`{}`) for ELB, whereas
// with APIGW it's okay to send `null`.
output.headers = _.reduce(output.multiValueHeaders, (memo, v, k) => {
output.headers = _.reduce(output.multiValueHeaders || {}, (memo, v, k) => {
memo[k] = v[v.length - 1];
return memo;
}, {} as StringMap);
}, {} as Record<string, boolean | number | string>);

// TODO - the following comment seems to conflict with the new data type
// Finally, note that ELB requires that all header values be strings already,
// whereas APIGW will allow booleans / integers as values, which it would then
// convert. As long as you're using this library from a TypeScript project, the
Expand Down
47 changes: 24 additions & 23 deletions src/request-response-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,35 @@

import {
APIGatewayEventRequestContext as OrigAPIGatewayEventRequestContext,
APIGatewayEventRequestContextV2 as OrigAPIGatewayEventRequestContextV2,
APIGatewayProxyEvent,
APIGatewayProxyEventV2,
Context,
APIGatewayProxyResult,
APIGatewayProxyStructuredResultV2,
ALBEvent,
ALBEventRequestContext,
ALBResult,
} from 'aws-lambda';
import { StringMap, StringArrayOfStringsMap } from '@silvermine/toolbox';

/* COMBO TYPES */

/**
* The `evt` argument passed to a Lambda handler that represents the request (from API
* Gateway or ALB).
*/
export type RequestEvent = ApplicationLoadBalancerRequestEvent | APIGatewayRequestEvent;
export type RequestEvent = ApplicationLoadBalancerRequestEvent | APIGatewayRequestEvent | APIGatewayRequestEventV2;

/**
* The "request context", which is accessible at `evt.requestContext`.
*/
export type RequestEventRequestContext = APIGatewayEventRequestContext | ApplicationLoadBalancerEventRequestContext;

export interface ResponseResult extends APIGatewayProxyResult {
multiValueHeaders: StringArrayOfStringsMap;
statusDescription?: string;
export type ResponseResult = APIGatewayProxyResult | APIGatewayProxyStructuredResultV2 | ALBResult;

export function isALBResult(evt: ResponseResult, test: boolean): evt is ALBResult {
// TODO - this type gaurd doesn't do any useful checking
return test && 'statusCode' in evt;
}
Comment on lines +31 to 34
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagging this TODO for feedback on this approach to using a type guard. Doesn't seem to be any attributes that can consistently be used to differentiate between the types. So seems like we can either:
A. Use a type guard like this.
B. Extend ALBResult to have an attribute we set to indicate if it is ALB, and then base the type guard on that attribute.


/**
Expand Down Expand Up @@ -51,29 +58,23 @@ if needed at a later time) */
export interface APIGatewayRequestEvent extends APIGatewayProxyEvent {}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface APIGatewayEventRequestContext extends OrigAPIGatewayEventRequestContext {}
export interface APIGatewayRequestEventV2 extends APIGatewayProxyEventV2 {}

export function isAPIGatewayRequestEventV2(evt: RequestEvent): evt is APIGatewayRequestEventV2 {
return ('apiId' in evt.requestContext && 'version' in evt && evt.version === '2.0');
}

/* APPLICATION LOAD BALANCER TYPES (these are not yet included in aws-lambda) */
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface APIGatewayEventRequestContext extends OrigAPIGatewayEventRequestContext {}

export interface ApplicationLoadBalancerRequestEvent {
body: string | null;
httpMethod: string;
isBase64Encoded: boolean;
path: string;
headers?: StringMap;
multiValueHeaders?: StringArrayOfStringsMap;
queryStringParameters?: StringMap;
multiValueQueryStringParameters?: StringArrayOfStringsMap;
requestContext: ApplicationLoadBalancerEventRequestContext;
}
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface APIGatewayEventRequestContextV2 extends OrigAPIGatewayEventRequestContextV2 {}

export interface ApplicationLoadBalancerEventRequestContext {
elb: {
targetGroupArn: string;
};
}
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface ApplicationLoadBalancerRequestEvent extends ALBEvent {}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface ApplicationLoadBalancerEventRequestContext extends ALBEventRequestContext {}

/* OTHER TYPES RELATED TO REQUESTS AND RESPONSES */
export interface CookieOpts {
Expand Down
Loading