chore(clients): strictly parse booleans #2514
Conversation
This adds a helper for parsing booleans from strings. Previously a simple `=== "true"` check was being performed, but this implies that anything that isn't that particular string is false.
JordonPhillips
force-pushed
the
parse-boolean
branch
from
be0a07c to
c909893
Compare
Co-authored-by: Trivikram Kamat <[email protected]>
|
CI Error: lerna ERR! yarn run build exited 2 in '@aws-sdk/aws-restjson'
--
429 | lerna ERR! yarn run build stdout:
430 | $ yarn build:cjs && yarn build:es
431 | $ tsc -p tsconfig.json
432 | protocols/Aws_restJson1.ts(2561,46): error TS2345: Argument of type 'string' is not assignable to parameter of type '"true" \| "false"'.
433 | protocols/Aws_restJson1.ts(2564,47): error TS2345: Argument of type 'string' is not assignable to parameter of type '"true" \| "false"'.
434 | protocols/Aws_restJson1.ts(2580,39): error TS2345: Argument of type 'string' is not assignable to parameter of type '"true" \| "false"'.
435 | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
436 | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
|
Co-authored-by: Trivikram Kamat <[email protected]>
trivikr
changed the title
Codecov Report
@@ Coverage Diff @@
## main #2514 +/- ##
=======================================
Coverage ? 59.55%
=======================================
Files ? 495
Lines ? 26402
Branches ? 6272
=======================================
Hits ? 15723
Misses ? 10679
Partials ? 0 Continue to review full report at Codecov.
|
| case "false": | ||
| return false; | ||
| default: | ||
| throw new Error(`Unable to parse boolean value "${value}"`); |
There was a problem hiding this comment.
should this be TypeError like the expect methods, or no?
There was a problem hiding this comment.
Technically it's not a type error, it's a parse error. We can always change it if necessary
There was a problem hiding this comment.
As long as at the end of the day we can generate a meaningful SerializationException for callers (when SerializationException has messages... whoops), that's all I care about. Reading messages from bare Error types is dangerous, could leak something unintended.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Issue
n/a
Description
This updates boolean parsing to be more strict. Previously there was a simple
=== "true"check, which allowed malformed requests to be accepted.Testing
Tests were added, including tests covering common alternatives for boolean string representations.
Additional context
This is part of a pass through to make sure the types we're receiving are actually the types we expect. There will likely be more PRs on this theme.
See smithy-lang/smithy-typescript#364 for the other half of this.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.