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.
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
fix(cognito): quote or mime-encode fromName
to comply RFC 5322
#23227
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
I tried to update integration test but it failed with following error:
I cannot verify |
Is it acceptable to try following steps?
|
This is an excellent solution. |
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
Pull request has been modified.
@corymhall Thank you for review! I refactored them to separate functions and more explanations. |
27e2e71
to
0c336ca
Compare
* | ||
* @see https://www.rfc-editor.org/rfc/rfc5322#section-3.4 | ||
*/ | ||
function formatFromName(fromName: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks this is much easier to follow! The only question I have is how this
handles different types of quotes? Will it still work if you use "
or ```
fromName: "'foo.bar'"
fromName: `'${someVar}'`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per RFC 5322, both the single quote '
and the backquote `
are "atext". They are same as alphabet/digit letters. But surprising, the dot .
is a "specials".
atext = ALPHA / DIGIT / ; Printable US-ASCII
"!" / "#" / ; characters not including
"$" / "%" / ; specials. Used for atoms.
"&" / "'" /
"*" / "+" /
"-" / "/" /
"=" / "?" /
"^" / "_" /
"`" / "{" /
"|" / "}" /
"~"
Note: White space is not defined as "atext", but also allowed.
Examples:
source | template | cause |
---|---|---|
"'foo.bar'" |
quoted-string"From": "\"'foo.bar'\" <addr@example.com>" |
the dot . is "specials" |
"'foo bar'" |
bare"From": "'foo bar' <addr@example.com>" |
all chars are "atext" |
"Foo's Bar" |
bare"From": "Foo's Bar <addr@example.com>" |
all chars are "atext" |
'${price}' |
bare"From": "${price} <addr@example.com>" |
all chars are "atext" |
"`foo bar`" |
bare"From": "`foo bar` <addr@example.com>" |
all chars are "atext" |
'Café' |
mime encode"From": "=?UTF-8?B?Q2Fmw6k=?= <addr@example.com>" |
é is not US-ASCII |
'"Café"' |
entire "Café" including double quote " is mime encoded"From": "=?UTF-8?B?IkNhZsOpIg==?= <addr@example.com>" |
é is not US-ASCII |
`'${someVar}'` |
mime encode if someVar is not US-ASCIIquoted-string if someVar includes "specials"otherwise, as-is |
`~` is evaluated at runtimesingle quote ' is atext |
`"${someVar}"` |
mime encode if someVar is not US-ASCIIquoted-string again if someVar includes malformed escapeotherwise, as-is |
already quoted-string form |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…#23227) Closes aws#18903 When `fromName` of `UserPoolEmail.withSES()` does not comply RFC 5322 atom or quoted-string, it will be quoted or mime-encoded. For example: |`fromName`|Template|Description| |-|-|-| |`'simple atom'`|`"simple atom <address@example.com>"`|as is| |`'"quoted string"'`|`"\"quoted string\" <address@example.com>"`|as is| |`'あいう'`|`"=?UTF-8?B?44GC44GE44GG?= <address@example.com>"`|mime encode (RFC 2047)| |`'name@company'`|`"\"name@company\" <address@example.com>"`|make quoted-string| For details, see [RFC 5322 Section 3.4](https://www.rfc-editor.org/rfc/rfc5322#section-3.4) and [RFC 2047](https://www.rfc-editor.org/rfc/rfc2047) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…#23227) Closes aws#18903 When `fromName` of `UserPoolEmail.withSES()` does not comply RFC 5322 atom or quoted-string, it will be quoted or mime-encoded. For example: |`fromName`|Template|Description| |-|-|-| |`'simple atom'`|`"simple atom <address@example.com>"`|as is| |`'"quoted string"'`|`"\"quoted string\" <address@example.com>"`|as is| |`'あいう'`|`"=?UTF-8?B?44GC44GE44GG?= <address@example.com>"`|mime encode (RFC 2047)| |`'name@company'`|`"\"name@company\" <address@example.com>"`|make quoted-string| For details, see [RFC 5322 Section 3.4](https://www.rfc-editor.org/rfc/rfc5322#section-3.4) and [RFC 2047](https://www.rfc-editor.org/rfc/rfc2047) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Closes #18903
When
fromName
ofUserPoolEmail.withSES()
does not comply RFC 5322 atom or quoted-string, it will be quoted or mime-encoded.For example:
fromName
'simple atom'
"simple atom <address@example.com>"
'"quoted string"'
"\"quoted string\" <address@example.com>"
'あいう'
"=?UTF-8?B?44GC44GE44GG?= <address@example.com>"
'name@company'
"\"name@company\" <address@example.com>"
For details, see RFC 5322 Section 3.4 and RFC 2047
All Submissions:
Adding new Construct Runtime Dependencies:
New Features
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn integ
without--dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license