knex
knex copied to clipboard
fix: clone config in client constructor
This PR ensures that config
objects passed to client constructors are deep-cloned before marking some of its sensitive properties as non-enumerable.
Fixes #5629 .
coverage: 92.784% (-0.003%) from 92.787% when pulling 0567fecf7a69ce28cbcfc2bdf205441004ef01f0 on castarco:regression-check into 4ca3dd5bc28e0665c5bed55026fac2ec45489d81 on knex:master.
Thanks for you work @castarco I will review as soon as possible (it's seems CI fail because of Node12...xd)
🙏
Rebased, seeing that my branch was a bit outdated.
Seems like CI is being randomly flaky again 😅
Any update on this?
I don't know. Nobody gave me any feedback regarding this patch.
will check it out this weeek, sorry
Adding my completely unhelpful context that I'm running into this using mssql, and am hopeful this would resolve the login error we're running into that is currently preventing us from adopting 3.0.1 (doesn't occur on 2.4.2)
I am waiting for the pull requests to be merged and the library versioned.😉
When will this fix be merged?
When will this fix be merged?
@araera111 I really don't know. I didn't get any extra feedback (besides a comment on test code that was only relevant for production code).
I guess I should rebase at some point, but I'm not sure it's worth investing time on it unless we get some guarantees that this has any chance of being merged.
@OlivierCavadenti @kibertoad When will this fix be merged?
What's needed to move this PR across the line? How can I help? I just lost hours of debugging to figure out that knex was unexpectedly mutating a configuration object.
@rluvaton could you please take a look?
Would it be quicker to release a patch with https://github.com/knex/knex/pull/5559 reverted?