-
Notifications
You must be signed in to change notification settings - Fork 31
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: properly restore nested wildcard values #59
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.
Very hard to make sense of what's going on here without intimate knowledge of this package, so I'll just comment for now.
I'm fairly confident that this package has extensive code coverage and therefore, if all tests pass in addition to the new ones, you fixed the issue without introducing any regression, but the package maintainer is probably the best person to confirm that.
Apart from this, excellent job, the code is quite cryptic and solving the issue without introducing regressions is a remarkable achievement.
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.
lgtm
this is amazing work, thanks! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [fast-redact](https://togithub.com/davidmarkclements/fast-redact) | [`^3.2.0` -> `^3.3.0`](https://renovatebot.com/diffs/npm/fast-redact/3.2.0/3.3.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/fast-redact/3.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/fast-redact/3.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/fast-redact/3.2.0/3.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/fast-redact/3.2.0/3.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>davidmarkclements/fast-redact (fast-redact)</summary> ### [`v3.3.0`](https://togithub.com/davidmarkclements/fast-redact/releases/tag/v3.3.0) [Compare Source](https://togithub.com/davidmarkclements/fast-redact/compare/v3.2.0...v3.3.0) #### What's Changed - fix: properly restore nested wildcard values by [@​matt-clarson](https://togithub.com/matt-clarson) in [https://github.com/davidmarkclements/fast-redact/pull/59](https://togithub.com/davidmarkclements/fast-redact/pull/59) #### New Contributors - [@​matt-clarson](https://togithub.com/matt-clarson) made their first contribution in [https://github.com/davidmarkclements/fast-redact/pull/59](https://togithub.com/davidmarkclements/fast-redact/pull/59) **Full Changelog**: davidmarkclements/fast-redact@v3.2.0...v3.3.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - "after 8:00 before 23:00 every weekday except on Friday" in timezone UTC. 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/valora-inc/logging). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4yNC4yIiwidXBkYXRlZEluVmVyIjoiMzYuMjQuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fixes #50
also fixes pinojs/pino#1745
This fixes the issue of restoring redacting values across multiple nested wildcards - the problem was that the redact/restore modifiers assumed the same value could be used to restore all instances of a redacted path. To fix, I've changed the
specialSet
function to keep track of the paths it visits in a tree, so that the correct path/value can be stored for use in restoring.This has required a couple of extra modifications to the
specialSet
anditerateNthLevel
functions:exists
value for paths that aren't being redacted - where that was being done we can just not update the storeiterateNthLevel
function also doesn't returning anything now, similarly taking and mutating the store itselfiterateNthLevel
would exit early, leaving some parts of a wildcard path untouched - this is covered in a new test case