-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Emit warning if ConvertTo-Json exceeds -Depth value
#13692
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
Conversation
|
@SteveL-MSFT If we will move to new .Net JSON API in next milestone (7.2 LTS!) does it make sense to change anything today in JSON cmdlets? |
|
@iSazonov this change doesn't impact functionality and addresses the concern that users aren't aware they aren't getting a full fidelity serialization of their object. I think it's ok to take this for 7.1 even if we plan to move to .NET JSON API in 7.2. |
test/powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1
Show resolved
Hide resolved
|
Thank you! |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
Not taking this to 7.0.x as it adds a new warning and is a change in behavior. |
PR Summary
When using
ConvertTo-Json, the default-Depthvalue is 2 to avoid recursion for objects like Services on Windows. Users who do not set a deep enough depth may be losing data they may not be aware of as they expected full fidelity of the object to be serialized to JSON. The change agreed by @PowerShell/powershell-committee is to emit a warning message once the depth is exceeded. The warning is only emitted once per object serialized, but note that arrays are treated as a single object and not unrolled.The SecureString tests were updated because it seems a credscan change identified a test "secret" in the script and caused CI to fail. After fixing that, I needed to have scriptanalyzer not fail on using plaintext "secret".
PR Context
Fix #8393
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.