Skip to content
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

Pass strings through wildcard key redaction #32

Merged

Conversation

72636c
Copy link
Contributor

@72636c 72636c commented Mar 23, 2021

Per #31, we currently throw a TypeError when we try to apply a wildcard key redaction configuration to a string target. Strings spoofed themselves into an object code path due to gotchas like:

Object.keys('string')
// => [ '0', '1', '2', '3', '4', '5' ]

We already bail early on nullish targets, so we can extend this behaviour to cover string targets as well. Other primitives are already handled safely but I've added tests to cover them off.

Per davidmarkclements#31, we currently throw a `TypeError` when we try to apply a
wildcard key redaction configuration to a string target. Strings spoofed
themselves into an object code path due to gotchas like:

```js
Object.keys('string')
// => [ '0', '1', '2', '3', '4', '5' ]
```

We already bail early on nullish targets, so we can extend this
behaviour to cover string targets as well. Other primitives are already
handled safely but I've added tests to cover them off.
@72636c
Copy link
Contributor Author

72636c commented Mar 23, 2021

Unrelated to this change, it looks like CI is failing on Node.js 6 due to a new tap > [email protected] patch version using an object literal spread.

If we want to keep Node.js 6 compatibility we can try locking that dependency to v2.0.0.

@72636c
Copy link
Contributor Author

72636c commented Mar 29, 2021

cc @mcollina, if you have a moment to review 🙇

Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@P4sca1
Copy link

P4sca1 commented Feb 5, 2024

@davidmarkclements Is there anything else that needs to be done so that this can get merged and released? Do you need help with anything?

@mcollina mcollina merged commit 3e93d32 into davidmarkclements:main Mar 19, 2024
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.

3 participants