Skip to content

fix: validation of dynamodb attribute name#2135

Merged
mergify[bot] merged 5 commits intomainlinefrom
unknown repository
Apr 5, 2021
Merged

fix: validation of dynamodb attribute name#2135
mergify[bot] merged 5 commits intomainlinefrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Apr 2, 2021

Fixes #2134

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ghost ghost self-requested a review as a code owner April 2, 2021 09:39
@ghost ghost requested a review from iamhopaul123 April 2, 2021 09:39
@ghost ghost changed the title fix validation of dynamodb attribute name fix: validation of dynamodb attribute name Apr 2, 2021
Copy link
Copy Markdown
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

Thank you so much for the fix!!! We really appreciate it 🙇🏼‍♀️

Comment thread internal/pkg/cli/validate.go Outdated
err = dynamoAttributeNameValidation(*attr.Name)
if err != nil {
return errValueBadFormatWithPeriodUnderscore
return errDDBAttributeBadSize
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we can return err here since it's also possible that err = errValueNotAString

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for your review. I agree with you and made another commit.

Copy link
Copy Markdown
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you very much for the fix!!

if err != nil {
return errDDBAttributeBadFormat
}
err = dynamoTableNameValidation(*attr.Name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is dynamoTableNameValidation used anywhere? if not can we delete it as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it is still in use in storage_init.go to validate DDB table name

@mergify mergify Bot merged commit 4e653ee into aws:mainline Apr 5, 2021
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
Fixes aws#2134

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Sign up for free to 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.

Storage init fails due to validateKey

3 participants