From b0b22ca3780bb9855ddda85c3910202c4c99a31d Mon Sep 17 00:00:00 2001 From: Warren Parad Date: Mon, 23 Sep 2024 11:56:31 +0200 Subject: [PATCH] Ensure no undefines in update requests. --- .vscode/settings.json | 1 + CHANGELOG.md | 1 + package.json | 3 ++ src/dynamoDbSafe.js | 25 ++++++++++---- tests/attributeNamesAndValues.test.js | 47 +++++++++++++++++++++++++++ yarn.lock | 2 +- 6 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 tests/attributeNamesAndValues.test.js diff --git a/.vscode/settings.json b/.vscode/settings.json index 953a4f3..42404f6 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,6 +2,7 @@ "cSpell.words": [ "atsquad", "cimpress", + "clonedeep", "Fqdn", "microsoftonline", "nosniff", diff --git a/CHANGELOG.md b/CHANGELOG.md index 57414ec..2b2a0f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ # Change log ## 1.0 ## +* Coerce `undefined` expression attribute values to `null`, because dynamoDb can't handle that, and it strips them from the request \ No newline at end of file diff --git a/package.json b/package.json index fff3155..ecfd27b 100644 --- a/package.json +++ b/package.json @@ -66,5 +66,8 @@ "homepage": "https://rhosys.ch", "engines": { "node": ">=16" + }, + "dependencies": { + "lodash.clonedeep": "^4.5.0" } } diff --git a/src/dynamoDbSafe.js b/src/dynamoDbSafe.js index 9b85f95..57d7b39 100644 --- a/src/dynamoDbSafe.js +++ b/src/dynamoDbSafe.js @@ -1,4 +1,5 @@ const DynamoDbOriginal = require('aws-sdk/clients/dynamodb'); +const cloneDeep = require('lodash.clonedeep'); process.env.AWS_NODEJS_CONNECTION_REUSE_ENABLED = 1; @@ -25,6 +26,18 @@ function parseExpression(logger, expression, isMultiExpression) { return { keys: {}, values: {} }; } + +function coerceExpressionAttributeNamesAndValues(parameters) { + const newParameters = cloneDeep(parameters); + Object.keys(newParameters.ExpressionAttributeValues || {}).map(key => { + if (newParameters.ExpressionAttributeValues[key] === undefined) { + newParameters.ExpressionAttributeValues[key] = null; + } + }); + + return newParameters; +} + class DynamoDB extends DynamoDbOriginal.DocumentClient { constructor(args) { super(args); @@ -35,7 +48,7 @@ class DynamoDB extends DynamoDbOriginal.DocumentClient { if (!originalParams || !originalParams.TableName) { throw new DynamoDbError({ error: 'TableName not specified', parameters: originalParams }, 'InvalidParameters'); } if (!originalParams.Key) { throw new DynamoDbError({ error: 'Key not specified', parameters: originalParams }, 'InvalidParameters'); } - const params = originalParams; + const params = coerceExpressionAttributeNamesAndValues(originalParams); const capturedStack = { name: 'DynamoDB.update() Error:' }; Error.captureStackTrace(capturedStack); const resultAsync = super.get(params).promise().catch(error => { @@ -53,7 +66,7 @@ class DynamoDB extends DynamoDbOriginal.DocumentClient { if (!originalParams || !originalParams.TableName) { throw new DynamoDbError({ error: 'TableName not specified', parameters: originalParams }, 'InvalidParameters'); } if (!originalParams.KeyConditionExpression) { throw new DynamoDbError({ error: 'KeyConditionExpression not specified', parameters: originalParams }, 'InvalidParameters'); } - const params = originalParams; + const params = coerceExpressionAttributeNamesAndValues(originalParams); const capturedStack = { name: 'DynamoDB.update() Error:' }; Error.captureStackTrace(capturedStack); const resultAsync = super.query(params).promise().catch(error => { @@ -76,7 +89,7 @@ class DynamoDB extends DynamoDbOriginal.DocumentClient { // Validate the tokens } - const params = originalParams; + const params = coerceExpressionAttributeNamesAndValues(originalParams); const capturedStack = { name: 'DynamoDB.update() Error:' }; Error.captureStackTrace(capturedStack); const resultAsync = super.delete(params).promise().catch(error => { @@ -99,7 +112,7 @@ class DynamoDB extends DynamoDbOriginal.DocumentClient { // Validate the tokens } - const params = originalParams; + const params = coerceExpressionAttributeNamesAndValues(originalParams); const capturedStack = { name: 'DynamoDB.update() Error:' }; Error.captureStackTrace(capturedStack); const resultAsync = super.put(params).promise().catch(error => { @@ -116,7 +129,7 @@ class DynamoDB extends DynamoDbOriginal.DocumentClient { scan(originalParams) { if (!originalParams || !originalParams.TableName) { throw new DynamoDbError({ error: 'TableName not specified', parameters: originalParams }, 'InvalidParameters'); } - const params = originalParams; + const params = coerceExpressionAttributeNamesAndValues(originalParams); const capturedStack = { name: 'DynamoDB.update() Error:' }; Error.captureStackTrace(capturedStack); const resultAsync = super.scan(params).promise().catch(error => { @@ -142,7 +155,7 @@ class DynamoDB extends DynamoDbOriginal.DocumentClient { // Validate the tokens } - const params = originalParams; + const params = coerceExpressionAttributeNamesAndValues(originalParams); const capturedStack = { name: 'DynamoDB.update() Error:' }; Error.captureStackTrace(capturedStack); const resultAsync = super.update(params).promise().catch(error => { diff --git a/tests/attributeNamesAndValues.test.js b/tests/attributeNamesAndValues.test.js new file mode 100644 index 0000000..4dd653a --- /dev/null +++ b/tests/attributeNamesAndValues.test.js @@ -0,0 +1,47 @@ +const { expect } = require('chai'); +const { describe, it, beforeEach, afterEach } = require('mocha'); +const sinon = require('sinon'); +const { DynamoDB: DynamoDbOriginal } = require('aws-sdk'); + +let sandbox; +beforeEach(() => { sandbox = sinon.createSandbox(); }); +afterEach(() => sandbox.restore()); + +const { DynamoDB } = require('../src/dynamoDbSafe'); +const cloneDeep = require('lodash.clonedeep'); + +describe('dynamoDbArmor.js', () => { + describe('namesAndValuesCoercion()', () => { + it('Fixes undefined expressionAttributeValue - Coerce `undefined` expression attribute values to `null`, because dynamoDb cant handle them, and it strips them from the request. nulls are not stripped', async () => { + const testTable = 'Test-TableId'; + const testHash = 'testHash'; + const testRange = 'testRange'; + const params = { + TableName: testTable, + Key: { + hash: testHash, + rang: testRange + }, + UpdateExpression: 'SET #key = :value', + ConditionExpression: 'attribute_exists(hash)', + ExpressionAttributeNames: { + '#key': 'key' + }, + ExpressionAttributeValues: { + ':value': undefined + } + }; + try { + const dynamoDbOriginalMock = sandbox.mock(DynamoDbOriginal.DocumentClient.prototype); + + const expectedParams = cloneDeep(params); + expectedParams.ExpressionAttributeValues[':value'] = null; + dynamoDbOriginalMock.expects('update').once().withArgs(expectedParams).returns({ promise() { return Promise.resolve(); } }); + await new DynamoDB().update(params); + dynamoDbOriginalMock.verify(); + } catch (error) { + expect(error.message).to.eql(null, JSON.stringify(error.message, null, 2)); + } + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 7a1dbcb..b894fb9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2619,7 +2619,7 @@ locate-path@^6.0.0: lodash.clonedeep@^4.5.0: version "4.5.0" resolved "https://registry.yarnpkg.com/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef" - integrity sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8= + integrity sha512-H5ZhCF25riFd9uB5UCkVKo61m3S/xZk1x4wA6yp/L3RFP6Z/eHH1ymQcGLo7J3GMPfm0V/7m1tryHuGVxpqEBQ== lodash.defaults@^4.2.0: version "4.2.0"