Skip to content

fix(idempotency): Validate payload for optimistic idempotent writes. #2261

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 3, 2025
+84 −19
Merged

fix(idempotency): Validate payload for optimistic idempotent writes. #2261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84c0f75
Validate payload for optimistic idempotent writes.
phipag Nov 3, 2025
5bf44d8
Re-throw IdempotencyValidationException instead of wrapping in a pers…
phipag Nov 3, 2025
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
2 changes: 2 additions & 0 deletions .../main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@
}
} catch (IdempotencyKeyException ike) {
throw ike;
} catch (IdempotencyValidationException ive) {
throw ive;
} catch (Exception e) {

Check warning on line 114 in powertools-idempotency/powertools-idempotency-core/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java

View workflow job for this annotation

GitHub Actions / pmd_analyse

'catch' branch identical to 'IdempotencyKeyException' branch 

Identical `catch` branches use up vertical space and increase the complexity of code without
adding functionality. It's better style to collapse identical branches into a single multi-catch
branch.

IdenticalCatchBranches (Priority: 3, Ruleset: Code Style)
https://docs.pmd-code.org/snapshot/pmd_rules_java_codestyle.html#identicalcatchbranches
throw new IdempotencyPersistenceLayerException(
"Failed to save in progress record to idempotency store. If you believe this is a Powertools for AWS Lambda (Java) bug, please open an issue.",
e);
Expand Down Expand Up @@ -210,7 +212,7 @@
Object response;
try {
response = function.execute();
} catch (Throwable handlerException) {

Check failure on line 215 in powertools-idempotency/powertools-idempotency-core/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java

View workflow job for this annotation

GitHub Actions / pmd_analyse

A catch statement should never catch throwable since it includes errors. 

Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such as
OutOfMemoryError that should be exposed and managed separately.

**Deprecated:** This rule is deprecated since PMD 7.18.0 and will be removed with PMD 8.0.0.
This rule has been subsumed by {% rule AvoidCatchingGenericException %},
which is now configurable as to which exceptions cause a violation.

AvoidCatchingThrowable (Priority: 1, Ruleset: Error Prone)
https://docs.pmd-code.org/snapshot/pmd_rules_java_errorprone.html#avoidcatchingthrowable
// We need these nested blocks to preserve function's exception in case the persistence store operation
// also raises an exception
try {
Expand Down
45 changes: 30 additions & 15 deletions .../java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@

import static software.amazon.lambda.powertools.common.internal.LambdaConstants.LAMBDA_FUNCTION_NAME_ENV;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectWriter;
import io.burt.jmespath.Expression;
import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
Expand All @@ -33,8 +29,15 @@
import java.util.Spliterators;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectWriter;

import io.burt.jmespath.Expression;
import software.amazon.lambda.powertools.idempotency.IdempotencyConfig;
import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyItemAlreadyExistsException;
import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyItemNotFoundException;
Expand Down Expand Up @@ -125,8 +128,7 @@ public void saveSuccess(JsonNode data, Object result, Instant now) {
DataRecord.Status.COMPLETED,
getExpiryEpochSecond(now),
responseJson,
getHashedPayload(data)
);
getHashedPayload(data));
LOG.debug("Function successfully executed. Saving record to persistence store with idempotency key: {}",
dataRecord.getIdempotencyKey());
updateRecord(dataRecord);
Expand Down Expand Up @@ -157,8 +159,8 @@ public void saveInProgress(JsonNode data, Instant now, OptionalInt remainingTime

OptionalLong inProgressExpirationMsTimestamp = OptionalLong.empty();
if (remainingTimeInMs.isPresent()) {
inProgressExpirationMsTimestamp =
OptionalLong.of(now.plus(remainingTimeInMs.getAsInt(), ChronoUnit.MILLIS).toEpochMilli());
inProgressExpirationMsTimestamp = OptionalLong
.of(now.plus(remainingTimeInMs.getAsInt(), ChronoUnit.MILLIS).toEpochMilli());
}

DataRecord dataRecord = new DataRecord(
Expand All @@ -167,10 +169,23 @@ public void saveInProgress(JsonNode data, Instant now, OptionalInt remainingTime
getExpiryEpochSecond(now),
null,
getHashedPayload(data),
inProgressExpirationMsTimestamp
);
inProgressExpirationMsTimestamp);
LOG.debug("saving in progress record for idempotency key: {}", dataRecord.getIdempotencyKey());
putRecord(dataRecord, now);

try {
putRecord(dataRecord, now);
} catch (IdempotencyItemAlreadyExistsException iaee) {
// Similar to getRecord, we need to call validatePayload before returning a data record.
// PR https://github.com/aws-powertools/powertools-lambda-java/pull/1821 introduced returning a data record
// through IdempotencyItemAlreadyExistsException to save DynamoDB calls when using DDB as store.
Optional<DataRecord> dr = iaee.getDataRecord();
if (dr.isPresent()) {
// throws IdempotencyValidationException if payload validation is enabled and failing
validatePayload(data, dr.get());
}

throw iaee;
}
}

/**
Expand All @@ -188,7 +203,7 @@ public void deleteRecord(JsonNode data, Throwable throwable) {

String idemPotencyKey = hashedIdempotencyKey.get();
LOG.debug("Function raised an exception {}. " +
"Clearing in progress record in persistence store for idempotency key: {}",
"Clearing in progress record in persistence store for idempotency key: {}",
throwable.getClass(),
idemPotencyKey);

Expand Down Expand Up @@ -255,9 +270,9 @@ private Optional<String> getHashedIdempotencyKey(JsonNode data) {

private boolean isMissingIdemPotencyKey(JsonNode data) {
if (data.isContainerNode()) {
Stream<JsonNode> stream =
StreamSupport.stream(Spliterators.spliteratorUnknownSize(data.elements(), Spliterator.ORDERED),
false);
Stream<JsonNode> stream = StreamSupport.stream(
Spliterators.spliteratorUnknownSize(data.elements(), Spliterator.ORDERED),
false);
return stream.allMatch(JsonNode::isNull);
}
return data.isNull();
Expand Down
56 changes: 52 additions & 4 deletions ...a/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.amazonaws.services.lambda.runtime.events.APIGatewayProxyRequestEvent;
import com.amazonaws.services.lambda.runtime.tests.EventLoader;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.DoubleNode;
import com.fasterxml.jackson.databind.node.TextNode;

Expand Down Expand Up @@ -145,8 +146,8 @@ void saveInProgress_jmespath_NotFound_shouldThrowException() {
assertThatThrownBy(
() -> persistenceStore.saveInProgress(JsonConfig.get().getObjectMapper().valueToTree(event), now,
OptionalInt.empty()))
.isInstanceOf(IdempotencyKeyException.class)
.hasMessageContaining("No data found to create a hashed idempotency key");
.isInstanceOf(IdempotencyKeyException.class)
.hasMessageContaining("No data found to create a hashed idempotency key");
assertThat(status).isEqualTo(-1);
}

Expand Down Expand Up @@ -181,7 +182,7 @@ void saveInProgress_withLocalCache_NotExpired_ShouldThrowException() {
assertThatThrownBy(
() -> persistenceStore.saveInProgress(JsonConfig.get().getObjectMapper().valueToTree(event), now,
OptionalInt.empty()))
.isInstanceOf(IdempotencyItemAlreadyExistsException.class);
.isInstanceOf(IdempotencyItemAlreadyExistsException.class);
assertThat(status).isEqualTo(-1);
}

Expand Down Expand Up @@ -243,7 +244,8 @@ void saveSuccess_withCacheEnabled_shouldSaveInCache() throws JsonProcessingExcep
DataRecord cachedDr = cache.get("testFunction#8d6a8f173b46479eff55e0997864a514");
assertThat(cachedDr.getStatus()).isEqualTo(DataRecord.Status.COMPLETED);
assertThat(cachedDr.getExpiryTimestamp()).isEqualTo(now.plus(3600, ChronoUnit.SECONDS).getEpochSecond());
assertThat(cachedDr.getResponseData()).isEqualTo(JsonConfig.get().getObjectMapper().writeValueAsString(product));
assertThat(cachedDr.getResponseData())
.isEqualTo(JsonConfig.get().getObjectMapper().writeValueAsString(product));
assertThat(cachedDr.getIdempotencyKey()).isEqualTo("testFunction#8d6a8f173b46479eff55e0997864a514");
assertThat(cachedDr.getPayloadHash()).isEmpty();
}
Expand Down Expand Up @@ -325,6 +327,52 @@ void getRecord_invalidPayload_shouldThrowValidationException() {

assertThatThrownBy(
() -> persistenceStore.getRecord(JsonConfig.get().getObjectMapper().valueToTree(event), Instant.now()))
.isInstanceOf(IdempotencyValidationException.class);
}

@Test
void saveInProgress_invalidPayload_shouldThrowValidationException() {
APIGatewayProxyRequestEvent event = EventLoader.loadApiGatewayRestEvent("apigw_event.json");
persistenceStore = new BasePersistenceStore() {
@Override
public DataRecord getRecord(String idempotencyKey) throws IdempotencyItemNotFoundException {
return new DataRecord(idempotencyKey, DataRecord.Status.INPROGRESS,
Instant.now().plus(3600, ChronoUnit.SECONDS).getEpochSecond(), "Response", "different hash");
}

@Override
public void putRecord(DataRecord dataRecord, Instant now) throws IdempotencyItemAlreadyExistsException {
DataRecord existingRecord = new DataRecord(
dataRecord.getIdempotencyKey(),
DataRecord.Status.INPROGRESS,
Instant.now().plus(3600, ChronoUnit.SECONDS).getEpochSecond(),
null,
"different hash");
throw new IdempotencyItemAlreadyExistsException("Item already exists", new Exception(), existingRecord);
}

@Override
public void updateRecord(DataRecord dataRecord) {
// Not needed for this test.
}

@Override
public void deleteRecord(String idempotencyKey) {
// Not needed for this test.

}
};

persistenceStore.configure(IdempotencyConfig.builder()
.withEventKeyJMESPath("powertools_json(body).id")
.withPayloadValidationJMESPath("powertools_json(body).message")
.build(),
"myfunc");

Instant now = Instant.now();
OptionalInt remainingTime = OptionalInt.empty();
JsonNode eventJson = JsonConfig.get().getObjectMapper().valueToTree(event);
assertThatThrownBy(() -> persistenceStore.saveInProgress(eventJson, now, remainingTime))
.isInstanceOf(IdempotencyValidationException.class);
}

Expand Down
Loading