Skip to content

Commit

Permalink
fix: crash in Lambda handler with ELB or API Gateway event with no 'h…
Browse files Browse the repository at this point in the history
…eaders' (#3289)

If the lambda handler object for an ELB- or API Gateway-triggered Lambda
invocation did not have a 'headers' field, then the Lambda
instrumentation would crash. I'm not sure how to get an event with no
'headers' field, but we should still be defensive here.

Fixes: #3286
  • Loading branch information
trentm committed Apr 27, 2023
1 parent 530cc05 commit 3232ac2
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 5 deletions.
6 changes: 3 additions & 3 deletions lib/lambda.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ function setApiGatewayData (agent, trans, event, context, faasId, isColdStart) {
: undefined),
method: requestContext.http.method,
url: event.rawPath + (event.rawQueryString ? '?' + event.rawQueryString : ''),
headers: event.normedHeaders,
headers: event.normedHeaders || {},
socket: { remoteAddress: requestContext.http.sourceIp },
body: event.body
}
Expand All @@ -162,7 +162,7 @@ function setApiGatewayData (agent, trans, event, context, faasId, isColdStart) {
url: requestContext.path + (event.queryStringParameters
? '?' + querystring.encode(event.queryStringParameters)
: ''),
headers: event.normedHeaders,
headers: event.normedHeaders || {},
socket: { remoteAddress: requestContext.identity && requestContext.identity.sourceIp },
// Limitation: Note that `getContextFromRequest` does *not* use this body,
// because API Gateway payload format 1.0 does not include the
Expand Down Expand Up @@ -264,7 +264,7 @@ function setElbData (agent, trans, event, context, faasId, isColdStart) {
event.queryStringParameters && Object.keys(event.queryStringParameters) > 0
? '?' + querystring.encode(event.queryStringParameters)
: ''),
headers: event.normedHeaders,
headers: event.normedHeaders || {},
body: event.body,
bodyIsBase64Encoded: event.isBase64Encoded
}
Expand Down
15 changes: 14 additions & 1 deletion lib/parsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {
* However, some cases (e.g. Lambda and Azure Functions instrumentation)
* create a pseudo-req object that matches well enough for this function.
* Some relevant fields: (TODO: document all used fields)
* - `headers` - Required. An object.
* - `body` - The incoming request body, if available. The `json` and
* `payload` fields are also checked to accomodate some web frameworks.
* - `bodyIsBase64Encoded` - An optional boolean. If `true`, then the `body`
Expand Down Expand Up @@ -109,9 +110,21 @@ function getContextFromResponse (res, conf, isError) {
return context
}

/**
* Extract appropriate `{transaction,error}.context.user` from an HTTP
* request object.
*
* @param {Object} req - Typically `req` is a Node.js `http.IncomingMessage`.
* However, some cases (e.g. Lambda and Azure Functions instrumentation)
* create a pseudo-req object that matches well enough for this function.
* Some relevant fields: (TODO: document all used fields)
* - `headers` - Required. An object.
*/
function getUserContextFromRequest (req) {
var user = req.user || basicAuth(req) || req.session
if (!user) return
if (!user) {
return
}

var context = {}

Expand Down
38 changes: 37 additions & 1 deletion test/lambda/transaction2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// "AgentMock". The mock doesn't fully test the "transaction" intake event
// object creation path.)

const fs = require('fs')
const path = require('path')

const lambdaLocal = require('lambda-local')
const tape = require('tape')

Expand All @@ -35,7 +38,7 @@ process.env.AWS_LAMBDA_LOG_STREAM_NAME = '2021/11/01/[1.0]lambda/e7b05091b39b4aa
process.env.AWS_PROFILE = 'fake'

function loadFixture (file) {
return require('./fixtures/' + file)
return JSON.parse(fs.readFileSync(path.join(__dirname, 'fixtures', file)))
}

tape.test('lambda transactions', function (suite) {
Expand Down Expand Up @@ -215,8 +218,41 @@ tape.test('lambda transactions', function (suite) {
t.equal(trans.context.request.method, 'POST', 'transaction.context.request.method')
t.deepEqual(trans.context.response, { status_code: 502, headers: {} }, 'transaction.context.response')
}
},
{
name: 'API Gateway event, but without ".headers" field: APM agent should not crash',
event: (function () {
const ev = loadFixture('aws_api_http_test_data.json')
delete ev.headers
return ev
})(),
handler: (_event, _context, cb) => {
cb(null, { statusCode: 200, body: 'hi' })
},
checkApmEvents: (t, events) => {
const trans = events[1].transaction
t.equal(trans.name, 'POST /default/the-function-name', 'transaction.name')
t.equal(trans.outcome, 'success', 'transaction.outcome')
}
},
{
name: 'ELB event, but without ".headers" field: APM agent should not crash',
event: (function () {
const ev = loadFixture('aws_elb_test_data.json')
delete ev.headers
return ev
})(),
handler: (_event, _context, cb) => {
cb(null, { statusCode: 200, body: 'hi' })
},
checkApmEvents: (t, events) => {
const trans = events[1].transaction
t.equal(trans.faas.name, 'fixture-function-name', 'transaction.faas.name')
t.equal(trans.outcome, 'success', 'transaction.outcome')
}
}
]

testCases.forEach(c => {
suite.test(c.name, { skip: c.skip || false }, function (t) {
const handler = c.handler || (
Expand Down

0 comments on commit 3232ac2

Please sign in to comment.