Skip to content

added invalid request on invalid payload deserialization - #310

Merged
ammokhov merged 3 commits into
aws-cloudformation:masterfrom
ammokhov:invalid-nested-request
Sep 16, 2020
Merged

added invalid request on invalid payload deserialization#310
ammokhov merged 3 commits into
aws-cloudformation:masterfrom
ammokhov:invalid-nested-request

Conversation

@ammokhov

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Currently, when we hit MismatchedInputException on payload deserialization we get InternalFailure, which is not surfacing real issue to the customer; Alternatively we could put model validation upfront but then we will have strict validation on stringification; this is a good alternative as customers can see that the input is actually invalid

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Comment threadsrc/test/java/software/amazon/cloudformation/LambdaWrapperTest.java Outdated
Comment threadpom.xml Outdated
Comment threadsrc/main/java/software/amazon/cloudformation/LambdaWrapper.java Outdated
Comment threadsrc/main/java/software/amazon/cloudformation/LambdaWrapper.java Outdated
Comment threadsrc/main/java/software/amazon/cloudformation/LambdaWrapper.java Outdated
"resourceProperties": {
"property1": "abc",
"property2": 123,
"fieldCausesValidationError": {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So the deserialization will fail if nested properties? But how the validation can successful? There is additionalProperties: false for the schema, so additional property should fail on validation.

@ammokhov
ammokhovforce-pushed the invalid-nested-request branch from e71b145 to 330f0c4CompareSeptember 16, 2020 07:20
@ammokhov
ammokhovforce-pushed the invalid-nested-request branch from 330f0c4 to 12435c5CompareSeptember 16, 2020 07:22

@wbingliwbingli left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Better to update the error message as it will be customer visible.

this.validator.validateObject(rawModelObject, resourceSchemaJSONObject);

handlerResponse = ProgressEvent.defaultFailureHandler(
new CfnInvalidRequestException("Model validation failed caused by invalid input provided", e),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This message will be customer visible. Maybe better wording as "Resource properties validation failed with invalid configuration".

"definitions": {
"subProperty": {
"type": "object",
"properties": {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All right, I think the whole problem is that the schema doesn't define "additionalProperties": false for the nest property. In such way, the deserialization to POJO fails due to this mismatch. However, validation pass as "additionalProperties": false is missing and validation will skip this additional properties.

The resource should fix the schema in such cases. But of course, this fix is still valuable.

@ammokhov
ammokhov merged commit 81b08b6 into aws-cloudformation:masterSep 16, 2020
@ammokhov
ammokhov deleted the invalid-nested-request branch September 16, 2020 17:19
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ammokhov@wbingli@sungkkim