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

fix: restore with wildcard pattern repetition #51

Conversation

meirkl
Copy link
Contributor

@meirkl meirkl commented Aug 10, 2022

No description provided.

@meirkl
Copy link
Contributor Author

meirkl commented Aug 10, 2022

This PR is a draft because I discovered this bug through pino.

The behavior in pino is different than the behavior described in #50. In pino it causes infinite recursion.
The changes in the PR doesn't fix the issue in pino.

Pino reproduction

const pino = require('pino');

const logger = pino({
  redact: { paths: ['*.d', '*.*.d', '*.*.*.d'] },
});

const obj = {
  x: { c: { d: 'hide me', e: 'leave me be' } },
  y: { c: { d: 'and me', f: 'I want to live' } },
  z: { c: { d: 'and also I', g: 'I want to run in a stream' } },
};

logger.info(obj);
console.log(obj);

@mcollina
Copy link
Collaborator

This seems correct

@meirkeller
Copy link

It doesn't fix pinojs/pino#1513

@meirkl
Copy link
Contributor Author

meirkl commented Aug 15, 2022

@mcollina What about this?

@meirkl meirkl marked this pull request as ready for review August 15, 2022 18:51
@mcollina
Copy link
Collaborator

@meirkl send a PR.

@meirkl
Copy link
Contributor Author

meirkl commented Aug 17, 2022

@meirkl send a PR.

You want me to add this to the PR?

@mcollina
Copy link
Collaborator

I don't understand the status of this. We want the problem in pino resolved, right? Make it so ;).

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

@mcollina mcollina merged commit 32d4686 into davidmarkclements:main Aug 22, 2022
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.

4 participants